| Message ID | 20231106093605.1349201-4-iii@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/s390x: CC fixes | expand |
On 11/6/23 01:31, Ilya Leoshkevich wrote: > LAALG uses op_laa() and wout_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 introducing op_laa_addu64(), which sets cc_src, and using it for > LAALG. > > 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/insn-data.h.inc | 2 +- > target/s390x/tcg/translate.c | 19 +++++++++++++++++-- > 2 files changed, 18 insertions(+), 3 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 06.11.23 10:31, Ilya Leoshkevich wrote: > LAALG uses op_laa() and wout_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 introducing op_laa_addu64(), which sets cc_src, and using it for > LAALG. > > 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/insn-data.h.inc | 2 +- > target/s390x/tcg/translate.c | 19 +++++++++++++++++-- > 2 files changed, 18 insertions(+), 3 deletions(-) > > diff --git a/target/s390x/tcg/insn-data.h.inc b/target/s390x/tcg/insn-data.h.inc > index 0bfd88d3c3a..2f07f39d9cb 100644 > --- a/target/s390x/tcg/insn-data.h.inc > +++ b/target/s390x/tcg/insn-data.h.inc > @@ -442,7 +442,7 @@ > D(0xebe8, LAAG, RSY_a, ILA, r3, a2, new, in2_r1, laa, adds64, MO_TEUQ) > /* LOAD AND ADD LOGICAL */ > D(0xebfa, LAAL, RSY_a, ILA, r3_32u, a2, new, in2_r1_32, laa, addu32, MO_TEUL) > - D(0xebea, LAALG, RSY_a, ILA, r3, a2, new, in2_r1, laa, addu64, MO_TEUQ) > + D(0xebea, LAALG, RSY_a, ILA, r3, a2, new, in2_r1, laa_addu64, addu64, MO_TEUQ) > /* LOAD AND AND */ > D(0xebf4, LAN, RSY_a, ILA, r3_32s, a2, new, in2_r1_32, lan, nz32, MO_TESL) > D(0xebe4, LANG, RSY_a, ILA, r3, a2, new, in2_r1, lan, nz64, MO_TEUQ) > diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c > index a0d6a2a35dd..62ab2be8b12 100644 > --- a/target/s390x/tcg/translate.c > +++ b/target/s390x/tcg/translate.c > @@ -2677,17 +2677,32 @@ static DisasJumpType op_kxb(DisasContext *s, DisasOps *o) > return DISAS_NEXT; > } > > -static DisasJumpType op_laa(DisasContext *s, DisasOps *o) > +static DisasJumpType help_laa(DisasContext *s, DisasOps *o, bool addu64) > { > /* The real output is indeed the original value in memory; > recompute the addition for the computation of CC. */ > 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); > + if (addu64) { > + tcg_gen_movi_i64(cc_src, 0); > + tcg_gen_add2_i64(o->out, cc_src, o->in1, cc_src, o->in2, cc_src); > + } else { > + tcg_gen_add_i64(o->out, o->in1, o->in2); > + } > return DISAS_NEXT; > } > > +static DisasJumpType op_laa(DisasContext *s, DisasOps *o) > +{ > + return help_laa(s, o, false); > +} > + > +static DisasJumpType op_laa_addu64(DisasContext *s, DisasOps *o) > +{ > + return help_laa(s, o, true); > +} > + > static DisasJumpType op_lan(DisasContext *s, DisasOps *o) > { > /* The real output is indeed the original value in memory; Reviewed-by: David Hildenbrand <david@redhat.com>
diff --git a/target/s390x/tcg/insn-data.h.inc b/target/s390x/tcg/insn-data.h.inc index 0bfd88d3c3a..2f07f39d9cb 100644 --- a/target/s390x/tcg/insn-data.h.inc +++ b/target/s390x/tcg/insn-data.h.inc @@ -442,7 +442,7 @@ D(0xebe8, LAAG, RSY_a, ILA, r3, a2, new, in2_r1, laa, adds64, MO_TEUQ) /* LOAD AND ADD LOGICAL */ D(0xebfa, LAAL, RSY_a, ILA, r3_32u, a2, new, in2_r1_32, laa, addu32, MO_TEUL) - D(0xebea, LAALG, RSY_a, ILA, r3, a2, new, in2_r1, laa, addu64, MO_TEUQ) + D(0xebea, LAALG, RSY_a, ILA, r3, a2, new, in2_r1, laa_addu64, addu64, MO_TEUQ) /* LOAD AND AND */ D(0xebf4, LAN, RSY_a, ILA, r3_32s, a2, new, in2_r1_32, lan, nz32, MO_TESL) D(0xebe4, LANG, RSY_a, ILA, r3, a2, new, in2_r1, lan, nz64, MO_TEUQ) diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c index a0d6a2a35dd..62ab2be8b12 100644 --- a/target/s390x/tcg/translate.c +++ b/target/s390x/tcg/translate.c @@ -2677,17 +2677,32 @@ static DisasJumpType op_kxb(DisasContext *s, DisasOps *o) return DISAS_NEXT; } -static DisasJumpType op_laa(DisasContext *s, DisasOps *o) +static DisasJumpType help_laa(DisasContext *s, DisasOps *o, bool addu64) { /* The real output is indeed the original value in memory; recompute the addition for the computation of CC. */ 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); + if (addu64) { + tcg_gen_movi_i64(cc_src, 0); + tcg_gen_add2_i64(o->out, cc_src, o->in1, cc_src, o->in2, cc_src); + } else { + tcg_gen_add_i64(o->out, o->in1, o->in2); + } return DISAS_NEXT; } +static DisasJumpType op_laa(DisasContext *s, DisasOps *o) +{ + return help_laa(s, o, false); +} + +static DisasJumpType op_laa_addu64(DisasContext *s, DisasOps *o) +{ + return help_laa(s, o, true); +} + static DisasJumpType op_lan(DisasContext *s, DisasOps *o) { /* The real output is indeed the original value in memory;
LAALG uses op_laa() and wout_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 introducing op_laa_addu64(), which sets cc_src, and using it for LAALG. 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/insn-data.h.inc | 2 +- target/s390x/tcg/translate.c | 19 +++++++++++++++++-- 2 files changed, 18 insertions(+), 3 deletions(-)