Re: [PATCH 3/4] target/s390x: Fix LAALG not updating cc_src

[Richard Henderson]

On 10/30/23 22:32, Ilya Leoshkevich wrote:
> LAALG uses op_laa() and cout_addu64(). The latter expects cc_src to be
> set, but the former does not do it. This can lead to assertion failures
> if something sets cc_src to neither 0 nor 1 before.
>
> Fix by setting cc_src in op_laa().
>
> Fixes: 4dba4d6fef61 ("target/s390x: Use atomic operations for LOAD AND OP")
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   target/s390x/tcg/translate.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
> index a0d6a2a35dd..db386ea94d8 100644
> --- a/target/s390x/tcg/translate.c
> +++ b/target/s390x/tcg/translate.c
> @@ -2684,7 +2684,8 @@ static DisasJumpType op_laa(DisasContext *s, DisasOps *o)
>       tcg_gen_atomic_fetch_add_i64(o->in2, o->in2, o->in1, get_mem_index(s),
>                                    s->insn->data | MO_ALIGN);
>       /* However, we need to recompute the addition for setting CC.  */
> -    tcg_gen_add_i64(o->out, o->in1, o->in2);
> +    tcg_gen_movi_i64(cc_src, 0);
> +    tcg_gen_add2_i64(o->out, cc_src, o->in1, cc_src, o->in2, cc_src);
>       return DISAS_NEXT;
>   }
>   

This is wrong, or at least not ideal -- op_laa is not *only* used with 
cout_addu64.
I think this would be better with a second version of op_laa.


r~