Re: [PATCH v2 1/2] target/s390x: Fix R[NOX]SBG with T=1

[Philippe Mathieu-Daudé]

On 16/3/23 00:56, Ilya Leoshkevich wrote:
> RXSBG usage in the "filetests" test from the wasmtime testsuite makes
> tcg_reg_alloc_op() attempt to temp_load() a TEMP_VAL_DEAD temporary,
> causing an assertion failure:
>
>      0x01000a70:  ec14 b040 3057  rxsbg    %r1, %r4, 0xb0, 0x40, 0x30
>
>      OP after optimization and liveness analysis:
>       ---- 0000000001000a70 0000000000000004 0000000000000006
>       rotl_i64 tmp2,r4,$0x30                   dead: 1 2  pref=0xffff
>       and_i64 tmp2,tmp2,$0x800000000000ffff    dead: 1  pref=0xffff
>      [xor_i64 tmp3,tmp3,tmp2                   dead: 1 2  pref=0xffff]
>       and_i64 cc_dst,tmp3,$0x800000000000ffff  sync: 0  dead: 0 1 2  pref=0xffff
>       mov_i64 psw_addr,$0x1000a76              sync: 0  dead: 0 1  pref=0xffff
>       mov_i32 cc_op,$0x6                       sync: 0  dead: 0 1  pref=0xffff
>       call lookup_tb_ptr,$0x6,$1,tmp8,env      dead: 1  pref=none
>       goto_ptr tmp8                            dead: 0
>       set_label $L0
>       exit_tb $0x7fffe809d183
>
>      ../tcg/tcg.c:3865: tcg fatal error
>
> The reason is that tmp3 does not have an initial value, which confuses
> the register allocator. This also affects the correctness of the
> results.
>
> Fix by assigning R1 to it.
>
> Fixes: d6c6372e186e ("target-s390: Implement R[NOX]SBG")

Exposed by 3ac6f91bca..dd161de75f?

3ac6f91bca target/s390x: Drop tcg_temp_free from translate.c
dd161de75f target/s390x: Remove g_out, g_out2, g_in1, g_in2

> Reviewed-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   target/s390x/tcg/translate.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
> index 14c3896d529..6dd2f41ad08 100644
> --- a/target/s390x/tcg/translate.c
> +++ b/target/s390x/tcg/translate.c
> @@ -3696,10 +3696,13 @@ static DisasJumpType op_rosbg(DisasContext *s, DisasOps 
> *o)
>       int i4 = get_field(s, i4);
>       int i5 = get_field(s, i5);
>       uint64_t mask;
> +    TCGv_i64 tmp;
>   
>       /* If this is a test-only form, arrange to discard the result.  */
>       if (i3 & 0x80) {

          tcg_debug_assert(o->out != NULL); ?

> +        tmp = o->out;
>           o->out = tcg_temp_new_i64();
> +        tcg_gen_mov_i64(o->out, tmp);

Something bugs me with this pattern but I can't say why yet :(

>       }
>   
>       i3 &= 63;