Re: [RFC PATCH 8/9] target/cris: Use tcg_gen_sextract_tl

[Edgar E. Iglesias]

On Tue, Oct 24, 2023 at 2:26 AM Richard Henderson <richard.henderson@linaro.org> 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>
> ---
> RFC: please double-check bits
> ---
>   target/cris/translate.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/target/cris/translate.c b/target/cris/translate.c
> index 65b07e1d80..3a161f8f73 100644
> --- a/target/cris/translate.c
> +++ b/target/cris/translate.c
> @@ -336,8 +336,7 @@ static void t_gen_cris_mstep(TCGv d, TCGv a, TCGv b, TCGv ccs)
>        */
>       t = tcg_temp_new();
>       tcg_gen_shli_tl(d, a, 1);
> -    tcg_gen_shli_tl(t, ccs, 31 - 3);
> -    tcg_gen_sari_tl(t, t, 31);
> +    tcg_gen_sextract_tl(t, ccs, 3, 1);

tcg_gen_sextract_tl(t, ccs, ctz32(N_FLAG), 1);

Looks good!

I think the intent was a branch-free version of:

if (ccs & N_FLAG) {

    d += b;

}

Or:

t = ccs & N_FLAG ? UINT32_MAX : 0;

d += b & t;

 

Also, it appears t_gen_cris_mstep consumes CCS without making sure that it is up-to-date.
Edgar?

Yes, that looks suspicious!

Best regards,

Edgar

 

r~