| Message ID | 20220204070011.573941-2-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tcg/sparc: Unaligned access for user-only | expand |
On Fri, 4 Feb 2022 at 07:53, Richard Henderson <richard.henderson@linaro.org> wrote: > > This will allow us to control exactly what scratch register is > used for loading the constant. Also, fix a theoretical problem > in recursing through tcg_out_movi, which may provide a different > value for in_prologue. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > tcg/sparc/tcg-target.c.inc | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > static void tcg_out_movi(TCGContext *s, TCGType type, > TCGReg ret, tcg_target_long arg) > { > - tcg_out_movi_int(s, type, ret, arg, false); > + tcg_debug_assert(ret != TCG_REG_T2); > + tcg_out_movi_int(s, type, ret, arg, false, TCG_REG_T2); Here we assert that 'ret' isn't TCG_REG_T2, but in tcg_out_addsub2_i64() we do: tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_T2, bh); and tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_T2, bh + (is_sub ? -1 : 1)); Otherwise looks OK. thanks -- PMM
On 2/5/22 04:35, Peter Maydell wrote: > On Fri, 4 Feb 2022 at 07:53, Richard Henderson > <richard.henderson@linaro.org> wrote: >> >> This will allow us to control exactly what scratch register is >> used for loading the constant. Also, fix a theoretical problem >> in recursing through tcg_out_movi, which may provide a different >> value for in_prologue. >> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> tcg/sparc/tcg-target.c.inc | 20 ++++++++++++-------- >> 1 file changed, 12 insertions(+), 8 deletions(-) > >> static void tcg_out_movi(TCGContext *s, TCGType type, >> TCGReg ret, tcg_target_long arg) >> { >> - tcg_out_movi_int(s, type, ret, arg, false); >> + tcg_debug_assert(ret != TCG_REG_T2); >> + tcg_out_movi_int(s, type, ret, arg, false, TCG_REG_T2); > > Here we assert that 'ret' isn't TCG_REG_T2, but in > tcg_out_addsub2_i64() we do: > > tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_T2, bh); > and > tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_T2, bh + (is_sub ? -1 : 1)); > > Otherwise looks OK. Oops. Good catch. I shouldn't have moved the assert. r~
diff --git a/tcg/sparc/tcg-target.c.inc b/tcg/sparc/tcg-target.c.inc index 0c062c60eb..7e3758b798 100644 --- a/tcg/sparc/tcg-target.c.inc +++ b/tcg/sparc/tcg-target.c.inc @@ -414,7 +414,8 @@ static void tcg_out_movi_imm13(TCGContext *s, TCGReg ret, int32_t arg) } static void tcg_out_movi_int(TCGContext *s, TCGType type, TCGReg ret, - tcg_target_long arg, bool in_prologue) + tcg_target_long arg, bool in_prologue, + TCGReg scratch) { tcg_target_long hi, lo = (int32_t)arg; tcg_target_long test, lsb; @@ -471,22 +472,24 @@ static void tcg_out_movi_int(TCGContext *s, TCGType type, TCGReg ret, /* A 64-bit constant decomposed into 2 32-bit pieces. */ if (check_fit_i32(lo, 13)) { hi = (arg - lo) >> 32; - tcg_out_movi(s, TCG_TYPE_I32, ret, hi); + tcg_out_movi_int(s, TCG_TYPE_I32, ret, hi, in_prologue, scratch); tcg_out_arithi(s, ret, ret, 32, SHIFT_SLLX); tcg_out_arithi(s, ret, ret, lo, ARITH_ADD); } else { + tcg_debug_assert(scratch != TCG_REG_G0); hi = arg >> 32; - tcg_out_movi(s, TCG_TYPE_I32, ret, hi); - tcg_out_movi(s, TCG_TYPE_I32, TCG_REG_T2, lo); + tcg_out_movi_int(s, TCG_TYPE_I32, ret, hi, in_prologue, scratch); + tcg_out_movi_int(s, TCG_TYPE_I32, scratch, lo, in_prologue, TCG_REG_G0); tcg_out_arithi(s, ret, ret, 32, SHIFT_SLLX); - tcg_out_arith(s, ret, ret, TCG_REG_T2, ARITH_OR); + tcg_out_arith(s, ret, ret, scratch, ARITH_OR); } } static void tcg_out_movi(TCGContext *s, TCGType type, TCGReg ret, tcg_target_long arg) { - tcg_out_movi_int(s, type, ret, arg, false); + tcg_debug_assert(ret != TCG_REG_T2); + tcg_out_movi_int(s, type, ret, arg, false, TCG_REG_T2); } static void tcg_out_ldst_rr(TCGContext *s, TCGReg data, TCGReg a1, @@ -837,7 +840,7 @@ static void tcg_out_call_nodelay(TCGContext *s, const tcg_insn_unit *dest, } else { uintptr_t desti = (uintptr_t)dest; tcg_out_movi_int(s, TCG_TYPE_PTR, TCG_REG_T1, - desti & ~0xfff, in_prologue); + desti & ~0xfff, in_prologue, TCG_REG_O7); tcg_out_arithi(s, TCG_REG_O7, TCG_REG_T1, desti & 0xfff, JMPL); } } @@ -1013,7 +1016,8 @@ static void tcg_target_qemu_prologue(TCGContext *s) #ifndef CONFIG_SOFTMMU if (guest_base != 0) { - tcg_out_movi_int(s, TCG_TYPE_PTR, TCG_GUEST_BASE_REG, guest_base, true); + tcg_out_movi_int(s, TCG_TYPE_PTR, TCG_GUEST_BASE_REG, guest_base, + true, TCG_REG_T1); tcg_regset_set_reg(s->reserved_regs, TCG_GUEST_BASE_REG); } #endif
This will allow us to control exactly what scratch register is used for loading the constant. Also, fix a theoretical problem in recursing through tcg_out_movi, which may provide a different value for in_prologue. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- tcg/sparc/tcg-target.c.inc | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)