diff mbox series

[v2,20/32] arm/translate-a64: add FP16 FCMxx (zero) to simd_two_reg_misc_fp16

Message ID 20180208173157.24705-21-alex.bennee@linaro.org
State New
Headers show
Series Add ARMv8.2 half-precision functions | expand

Commit Message

Alex Bennée Feb. 8, 2018, 5:31 p.m. UTC 
I re-use the existing handle_2misc_fcmp_zero handler and tweak it
slightly to deal with the half-precision case.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 target/arm/translate-a64.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

Comments

Richard Henderson Feb. 8, 2018, 10:39 p.m. UTC | #1 
On 02/08/2018 09:31 AM, Alex Bennée wrote:
> +            maxpasses = hp ? (is_q ? 8 : 4) : (is_q ? 4 : 2);

  (8 << is_q) >> size

?


> +            read_vec_element_i32(s, tcg_op, rn, pass, hp ? MO_16 : MO_32);

You already have size.

> +        return;
> +        break;

Unreachable break.


r~
Alex Bennée Feb. 22, 2018, 5:23 p.m. UTC | #2 
Richard Henderson <richard.henderson@linaro.org> writes:

> On 02/08/2018 09:31 AM, Alex Bennée wrote:
>> +            maxpasses = hp ? (is_q ? 8 : 4) : (is_q ? 4 : 2);
>
>   (8 << is_q) >> size
>
> ?

Hmm I'm not so sure about this. While mine is longer form at least the
intent is clear. What about:

    maxpasses = (is_q ? 4 : 2) << hp

It's still a little magical IMHO though...

--
Alex Bennée
Richard Henderson Feb. 22, 2018, 7:40 p.m. UTC | #3 
On 02/22/2018 09:23 AM, Alex Bennée wrote:
> 
> Richard Henderson <richard.henderson@linaro.org> writes:
> 
>> On 02/08/2018 09:31 AM, Alex Bennée wrote:
>>> +            maxpasses = hp ? (is_q ? 8 : 4) : (is_q ? 4 : 2);
>>
>>   (8 << is_q) >> size
>>
>> ?
> 
> Hmm I'm not so sure about this. While mine is longer form at least the
> intent is clear. What about:
> 
>     maxpasses = (is_q ? 4 : 2) << hp
> 
> It's still a little magical IMHO though...

Two variables then?

  vector_size = 8 << is_q;
  maxpasses = vector_size >> size;

Why do you want the "hp" variable when you already have "size"?


r~
Alex Bennée Feb. 23, 2018, 10:23 a.m. UTC | #4 
Richard Henderson <richard.henderson@linaro.org> writes:

> On 02/22/2018 09:23 AM, Alex Bennée wrote:
>>
>> Richard Henderson <richard.henderson@linaro.org> writes:
>>
>>> On 02/08/2018 09:31 AM, Alex Bennée wrote:
>>>> +            maxpasses = hp ? (is_q ? 8 : 4) : (is_q ? 4 : 2);
>>>
>>>   (8 << is_q) >> size
>>>
>>> ?
>>
>> Hmm I'm not so sure about this. While mine is longer form at least the
>> intent is clear. What about:
>>
>>     maxpasses = (is_q ? 4 : 2) << hp
>>
>> It's still a little magical IMHO though...
>
> Two variables then?
>
>   vector_size = 8 << is_q;
>   maxpasses = vector_size >> size;
>
> Why do you want the "hp" variable when you already have "size"?

Well I had introduced hp for:

  genfn = hp ? gen_helper_advsimd_cgt_f16 : gen_helper_neon_cgt_f32;

Instead of having it long form. I guess it could be neater though.

--
Alex Bennée
diff mbox series

Patch

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 0049111e6d..0efe9ae2fc 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -7769,14 +7769,14 @@  static void handle_2misc_fcmp_zero(DisasContext *s, int opcode,
                                    bool is_scalar, bool is_u, bool is_q,
                                    int size, int rn, int rd)
 {
-    bool is_double = (size == 3);
+    bool is_double = (size == MO_64);
     TCGv_ptr fpst;
 
     if (!fp_access_check(s)) {
         return;
     }
 
-    fpst = get_fpstatus_ptr(false);
+    fpst = get_fpstatus_ptr(size == MO_16);
 
     if (is_double) {
         TCGv_i64 tcg_op = tcg_temp_new_i64();
@@ -7828,6 +7828,7 @@  static void handle_2misc_fcmp_zero(DisasContext *s, int opcode,
         TCGv_i32 tcg_res = tcg_temp_new_i32();
         NeonGenTwoSingleOPFn *genfn;
         bool swap = false;
+        bool hp = (size == MO_16 ? true : false);
         int pass, maxpasses;
 
         switch (opcode) {
@@ -7835,16 +7836,16 @@  static void handle_2misc_fcmp_zero(DisasContext *s, int opcode,
             swap = true;
             /* fall through */
         case 0x2c: /* FCMGT (zero) */
-            genfn = gen_helper_neon_cgt_f32;
+            genfn = hp ? gen_helper_advsimd_cgt_f16 : gen_helper_neon_cgt_f32;
             break;
         case 0x2d: /* FCMEQ (zero) */
-            genfn = gen_helper_neon_ceq_f32;
+            genfn = hp ? gen_helper_advsimd_ceq_f16 : gen_helper_neon_ceq_f32;
             break;
         case 0x6d: /* FCMLE (zero) */
             swap = true;
             /* fall through */
         case 0x6c: /* FCMGE (zero) */
-            genfn = gen_helper_neon_cge_f32;
+            genfn = hp ? gen_helper_advsimd_cge_f16 : gen_helper_neon_cge_f32;
             break;
         default:
             g_assert_not_reached();
@@ -7853,11 +7854,11 @@  static void handle_2misc_fcmp_zero(DisasContext *s, int opcode,
         if (is_scalar) {
             maxpasses = 1;
         } else {
-            maxpasses = is_q ? 4 : 2;
+            maxpasses = hp ? (is_q ? 8 : 4) : (is_q ? 4 : 2);
         }
 
         for (pass = 0; pass < maxpasses; pass++) {
-            read_vec_element_i32(s, tcg_op, rn, pass, MO_32);
+            read_vec_element_i32(s, tcg_op, rn, pass, hp ? MO_16 : MO_32);
             if (swap) {
                 genfn(tcg_res, tcg_zero, tcg_op, fpst);
             } else {
@@ -7866,7 +7867,7 @@  static void handle_2misc_fcmp_zero(DisasContext *s, int opcode,
             if (is_scalar) {
                 write_fp_sreg(s, rd, tcg_res);
             } else {
-                write_vec_element_i32(s, tcg_res, rd, pass, MO_32);
+                write_vec_element_i32(s, tcg_res, rd, pass, hp ? MO_16 : MO_32);
             }
         }
         tcg_temp_free_i32(tcg_res);
@@ -10766,7 +10767,19 @@  static void disas_simd_two_reg_misc_fp16(DisasContext *s, uint32_t insn)
     fpop = deposit32(opcode, 5, 1, a);
     fpop = deposit32(fpop, 6, 1, u);
 
+    rd = extract32(insn, 0, 5);
+    rn = extract32(insn, 5, 5);
+
     switch (fpop) {
+    break;
+    case 0x2c: /* FCMGT (zero) */
+    case 0x2d: /* FCMEQ (zero) */
+    case 0x2e: /* FCMLT (zero) */
+    case 0x6c: /* FCMGE (zero) */
+    case 0x6d: /* FCMLE (zero) */
+        handle_2misc_fcmp_zero(s, fpop, is_scalar, 0, is_q, MO_16, rn, rd);
+        return;
+        break;
     case 0x18: /* FRINTN */
         need_rmode = true;
         only_in_vector = true;