Re: [PATCH 7/9] target/mips: Use tcg_gen_sextract_tl

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 24/10/23 02:14, Richard Henderson wrote:
> On 10/23/23 09:09, Philippe Mathieu-Daudé wrote:
>> Inspired-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/mips/tcg/mxu_translate.c | 6 ++----
>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/target/mips/tcg/mxu_translate.c 
>> b/target/mips/tcg/mxu_translate.c
>> index c517258ac5..6eb73256b2 100644
>> --- a/target/mips/tcg/mxu_translate.c
>> +++ b/target/mips/tcg/mxu_translate.c
>> @@ -3840,8 +3840,7 @@ static void gen_mxu_Q16SAT(DisasContext *ctx)
>>               tcg_gen_movi_tl(t0, 255);
>>               gen_set_label(l_lo);
>> -            tcg_gen_shli_tl(t1, mxu_gpr[XRb - 1], 16);
>> -            tcg_gen_sari_tl(t1, t1, 16);
>> +            tcg_gen_sextract_tl(t1, mxu_gpr[XRb - 1], 0, 16);
> 
> The most simple replacement here is tcg_gen_ext16s_tl.
> 
> I'll also note that the entire function should be replaced, e.g.
> 
>      TCGv min = tcg_constant_tl(0);
>      TCGv max = tcg_constant_tl(0xff);
>      TCGv tmp[2];
> 
>      tmp[0] = tcg_temp_new();
>      tmp[1] = tcg_temp_new();
> 
>      for (i = 0; i < 4; i++) {
>          int rs = i & 2 ? XRc : XRb;
>          TCGv t = tmp[i & 1];
> 
>          gen_load_mxu_gpr(t, rs);
>          tcg_gen_sextract_tl(t, t, (i & 1) * 16, 16);
>          tcg_gen_smax_tl(t, t, min);
>          tcg_gen_smin_tl(t, t, max);
>          if (i != 0) {
>              tcg_gen_shli_tl(t, t, i * 8);
>              tcg_gen_or_tl(t, t, tmp[(i - 1) & 1];
>          }
>      }
>      gen_store_mxu_gpr(tmp[1], XRa);
> 
> 
> And lots of other similar work within this file.  :-/

Yeah, this code is upported from some old tree. It should
use the TCG GVec API. See:
https://lore.kernel.org/qemu-devel/781894e9-2565-b54f-8df3-9cbd1cf68e2a@linaro.org/