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.
The example that I have in mind is:
- Comparison: r0_64 s<= -0xffffffffL;
- tcg_target_const_match(-0xffffffffL, TCG_TYPE_I64,
TCG_CT_CONST_S33) == true;
- (long)(int)-0xffffffffL != -0xffffffff;
- So we should end up in the "everything else" branch.
I suppose I could invent a new constraint that matches INT_MIN32 .. UINT32_MAX, which
would exclude this particular case. But it would still leave us loading INT32MIN .. -1
for unsigned and INT32MAX+1 .. UINT32_MAX for signed.
Since S33 existed already, I thought I would just re-use it.
r~
