On Tue, Dec 13, 2022 at 10:43:07AM -0600, Richard Henderson wrote: > On 12/13/22 10:25, Ilya Leoshkevich wrote: > > On Thu, Dec 08, 2022 at 08:05:28PM -0600, Richard Henderson wrote: > > > Give 64-bit comparison second operand a signed 33-bit immediate. > > > This is the smallest superset of uint32_t and int32_t, as used > > > by CLGFI and CGFI respectively. The rest of the 33-bit space > > > can be loaded into TCG_TMP0. Drop use of the constant pool. > > > > > > Signed-off-by: Richard Henderson <richard.hender...@linaro.org> > > > --- > > > tcg/s390x/tcg-target-con-set.h | 3 +++ > > > tcg/s390x/tcg-target.c.inc | 27 ++++++++++++++------------- > > > 2 files changed, 17 insertions(+), 13 deletions(-) > > > > <...> > > > --- a/tcg/s390x/tcg-target.c.inc > > > +++ b/tcg/s390x/tcg-target.c.inc > > > @@ -1249,22 +1249,20 @@ static int tgen_cmp2(TCGContext *s, TCGType type, > > > TCGCond c, TCGReg r1, > > > tcg_out_insn_RIL(s, op, r1, c2); > > > goto exit; > > > } > > > + > > > + /* > > > + * Constraints are for a signed 33-bit operand, which is a > > > + * convenient superset of this signed/unsigned test. > > > + */ > > > if (c2 == (is_unsigned ? (TCGArg)(uint32_t)c2 : > > > (TCGArg)(int32_t)c2)) { > > > op = (is_unsigned ? RIL_CLGFI : RIL_CGFI); > > > tcg_out_insn_RIL(s, op, r1, c2); > > > goto exit; > > > } > > > - /* Use the constant pool, but not for small constants. */ > > > - if (maybe_out_small_movi(s, type, TCG_TMP0, c2)) { > > > - c2 = TCG_TMP0; > > > - /* fall through to reg-reg */ > > > - } else { > > > - op = (is_unsigned ? RIL_CLGRL : RIL_CGRL); > > > - tcg_out_insn_RIL(s, op, r1, 0); > > > - new_pool_label(s, c2, R_390_PC32DBL, s->code_ptr - 2, 2); > > > - goto exit; > > > - } > > > + /* Load everything else into a register. */ > > > + tcg_out_movi(s, TCG_TYPE_I64, TCG_TMP0, c2); > > > + c2 = TCG_TMP0; > > > > What does tightening the constraint give us, if we have to handle the > > "everything else" case anyway, even for values that match > > TCG_CT_CONST_S33? > > Values outside const_s33 get loaded by the register allocator, which means > the value in the register might get re-used.
Thanks for the explanation! I did not consider the reuse of already loaded large 64-bit values. Reviewed-by: Ilya Leoshkevich <i...@linux.ibm.com>