| Message ID | 20201208180118.157911-6-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm: enforce alignment | expand |
On Tue, 8 Dec 2020 at 18:01, Richard Henderson <richard.henderson@linaro.org> wrote: > > Just because operating on a TCGv_i64 temporary does not > mean that we're performing a 64-bit operation. Restrict > the frobbing to actual 64-bit operations. If I understand correctly, this patch isn't actually a behaviour change because at this point the only users of gen_aa32_ld_i64() and gen_aa32_st_i64() are in fact performing 64-bit operations so the (opc & MO_SIZE) == MO_64 test is always true. (Presumably subsequent patches are going to add uses of these functions that want to load smaller sizes?) If that's right, worth mentioning explicitly in the commit message, I think. > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/arm/translate.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/target/arm/translate.c b/target/arm/translate.c > index f35d376341..ef9192cf6b 100644 > --- a/target/arm/translate.c > +++ b/target/arm/translate.c > @@ -949,7 +949,7 @@ static void gen_aa32_ld_i64(DisasContext *s, TCGv_i64 val, TCGv_i32 a32, > tcg_gen_qemu_ld_i64(val, addr, index, opc); > > /* Not needed for user-mode BE32, where we use MO_BE instead. */ > - if (!IS_USER_ONLY && s->sctlr_b) { > + if (!IS_USER_ONLY && s->sctlr_b && (opc & MO_SIZE) == MO_64) { > tcg_gen_rotri_i64(val, val, 32); > } > > @@ -968,7 +968,7 @@ static void gen_aa32_st_i64(DisasContext *s, TCGv_i64 val, TCGv_i32 a32, > TCGv addr = gen_aa32_addr(s, a32, opc); > > /* Not needed for user-mode BE32, where we use MO_BE instead. */ > - if (!IS_USER_ONLY && s->sctlr_b) { > + if (!IS_USER_ONLY && s->sctlr_b && (opc & MO_SIZE) == MO_64) { > TCGv_i64 tmp = tcg_temp_new_i64(); > tcg_gen_rotri_i64(tmp, val, 32); > tcg_gen_qemu_st_i64(tmp, addr, index, opc); Otherwise Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
On 1/7/21 6:00 AM, Peter Maydell wrote: > On Tue, 8 Dec 2020 at 18:01, Richard Henderson > <richard.henderson@linaro.org> wrote: >> >> Just because operating on a TCGv_i64 temporary does not >> mean that we're performing a 64-bit operation. Restrict >> the frobbing to actual 64-bit operations. > > If I understand correctly, this patch isn't actually a behaviour > change because at this point the only users of gen_aa32_ld_i64() > and gen_aa32_st_i64() are in fact performing 64-bit operations > so the (opc & MO_SIZE) == MO_64 test is always true. Correct. > (Presumably > subsequent patches are going to add uses of these functions that > want to load smaller sizes?) Correct. > If that's right, worth mentioning > explicitly in the commit message, I think. Will do. r~
diff --git a/target/arm/translate.c b/target/arm/translate.c index f35d376341..ef9192cf6b 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -949,7 +949,7 @@ static void gen_aa32_ld_i64(DisasContext *s, TCGv_i64 val, TCGv_i32 a32, tcg_gen_qemu_ld_i64(val, addr, index, opc); /* Not needed for user-mode BE32, where we use MO_BE instead. */ - if (!IS_USER_ONLY && s->sctlr_b) { + if (!IS_USER_ONLY && s->sctlr_b && (opc & MO_SIZE) == MO_64) { tcg_gen_rotri_i64(val, val, 32); } @@ -968,7 +968,7 @@ static void gen_aa32_st_i64(DisasContext *s, TCGv_i64 val, TCGv_i32 a32, TCGv addr = gen_aa32_addr(s, a32, opc); /* Not needed for user-mode BE32, where we use MO_BE instead. */ - if (!IS_USER_ONLY && s->sctlr_b) { + if (!IS_USER_ONLY && s->sctlr_b && (opc & MO_SIZE) == MO_64) { TCGv_i64 tmp = tcg_temp_new_i64(); tcg_gen_rotri_i64(tmp, val, 32); tcg_gen_qemu_st_i64(tmp, addr, index, opc);
Just because operating on a TCGv_i64 temporary does not mean that we're performing a 64-bit operation. Restrict the frobbing to actual 64-bit operations. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/translate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 2.25.1