Re: [PATCH v3 4/8] target/ppc: add vmulld instruction

[Lijun Pan]

On Jun 25, 2020, at 1:25 PM, Richard Henderson <richard.henderson@linaro.org> wrote:

On 6/25/20 10:00 AM, Lijun Pan wrote:

vmulld: Vector Multiply Low Doubleword.

Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
---
v3: use tcg_gen_gvec_mul()

target/ppc/translate/vmx-impl.inc.c | 1 +
target/ppc/translate/vmx-ops.inc.c  | 4 ++++

This part looks fine.

tcg/ppc/tcg-target.h                | 2 ++
tcg/ppc/tcg-target.inc.c            | 7 +++++--

This part must be a separate patch.

@@ -3149,6 +3150,7 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
    static const uint32_t
        add_op[4] = { VADDUBM, VADDUHM, VADDUWM, VADDUDM },
        sub_op[4] = { VSUBUBM, VSUBUHM, VSUBUWM, VSUBUDM },
+        mul_op[4] = { 0, 0, VMULUWM, VMULLD },
        neg_op[4] = { 0, 0, VNEGW, VNEGD },
        eq_op[4]  = { VCMPEQUB, VCMPEQUH, VCMPEQUW, VCMPEQUD },
        ne_op[4]  = { VCMPNEB, VCMPNEH, VCMPNEW, 0 },
@@ -3199,8 +3201,9 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
        a1 = 0;
        break;
    case INDEX_op_mul_vec:
-        tcg_debug_assert(vece == MO_32 && have_isa_2_07);
-        insn = VMULUWM;
+        tcg_debug_assert((vece == MO_32 && have_isa_2_07) ||
+                         (vece == MO_64 && have_isa_3_10));
+        insn = mul_op[vece];

I think it would be ok to just index mul_op here, since the real isa check is
to be done elsewhere.

Just keep "insn = mul_op[vece];"

and remove"        tcg_debug_assert((vece == MO_32 && have_isa_2_07) ||

                         (vece == MO_64 && have_isa_3_10));“?

Missing a change to tcg_can_emit_vec_op to do that isa check, and allow
INDEX_op_mul_vec to be used for MO_64.

something like below?

"

@@ -3016,6 +3016,8 @@ int tcg_can_emit_vec_op(TCGOpcode opc, TCGType type, unsigned vece)

             return -1;

         case MO_32:

             return have_isa_2_07 ? 1 : -1;

+        case MO_64:

+            return have_isa_3_10 ? 1 : -1;

         }

"

Missing a change to tcg_target_init to test for PPC_FEATURE2_ARCH_3_1.

something like below?

@@ -3712,6 +3712,11 @@ static void tcg_target_init(TCGContext *s)

         have_isa = tcg_isa_3_00;

     }

 #endif

+#ifdef PPC_FEATURE2_ARCH_3_10

+    if (hwcap2 & PPC_FEATURE2_ARCH_3_10) {

+        have_isa = tcg_isa_3_10;

+    }

+#endif

+++ b/include/elf.h

@@ -554,6 +554,7 @@ typedef struct {

 #define PPC_FEATURE2_HTM_NOSC           0x01000000

 #define PPC_FEATURE2_ARCH_3_00          0x00800000

 #define PPC_FEATURE2_HAS_IEEE128        0x00400000

+#define PPC_FEATURE2_ARCH_3_10          0x00200000

Thanks,

Lijun