| Message ID | 20240909231910.14428-2-philmd@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On 9/9/24 16:19, Philippe Mathieu-Daudé wrote: > From: Richard Henderson <richard.henderson@linaro.org> > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > Message-ID: <20240605215739.4758-7-richard.henderson@linaro.org> > [PMD: Split patch, part 2/4] > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > target/s390x/tcg/translate.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c > index faa6d37c8e..53ec817e29 100644 > --- a/target/s390x/tcg/translate.c > +++ b/target/s390x/tcg/translate.c > @@ -1417,6 +1417,7 @@ static DisasJumpType op_bas(DisasContext *s, DisasOps *o) > > static void save_link_info(DisasContext *s, DisasOps *o) > { > + TCGv_i64 t1; > TCGv_i64 t2; > > if (s->base.tb->flags & (FLAG_MASK_32 | FLAG_MASK_64)) { > @@ -1425,14 +1426,17 @@ static void save_link_info(DisasContext *s, DisasOps *o) > } > > gen_op_calc_cc(s); > + t1 = tcg_temp_new_i64(); > t2 = tcg_temp_new_i64(); > + > tcg_gen_andi_i64(o->out, o->out, 0xffffffff00000000ull); > + > + /* Shift program mask into place, garbage outside of [27:24]. */ > + tcg_gen_shri_i64(t1, psw_mask, 16); > + /* Deposit pc to replace garbage bits below program mask. */ > gen_psw_addr_disp(s, t2, s->ilen); > - tcg_gen_or_i64(o->out, o->out, t2); > + tcg_gen_deposit_i64(o->out, t1, t2, 0, 24); This is incorrect, as you've lost the high 32-bits of out. r~ > tcg_gen_ori_i64(o->out, o->out, (s->ilen / 2) << 30); > - tcg_gen_shri_i64(t2, psw_mask, 16); > - tcg_gen_andi_i64(t2, t2, 0x0f000000); > - tcg_gen_or_i64(o->out, o->out, t2); > tcg_gen_extu_i32_i64(t2, cc_op); > tcg_gen_shli_i64(t2, t2, 28); > tcg_gen_or_i64(o->out, o->out, t2);
On 10/9/24 01:50, Richard Henderson wrote: > On 9/9/24 16:19, Philippe Mathieu-Daudé wrote: >> From: Richard Henderson <richard.henderson@linaro.org> >> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> Message-ID: <20240605215739.4758-7-richard.henderson@linaro.org> >> [PMD: Split patch, part 2/4] >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> target/s390x/tcg/translate.c | 12 ++++++++---- >> 1 file changed, 8 insertions(+), 4 deletions(-) >> >> diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c >> index faa6d37c8e..53ec817e29 100644 >> --- a/target/s390x/tcg/translate.c >> +++ b/target/s390x/tcg/translate.c >> @@ -1417,6 +1417,7 @@ static DisasJumpType op_bas(DisasContext *s, >> DisasOps *o) >> static void save_link_info(DisasContext *s, DisasOps *o) >> { >> + TCGv_i64 t1; >> TCGv_i64 t2; >> if (s->base.tb->flags & (FLAG_MASK_32 | FLAG_MASK_64)) { >> @@ -1425,14 +1426,17 @@ static void save_link_info(DisasContext *s, >> DisasOps *o) >> } >> gen_op_calc_cc(s); >> + t1 = tcg_temp_new_i64(); >> t2 = tcg_temp_new_i64(); >> + >> tcg_gen_andi_i64(o->out, o->out, 0xffffffff00000000ull); >> + >> + /* Shift program mask into place, garbage outside of [27:24]. */ >> + tcg_gen_shri_i64(t1, psw_mask, 16); >> + /* Deposit pc to replace garbage bits below program mask. */ >> gen_psw_addr_disp(s, t2, s->ilen); >> - tcg_gen_or_i64(o->out, o->out, t2); >> + tcg_gen_deposit_i64(o->out, t1, t2, 0, 24); > > This is incorrect, as you've lost the high 32-bits of out. Ah, I felt something was not right but couldn't figure it out, thanks for pointing at it. The original patch is not trivial to review... > > r~ > > >> tcg_gen_ori_i64(o->out, o->out, (s->ilen / 2) << 30); >> - tcg_gen_shri_i64(t2, psw_mask, 16); >> - tcg_gen_andi_i64(t2, t2, 0x0f000000); >> - tcg_gen_or_i64(o->out, o->out, t2); >> tcg_gen_extu_i32_i64(t2, cc_op); >> tcg_gen_shli_i64(t2, t2, 28); >> tcg_gen_or_i64(o->out, o->out, t2); >
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c index faa6d37c8e..53ec817e29 100644 --- a/target/s390x/tcg/translate.c +++ b/target/s390x/tcg/translate.c @@ -1417,6 +1417,7 @@ static DisasJumpType op_bas(DisasContext *s, DisasOps *o) static void save_link_info(DisasContext *s, DisasOps *o) { + TCGv_i64 t1; TCGv_i64 t2; if (s->base.tb->flags & (FLAG_MASK_32 | FLAG_MASK_64)) { @@ -1425,14 +1426,17 @@ static void save_link_info(DisasContext *s, DisasOps *o) } gen_op_calc_cc(s); + t1 = tcg_temp_new_i64(); t2 = tcg_temp_new_i64(); + tcg_gen_andi_i64(o->out, o->out, 0xffffffff00000000ull); + + /* Shift program mask into place, garbage outside of [27:24]. */ + tcg_gen_shri_i64(t1, psw_mask, 16); + /* Deposit pc to replace garbage bits below program mask. */ gen_psw_addr_disp(s, t2, s->ilen); - tcg_gen_or_i64(o->out, o->out, t2); + tcg_gen_deposit_i64(o->out, t1, t2, 0, 24); tcg_gen_ori_i64(o->out, o->out, (s->ilen / 2) << 30); - tcg_gen_shri_i64(t2, psw_mask, 16); - tcg_gen_andi_i64(t2, t2, 0x0f000000); - tcg_gen_or_i64(o->out, o->out, t2); tcg_gen_extu_i32_i64(t2, cc_op); tcg_gen_shli_i64(t2, t2, 28); tcg_gen_or_i64(o->out, o->out, t2);