Re: [Qemu-arm] [PATCH v1 06/12] target/arm: Decode aa32 armv8.1 three same

["Alex Bennée" <alex.bennee@linaro.org>]

Richard Henderson <richard.henderson@linaro.org> writes:

> On 11/13/2017 05:55 PM, Alex Bennée wrote:
>>> +        case NEON_3R_VFM_VQRDMLSH:
>>> +            if (!u) {
>>> +                /* VFM, VFMS */
>>> +                if ((5 & (1 << size)) == 0) {
>>> +                    return 1;
>>> +                }
>>> +                break;
>>> +            }
>>> +            /* VQRDMLSH */
>>> +            switch (size) {
>>> +            case 1:
>>> +                fn_gvec_ptr = gen_helper_gvec_qrdmlsh_s16;
>>> +                break;
>>> +            case 2:
>>> +                fn_gvec_ptr = gen_helper_gvec_qrdmlsh_s32;
>>> +                break;
>>> +            default:
>>> +                return 1;
>>> +            }
>>> +            goto do_vqrdmlx;
>> Could we not take the opportunity to re-factor out the common bit rather
>> than make this mega
>
> What, specifically, did you have in mind?

Something like:

translate: use helper to avoid goto shenanigans

1 file changed, 18 insertions(+), 17 deletions(-)
target/arm/translate.c | 35 ++++++++++++++++++-----------------

modified   target/arm/translate.c
@@ -5576,6 +5576,20 @@ static const uint8_t neon_2rm_sizes[] = {
     [NEON_2RM_VCVT_UF] = 0x4,
 };

+/* expand v8.1 simd helper */
+static int do_qrdml(DisasContext *s, gen_helper_gvec_3_ptr *fn, int q, int rd, int rn, int rm)
+{
+    if (arm_dc_feature(s, ARM_FEATURE_V8_1_SIMD)) {
+        int opr_sz = (1 + q) * 8;
+        tcg_gen_gvec_3_ptr(vfp_reg_offset(1, rd),
+                           vfp_reg_offset(1, rn),
+                           vfp_reg_offset(1, rm), cpu_env,
+                           opr_sz, opr_sz, 0, fn);
+        return 0;
+    }
+    return 1;
+}
+
 /* Translate a NEON data processing instruction.  Return nonzero if the
    instruction is invalid.
    We process data in a mixture of 32-bit and 64-bit chunks.
@@ -5583,7 +5597,6 @@ static const uint8_t neon_2rm_sizes[] = {

 static int disas_neon_data_insn(DisasContext *s, uint32_t insn)
 {
-    void (*fn_gvec_ptr)(TCGv_ptr, TCGv_ptr, TCGv_ptr, TCGv_ptr, TCGv_i32);
     int op;
     int q;
     int rd, rn, rm;
@@ -5678,24 +5691,13 @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn)
             /* VQRDMLAH */
             switch (size) {
             case 1:
-                fn_gvec_ptr = gen_helper_gvec_qrdmlah_s16;
-                break;
+                return do_qrdml(s, gen_helper_gvec_qrdmlah_s16, q, rd, rn, rm);
             case 2:
-                fn_gvec_ptr = gen_helper_gvec_qrdmlah_s32;
+                return do_qrdml(s, gen_helper_gvec_qrdmlah_s32, q, rd, rn, rm);
                 break;
             default:
                 return 1;
             }
-        do_vqrdmlx:
-            if (arm_dc_feature(s, ARM_FEATURE_V8_1_SIMD)) {
-                int opr_sz = (1 + q) * 8;
-                tcg_gen_gvec_3_ptr(vfp_reg_offset(1, rd),
-                                   vfp_reg_offset(1, rn),
-                                   vfp_reg_offset(1, rm), cpu_env,
-                                   opr_sz, opr_sz, 0, fn_gvec_ptr);
-                return 0;
-            }
-            return 1;

         case NEON_3R_VFM_VQRDMLSH:
             if (!u) {
@@ -5708,15 +5710,14 @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn)
             /* VQRDMLSH */
             switch (size) {
             case 1:
-                fn_gvec_ptr = gen_helper_gvec_qrdmlsh_s16;
+                return do_qrdml(s, gen_helper_gvec_qrdmlsh_s16, q, rd, rn, rm);
                 break;
             case 2:
-                fn_gvec_ptr = gen_helper_gvec_qrdmlsh_s32;
+                return do_qrdml(s, gen_helper_gvec_qrdmlsh_s32, q, rd, rn, rm);
                 break;
             default:
                 return 1;
             }
-            goto do_vqrdmlx;
         }
         if (size == 3 && op != NEON_3R_LOGIC) {
             /* 64-bit element instructions. */


--
Alex Bennée