diff mbox series

[9/9] target/mips: Explode gen_msa_branch() as gen_msa_BxZ_V/BxZ()

Message ID 20201202184415.1434484-10-f4bug@amsat.org (mailing list archive)
State New, archived
Headers show
Series target/mips: Simplify MSA TCG logic | expand

Commit Message

Philippe Mathieu-Daudé Dec. 2, 2020, 6:44 p.m. UTC
In preparation of using the decodetree script, explode
gen_msa_branch() as following:

- OPC_BZ_V              -> BxZ_V(EQ)
- OPC_BNZ_V             -> BxZ_V(NE)
- OPC_BZ_[BHWD]         -> BxZ(false)
- OPC_BNZ_[BHWD]        -> BxZ(true)

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/translate.c | 71 ++++++++++++++++++++++++++++-------------
 1 file changed, 49 insertions(+), 22 deletions(-)

Comments

Richard Henderson Dec. 4, 2020, 5:04 p.m. UTC | #1
On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
> +static bool gen_msa_BxZ(DisasContext *ctx, int df, int wt, int s16, bool if_not)
> +{
> +    check_msa_access(ctx);
> +
> +    if (ctx->hflags & MIPS_HFLAG_BMASK) {
> +        generate_exception_end(ctx, EXCP_RI);
> +        return true;
> +    }
> +
> +    gen_check_zero_element(bcond, df, wt);
> +    if (if_not) {
> +        tcg_gen_setcondi_tl(TCG_COND_EQ, bcond, bcond, 0);
> +    }

Since gen_check_zero_element already produces a boolean, this is better as

  tcg_gen_xori_tl(bcond, bcond, if_not);

where tcg_gen_xori_tl already contains the if.

>      case OPC_BNZ_D:
> -        gen_check_zero_element(bcond, df, wt);
> -        tcg_gen_setcondi_tl(TCG_COND_EQ, bcond, bcond, 0);
> +        gen_msa_BxZ(ctx, df, wt, s16, true);

... oops, that'd be for a follow-up patch, to make this patch just code movement.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~
Philippe Mathieu-Daudé Dec. 4, 2020, 10:53 p.m. UTC | #2
On 12/4/20 6:04 PM, Richard Henderson wrote:
> On 12/2/20 12:44 PM, Philippe Mathieu-Daudé wrote:
>> +static bool gen_msa_BxZ(DisasContext *ctx, int df, int wt, int s16, bool if_not)
>> +{
>> +    check_msa_access(ctx);
>> +
>> +    if (ctx->hflags & MIPS_HFLAG_BMASK) {
>> +        generate_exception_end(ctx, EXCP_RI);
>> +        return true;
>> +    }
>> +
>> +    gen_check_zero_element(bcond, df, wt);
>> +    if (if_not) {
>> +        tcg_gen_setcondi_tl(TCG_COND_EQ, bcond, bcond, 0);
>> +    }
> 
> Since gen_check_zero_element already produces a boolean, this is better as
> 
>   tcg_gen_xori_tl(bcond, bcond, if_not);
> 
> where tcg_gen_xori_tl already contains the if.

Ah, got it.

> 
>>      case OPC_BNZ_D:
>> -        gen_check_zero_element(bcond, df, wt);
>> -        tcg_gen_setcondi_tl(TCG_COND_EQ, bcond, bcond, 0);
>> +        gen_msa_BxZ(ctx, df, wt, s16, true);
> 
> ... oops, that'd be for a follow-up patch, to make this patch just code movement.

Yes, will follow. I'm tempted to inline gen_check_zero_element (actually
move gen_msa_BxZ as gen_check_zero_element prologue/epilogue). Not sure
gen_check_zero_element() can be reused later though.

> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Thanks!

> 
> r~
>
Richard Henderson Dec. 5, 2020, 12:46 p.m. UTC | #3
On 12/4/20 4:53 PM, Philippe Mathieu-Daudé wrote:
> Yes, will follow. I'm tempted to inline gen_check_zero_element (actually
> move gen_msa_BxZ as gen_check_zero_element prologue/epilogue). Not sure
> gen_check_zero_element() can be reused later though.

The other thing that could happen is that gen_check_zero_element could grow a
TCGCond argument (or boolean) for the setcond at the end, so that we generate
the correct sense of the test in the first place.


r~
diff mbox series

Patch

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 5311e6ced62..8a35d4d0d03 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -28744,49 +28744,76 @@  static void gen_check_zero_element(TCGv tresult, uint8_t df, uint8_t wt)
     tcg_temp_free_i64(t1);
 }
 
+static bool gen_msa_BxZ_V(DisasContext *ctx, int wt, int s16, TCGCond cond)
+{
+    TCGv_i64 t0;
+
+    check_msa_access(ctx);
+
+    if (ctx->hflags & MIPS_HFLAG_BMASK) {
+        generate_exception_end(ctx, EXCP_RI);
+        return true;
+    }
+    t0 = tcg_temp_new_i64();
+    tcg_gen_or_i64(t0, msa_wr_d[wt << 1], msa_wr_d[(wt << 1) + 1]);
+    tcg_gen_setcondi_i64(cond, t0, t0, 0);
+    tcg_gen_trunc_i64_tl(bcond, t0);
+    tcg_temp_free_i64(t0);
+
+    ctx->btarget = ctx->base.pc_next + (s16 << 2) + 4;
+
+    ctx->hflags |= MIPS_HFLAG_BC;
+    ctx->hflags |= MIPS_HFLAG_BDS32;
+
+    return true;
+}
+
+static bool gen_msa_BxZ(DisasContext *ctx, int df, int wt, int s16, bool if_not)
+{
+    check_msa_access(ctx);
+
+    if (ctx->hflags & MIPS_HFLAG_BMASK) {
+        generate_exception_end(ctx, EXCP_RI);
+        return true;
+    }
+
+    gen_check_zero_element(bcond, df, wt);
+    if (if_not) {
+        tcg_gen_setcondi_tl(TCG_COND_EQ, bcond, bcond, 0);
+    }
+
+    ctx->btarget = ctx->base.pc_next + (s16 << 2) + 4;
+    ctx->hflags |= MIPS_HFLAG_BC;
+    ctx->hflags |= MIPS_HFLAG_BDS32;
+
+    return true;
+}
+
 static void gen_msa_branch(DisasContext *ctx, uint32_t op1)
 {
     uint8_t df = (ctx->opcode >> 21) & 0x3;
     uint8_t wt = (ctx->opcode >> 16) & 0x1f;
     int64_t s16 = (int16_t)ctx->opcode;
 
-    check_msa_access(ctx);
-
-    if (ctx->hflags & MIPS_HFLAG_BMASK) {
-        generate_exception_end(ctx, EXCP_RI);
-        return;
-    }
     switch (op1) {
     case OPC_BZ_V:
     case OPC_BNZ_V:
-        {
-            TCGv_i64 t0 = tcg_temp_new_i64();
-            tcg_gen_or_i64(t0, msa_wr_d[wt << 1], msa_wr_d[(wt << 1) + 1]);
-            tcg_gen_setcondi_i64((op1 == OPC_BZ_V) ?
-                    TCG_COND_EQ : TCG_COND_NE, t0, t0, 0);
-            tcg_gen_trunc_i64_tl(bcond, t0);
-            tcg_temp_free_i64(t0);
-        }
+        gen_msa_BxZ_V(ctx, wt, s16, (op1 == OPC_BZ_V) ?
+                                    TCG_COND_EQ : TCG_COND_NE);
         break;
     case OPC_BZ_B:
     case OPC_BZ_H:
     case OPC_BZ_W:
     case OPC_BZ_D:
-        gen_check_zero_element(bcond, df, wt);
+        gen_msa_BxZ(ctx, df, wt, s16, false);
         break;
     case OPC_BNZ_B:
     case OPC_BNZ_H:
     case OPC_BNZ_W:
     case OPC_BNZ_D:
-        gen_check_zero_element(bcond, df, wt);
-        tcg_gen_setcondi_tl(TCG_COND_EQ, bcond, bcond, 0);
+        gen_msa_BxZ(ctx, df, wt, s16, true);
         break;
     }
-
-    ctx->btarget = ctx->base.pc_next + (s16 << 2) + 4;
-
-    ctx->hflags |= MIPS_HFLAG_BC;
-    ctx->hflags |= MIPS_HFLAG_BDS32;
 }
 
 static void gen_msa_i8(DisasContext *ctx)