| Message ID | 20220206103138.36105-2-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | tcg/sparc: Unaligned access for user-only | expand |
On Sun, 6 Feb 2022 at 10:31, 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 | 21 +++++++++++++-------- > 1 file changed, 13 insertions(+), 8 deletions(-) > > diff --git a/tcg/sparc/tcg-target.c.inc b/tcg/sparc/tcg-target.c.inc > index 0c062c60eb..8c3671f56a 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,25 @@ 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); > + /* When outputting to T2, we have no scratch available. */ > + TCGReg scratch = ret != TCG_REG_T2 ? TCG_REG_T2 : TCG_REG_G0; Why won't using G0 trip the assertion above that scratch != TCG_REG_G0 ? > + tcg_out_movi_int(s, type, ret, arg, false, scratch); > } > -- PMM
On 2/8/22 04:59, Peter Maydell wrote: > On Sun, 6 Feb 2022 at 10:31, 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 | 21 +++++++++++++-------- >> 1 file changed, 13 insertions(+), 8 deletions(-) >> >> diff --git a/tcg/sparc/tcg-target.c.inc b/tcg/sparc/tcg-target.c.inc >> index 0c062c60eb..8c3671f56a 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,25 @@ 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); >> + /* When outputting to T2, we have no scratch available. */ >> + TCGReg scratch = ret != TCG_REG_T2 ? TCG_REG_T2 : TCG_REG_G0; > > Why won't using G0 trip the assertion above that scratch != TCG_REG_G0 ? It would only do so for full 64-bit constants, where the scratch is needed. For the case in which we use T2 explicitly, the value is constrained to J (a 13-bit constant). I suppose I could have also changed addsub2 to use tcg_out_movi_imm13 directly... r~
diff --git a/tcg/sparc/tcg-target.c.inc b/tcg/sparc/tcg-target.c.inc index 0c062c60eb..8c3671f56a 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,25 @@ 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); + /* When outputting to T2, we have no scratch available. */ + TCGReg scratch = ret != TCG_REG_T2 ? TCG_REG_T2 : TCG_REG_G0; + tcg_out_movi_int(s, type, ret, arg, false, scratch); } static void tcg_out_ldst_rr(TCGContext *s, TCGReg data, TCGReg a1, @@ -837,7 +841,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 +1017,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 | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)