Re: [PATCH v4 25/27] tcg/s390x: Tighten constraints for 64-bit compare

[Ilya Leoshkevich]

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.henderson@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?

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.

<...>

Best regards,
Ilya