| Message ID | 20231031053718.347100-4-iii@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/s390x: CC fixes | expand |
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~
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; }
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(-)