On Wed, Dec 13, 2023 at 5:51 PM Andrew Pinski <quic_apin...@quicinc.com> wrote:
>
> After r14-2667-gceae1400cf24f329393e96dd9720, we force a constant to a
> register
> if it is shared with one of the other operands. The problem is used the
> comparison
> mode for the register but that could be different from the operand mode. This
> causes some issues on some targets.
> To fix it, we either need to have the modes match or if it is an integer mode,
> then we can use the lower part for the smaller mode.
>
> Bootstrapped and tested on both aarch64-linux-gnu and x86_64-linux.
I think to fulfil the original purpose requiring matching modes is enough,
the x86 backend checks for equality here, a subreg wouldn't be enough.
In fact the whole point preserving equality doesn't work then.
So please just check the modes are equal (I also see I used
'mode' for the cost check - I've really split out the check done
by prepare_cmp_insn here btw). This seemed to be the simplest
solution at the time, rather than for example trying to postpone
legitimizing the constants (so rtx_equal_p could continue to be
lazy with slight mode mismatches).
Richard.
> PR middle-end/111260
>
> gcc/ChangeLog:
>
> * optabs.cc (emit_conditional_move): Fix up mode handling for
> forcing the constant to a register.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.c-torture/compile/condmove-1.c: New test.
>
> Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> ---
> gcc/optabs.cc | 40 +++++++++++++++++--
> .../gcc.c-torture/compile/condmove-1.c | 9 +++++
> 2 files changed, 45 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/gcc.c-torture/compile/condmove-1.c
>
> diff --git a/gcc/optabs.cc b/gcc/optabs.cc
> index f0a048a6bdb..573cf22760e 100644
> --- a/gcc/optabs.cc
> +++ b/gcc/optabs.cc
> @@ -5131,26 +5131,58 @@ emit_conditional_move (rtx target, struct
> rtx_comparison comp,
> /* If we are optimizing, force expensive constants into a register
> but preserve an eventual equality with op2/op3. */
> if (CONSTANT_P (orig_op0) && optimize
> + && (cmpmode == mode
> + || (GET_MODE_CLASS (cmpmode) == MODE_INT
> + && GET_MODE_CLASS (mode) == MODE_INT))
> && (rtx_cost (orig_op0, mode, COMPARE, 0,
> optimize_insn_for_speed_p ())
> > COSTS_N_INSNS (1))
> && can_create_pseudo_p ())
> {
> + machine_mode new_mode;
> + if (known_le (GET_MODE_PRECISION (cmpmode), GET_MODE_PRECISION
> (mode)))
> + new_mode = mode;
> + else
> + new_mode = cmpmode;
> if (rtx_equal_p (orig_op0, op2))
> - op2p = XEXP (comparison, 0) = force_reg (cmpmode, orig_op0);
> + {
> + rtx r = force_reg (new_mode, orig_op0);
> + op2p = gen_lowpart (mode, r);
> + XEXP (comparison, 0) = gen_lowpart (cmpmode, r);
> + }
> else if (rtx_equal_p (orig_op0, op3))
> - op3p = XEXP (comparison, 0) = force_reg (cmpmode, orig_op0);
> + {
> + rtx r = force_reg (new_mode, orig_op0);
> + op3p = gen_lowpart (mode, r);
> + XEXP (comparison, 0) = gen_lowpart (cmpmode, r);
> + }
> }
> if (CONSTANT_P (orig_op1) && optimize
> + && (cmpmode == mode
> + || (GET_MODE_CLASS (cmpmode) == MODE_INT
> + && GET_MODE_CLASS (mode) == MODE_INT))
> && (rtx_cost (orig_op1, mode, COMPARE, 0,
> optimize_insn_for_speed_p ())
> > COSTS_N_INSNS (1))
> && can_create_pseudo_p ())
> {
> + machine_mode new_mode;
> + if (known_le (GET_MODE_PRECISION (cmpmode), GET_MODE_PRECISION
> (mode)))
> + new_mode = mode;
> + else
> + new_mode = cmpmode;
> if (rtx_equal_p (orig_op1, op2))
> - op2p = XEXP (comparison, 1) = force_reg (cmpmode, orig_op1);
> + {
> + rtx r = force_reg (new_mode, orig_op1);
> + op2p = gen_lowpart (mode, r);
> + XEXP (comparison, 1) = gen_lowpart (cmpmode, r);
> + }
> else if (rtx_equal_p (orig_op1, op3))
> - op3p = XEXP (comparison, 1) = force_reg (cmpmode, orig_op1);
> + {
> + rtx r = force_reg (new_mode, orig_op1);
> + op3p = gen_lowpart (mode, r);
> + XEXP (comparison, 1) = gen_lowpart (cmpmode, r);
> + }
> }
> prepare_cmp_insn (XEXP (comparison, 0), XEXP (comparison, 1),
> GET_CODE (comparison), NULL_RTX, unsignedp,
> diff --git a/gcc/testsuite/gcc.c-torture/compile/condmove-1.c
> b/gcc/testsuite/gcc.c-torture/compile/condmove-1.c
> new file mode 100644
> index 00000000000..3fcc591af00
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/compile/condmove-1.c
> @@ -0,0 +1,9 @@
> +/* PR middle-end/111260 */
> +
> +/* Used to ICE while expansion of the `(a == b) ? b : 0;` */
> +int f1(long long a)
> +{
> + int b = 822920;
> + int t = a == b;
> + return t * (int)b;
> +}
> --
> 2.39.3
>
