Re: [PATCH v5 1/6] tcg/sparc: Add scratch argument to tcg_out_movi_int

[Richard Henderson]

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~