diff mbox series

[11/12] target/loongarch: Use generic hrev64_i32() in REVB.2H opcode

Message ID 20230822125221.55046-1-philmd@linaro.org
State New
Headers show
Series tcg: Factor hrev{32,64}_{i32,i64,tl} out | expand

Commit Message

Philippe Mathieu-Daudé Aug. 22, 2023, 12:52 p.m. UTC
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/loongarch/insn_trans/trans_bit.c.inc | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

Comments

Philippe Mathieu-Daudé Aug. 22, 2023, 1:30 p.m. UTC | #1
On 22/8/23 14:52, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/loongarch/insn_trans/trans_bit.c.inc | 15 +--------------
>   1 file changed, 1 insertion(+), 14 deletions(-)
> 
> diff --git a/target/loongarch/insn_trans/trans_bit.c.inc b/target/loongarch/insn_trans/trans_bit.c.inc
> index c04806dc21..9d564a0999 100644
> --- a/target/loongarch/insn_trans/trans_bit.c.inc
> +++ b/target/loongarch/insn_trans/trans_bit.c.inc
> @@ -111,19 +111,6 @@ static void gen_revb_2w(TCGv dest, TCGv src1)
>       tcg_gen_rotri_i64(dest, dest, 32);
>   }
>   
> -static void gen_revb_2h(TCGv dest, TCGv src1)
> -{
> -    TCGv mask = tcg_constant_tl(0x00FF00FF);
> -    TCGv t0 = tcg_temp_new();
> -    TCGv t1 = tcg_temp_new();
> -
> -    tcg_gen_shri_tl(t0, src1, 8);
> -    tcg_gen_and_tl(t0, t0, mask);
> -    tcg_gen_and_tl(t1, src1, mask);
> -    tcg_gen_shli_tl(t1, t1, 8);
> -    tcg_gen_or_tl(dest, t0, t1);
> -}
> -
>   static void gen_revh_2w(TCGv dest, TCGv src1)
>   {
>       TCGv_i64 t0 = tcg_temp_new_i64();
> @@ -161,7 +148,7 @@ TRANS(clo_d, gen_rr, EXT_NONE, EXT_NONE, gen_clo_d)
>   TRANS(clz_d, gen_rr, EXT_NONE, EXT_NONE, gen_clz_d)
>   TRANS(cto_d, gen_rr, EXT_NONE, EXT_NONE, gen_cto_d)
>   TRANS(ctz_d, gen_rr, EXT_NONE, EXT_NONE, gen_ctz_d)
> -TRANS(revb_2h, gen_rr, EXT_NONE, EXT_SIGN, gen_revb_2h)
> +TRANS(revb_2h, gen_rr, EXT_NONE, EXT_SIGN, tcg_gen_hrev32_i64)

While this file uses a mix of _i64/_tl (so likely doesn't build
for 32-bit target), we should use tcg_gen_hrev32_tl() I suppose...

>   TRANS(revb_4h, gen_rr, EXT_NONE, EXT_NONE, tcg_gen_hrev64_i64)
>   TRANS(revb_2w, gen_rr, EXT_NONE, EXT_NONE, gen_revb_2w)
>   TRANS(revb_d, gen_rr, EXT_NONE, EXT_NONE, tcg_gen_bswap64_i64)
Philippe Mathieu-Daudé Aug. 22, 2023, 4:11 p.m. UTC | #2
On 22/8/23 15:30, Philippe Mathieu-Daudé wrote:
> On 22/8/23 14:52, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/loongarch/insn_trans/trans_bit.c.inc | 15 +--------------
>>   1 file changed, 1 insertion(+), 14 deletions(-)
>>
>> diff --git a/target/loongarch/insn_trans/trans_bit.c.inc 
>> b/target/loongarch/insn_trans/trans_bit.c.inc
>> index c04806dc21..9d564a0999 100644
>> --- a/target/loongarch/insn_trans/trans_bit.c.inc
>> +++ b/target/loongarch/insn_trans/trans_bit.c.inc
>> @@ -111,19 +111,6 @@ static void gen_revb_2w(TCGv dest, TCGv src1)
>>       tcg_gen_rotri_i64(dest, dest, 32);
>>   }
>> -static void gen_revb_2h(TCGv dest, TCGv src1)
>> -{
>> -    TCGv mask = tcg_constant_tl(0x00FF00FF);
>> -    TCGv t0 = tcg_temp_new();
>> -    TCGv t1 = tcg_temp_new();
>> -
>> -    tcg_gen_shri_tl(t0, src1, 8);
>> -    tcg_gen_and_tl(t0, t0, mask);
>> -    tcg_gen_and_tl(t1, src1, mask);
>> -    tcg_gen_shli_tl(t1, t1, 8);

Looking at 
https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html#_revb_2h4h2wd
the sign extension is missing, so the next line:

>> -    tcg_gen_or_tl(dest, t0, t1);

should be replaced by smth like:

         tcg_gen_or_tl(t0, t0, t1);
         tcg_gen_ext32s_tl(dest, t0);

>> -}
Richard Henderson Aug. 22, 2023, 5:04 p.m. UTC | #3
On 8/22/23 09:11, Philippe Mathieu-Daudé wrote:
>>> -static void gen_revb_2h(TCGv dest, TCGv src1)
>>> -{
>>> -    TCGv mask = tcg_constant_tl(0x00FF00FF);
>>> -    TCGv t0 = tcg_temp_new();
>>> -    TCGv t1 = tcg_temp_new();
>>> -
>>> -    tcg_gen_shri_tl(t0, src1, 8);
>>> -    tcg_gen_and_tl(t0, t0, mask);
>>> -    tcg_gen_and_tl(t1, src1, mask);
>>> -    tcg_gen_shli_tl(t1, t1, 8);
> 
> Looking at 
> https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html#_revb_2h4h2wd
> the sign extension is missing, so the next line:
> 
>>> -    tcg_gen_or_tl(dest, t0, t1);
> 
> should be replaced by smth like:
> 
>          tcg_gen_or_tl(t0, t0, t1);
>          tcg_gen_ext32s_tl(dest, t0);

Extension is handled by

TRANS(revb_2h, gen_rr, EXT_NONE, EXT_SIGN, gen_revb_2h)
                                  ^^^^^^^^^
this


r~
diff mbox series

Patch

diff --git a/target/loongarch/insn_trans/trans_bit.c.inc b/target/loongarch/insn_trans/trans_bit.c.inc
index c04806dc21..9d564a0999 100644
--- a/target/loongarch/insn_trans/trans_bit.c.inc
+++ b/target/loongarch/insn_trans/trans_bit.c.inc
@@ -111,19 +111,6 @@  static void gen_revb_2w(TCGv dest, TCGv src1)
     tcg_gen_rotri_i64(dest, dest, 32);
 }
 
-static void gen_revb_2h(TCGv dest, TCGv src1)
-{
-    TCGv mask = tcg_constant_tl(0x00FF00FF);
-    TCGv t0 = tcg_temp_new();
-    TCGv t1 = tcg_temp_new();
-
-    tcg_gen_shri_tl(t0, src1, 8);
-    tcg_gen_and_tl(t0, t0, mask);
-    tcg_gen_and_tl(t1, src1, mask);
-    tcg_gen_shli_tl(t1, t1, 8);
-    tcg_gen_or_tl(dest, t0, t1);
-}
-
 static void gen_revh_2w(TCGv dest, TCGv src1)
 {
     TCGv_i64 t0 = tcg_temp_new_i64();
@@ -161,7 +148,7 @@  TRANS(clo_d, gen_rr, EXT_NONE, EXT_NONE, gen_clo_d)
 TRANS(clz_d, gen_rr, EXT_NONE, EXT_NONE, gen_clz_d)
 TRANS(cto_d, gen_rr, EXT_NONE, EXT_NONE, gen_cto_d)
 TRANS(ctz_d, gen_rr, EXT_NONE, EXT_NONE, gen_ctz_d)
-TRANS(revb_2h, gen_rr, EXT_NONE, EXT_SIGN, gen_revb_2h)
+TRANS(revb_2h, gen_rr, EXT_NONE, EXT_SIGN, tcg_gen_hrev32_i64)
 TRANS(revb_4h, gen_rr, EXT_NONE, EXT_NONE, tcg_gen_hrev64_i64)
 TRANS(revb_2w, gen_rr, EXT_NONE, EXT_NONE, gen_revb_2w)
 TRANS(revb_d, gen_rr, EXT_NONE, EXT_NONE, tcg_gen_bswap64_i64)