On Thu, Oct 15, 2026 at 8:08 AM Kyrylo Tkachov <[email protected]> wrote:
>
>
>
> > On 15 Oct 2026, at 14:37, Richard Earnshaw <[email protected]> wrote:
> >
> > On 02/10/2026 20:43, Dominic P wrote:
> >> MAX (x, -1) is x | (x >> (N - 1)) and MIN (x, -1) is x | ~(x >> (N - 1)),
> >> one orr/orn with a shifted operand, but without CSSC the smax/smin expander
> >> FAILs for anything other than 0 and we get cmp + csinv. Since
> >> r17-1683 (PR125641) match.pd also folds the source forms
> >> x | (x >> 31) and x | ~(x >> 31) to MAX_EXPR/MIN_EXPR, so these now
> >> regress from one instruction to two as well. Accept -1 in the expander
> >> and add a pattern next to the existing one for 0.
> >> Assisted-by: Claude Opus 5 (Anthropic)
> >> PR target/125640
> >> gcc/ChangeLog:
> >> * config/aarch64/aarch64.md (<optab><mode>3): Do not FAIL for
> >> SMAX and SMIN with -1.
> >> (*aarch64_<optab><mode>3_m1): New pattern.
> >> * config/aarch64/iterators.md (maxminorr): New code attribute.
> >> gcc/testsuite/ChangeLog:
> >> * gcc.target/aarch64/sminmax-asr_2.c: New test.
> >> * gcc.target/aarch64/sminmax-asr_3.c: New test.
> >> Signed-off-by: Dominic P <[email protected]>
> >> ---
> >> Effect at -O2 on an arm64 Linux 6.12 kernel (bcm2712_defconfig): 4
> >> cmp + csinv pairs become orr/orn in 4 objects, none grow; .text -76 bytes
> >> including alignment. On a Cortex-A72 (Pi 400) dependency-chain latency
> >> is unchanged (orr with an asr operand takes 2 cycles, as cmp + csinv
> >> do), so this is a size win of one instruction per use.
> >> Regression-tested with an aarch64-linux-gnu cross compiler (C testsuite
> >> under
> >> qemu user mode), together with the aarch64 patches for PR118666 and
> >> PR118648; no regressions.
> >> OK for trunk?
> >
> > This is a generic optimization, so I think should be recognized and handled
> > in match.pd. Even on targets without a shift/or instruction a shift
> > followed by an OR is likely to be better than a conditional sequence.
> >
>
> FWIW IMO this should be an expand-time optimization rather than match.pd.
> MAX with -1 is simpler GIMPLE than x | x >> (N - 1). Expand should sit in the
> sweet spot of knowing the target capabilities (single-instruction max or
> conditional moves) and knowing which constants are cheap.
Yes. I wrote something similar in the bug report:
The max form is Canonical for gimple. So if anything then expand.
Thanks,
Andrea
> Thanks,
> Kyrill
>
> > R.
> >
> >> gcc/config/aarch64/aarch64.md | 17 +++-
> >> gcc/config/aarch64/iterators.md | 1 +
> >> .../gcc.target/aarch64/sminmax-asr_2.c | 78 +++++++++++++++++++
> >> .../gcc.target/aarch64/sminmax-asr_3.c | 36 +++++++++
> >> 4 files changed, 129 insertions(+), 3 deletions(-)
> >> create mode 100644 gcc/testsuite/gcc.target/aarch64/sminmax-asr_2.c
> >> create mode 100644 gcc/testsuite/gcc.target/aarch64/sminmax-asr_3.c
> >> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> >> index 7c8915ec389..21fd6c909fb 100644
> >> --- a/gcc/config/aarch64/aarch64.md
> >> +++ b/gcc/config/aarch64/aarch64.md
> >> @@ -7684,8 +7684,8 @@
> >> ;; Expander for integer smin, smax, umin. Mainly used to generate
> >> ;; straightforward RTL for TARGET_CSSC. When that is not available
> >> ;; FAIL and let the generic expanders generate the CMP + CSEL sequences,
> >> -;; except for the SMIN and SMAX with zero cases, for which we have a
> >> -;; single instruction even for the base architecture.
> >> +;; except for the SMIN and SMAX with zero and minus one cases, for which
> >> +;; we have a single instruction even for the base architecture.
> >> (define_expand "<optab><mode>3"
> >> [(set (match_operand:GPI 0 "register_operand")
> >> (MAXMIN_NOUMAX:GPI
> >> @@ -7695,7 +7695,8 @@
> >> {
> >> if (!TARGET_CSSC)
> >> {
> >> - if (operands[2] != CONST0_RTX (<MODE>mode)
> >> + if ((operands[2] != CONST0_RTX (<MODE>mode)
> >> + && operands[2] != CONSTM1_RTX (<MODE>mode))
> >> || !(<CODE> == SMAX || <CODE> == SMIN))
> >> FAIL;
> >> }
> >> @@ -7723,6 +7724,16 @@
> >> [(set_attr "type" "logic_shift_imm")]
> >> )
> >> +(define_insn "*aarch64_<optab><mode>3_m1"
> >> + [(set (match_operand:GPI 0 "register_operand" "=r")
> >> + (FMAXMIN:GPI
> >> + (match_operand:GPI 1 "register_operand" "r")
> >> + (const_int -1)))]
> >> + ""
> >> + "<maxminorr>\\t%<w>0, %<w>1, %<w>1, asr <sizem1>";
> >> + [(set_attr "type" "logic_shift_imm")]
> >> +)
> >> +
> >> ;; Given that smax/smin do not specify the result when either input is
> >> NaN,
> >> ;; we could use either FMAXNM or FMAX for smax, and either FMINNM or FMIN
> >> ;; for smin.
> >> diff --git a/gcc/config/aarch64/iterators.md
> >> b/gcc/config/aarch64/iterators.md
> >> index 568786e3714..617a955e86d 100644
> >> --- a/gcc/config/aarch64/iterators.md
> >> +++ b/gcc/config/aarch64/iterators.md
> >> @@ -3540,6 +3540,7 @@
> >> (umin "min")])
> >> (define_code_attr maxminand [(smax "bic") (smin "and")])
> >> +(define_code_attr maxminorr [(smax "orr") (smin "orn")])
> >> (define_code_attr ovf_add_cmp [(umax "geu") (umin "ltu")])
> >> (define_code_attr udf_sub_cmp [(umax "ltu") (umin "geu")])
> >> diff --git a/gcc/testsuite/gcc.target/aarch64/sminmax-asr_2.c
> >> b/gcc/testsuite/gcc.target/aarch64/sminmax-asr_2.c
> >> new file mode 100644
> >> index 00000000000..05714b217a6
> >> --- /dev/null
> >> +++ b/gcc/testsuite/gcc.target/aarch64/sminmax-asr_2.c
> >> @@ -0,0 +1,78 @@
> >> +/* PR target/125640 */
> >> +/* { dg-do compile } */
> >> +/* { dg-options "-O2" } */
> >> +/* { dg-add-options check_function_bodies } */
> >> +/* { dg-final { check-function-bodies "**" "" "" } } */
> >> +
> >> +#include <stdint.h>
> >> +
> >> +#pragma GCC target "+nocssc"
> >> +
> >> +#define MAX(X, Y) ((X) > (Y) ? (X) : (Y))
> >> +#define MIN(X, Y) ((X) < (Y) ? (X) : (Y))
> >> +
> >> +/*
> >> +** maxm1:
> >> +** orr w0, w0, w0, asr #31
> >> +** ret
> >> +*/
> >> +int
> >> +maxm1 (int a)
> >> +{
> >> + return MAX (a, -1);
> >> +}
> >> +
> >> +/*
> >> +** minm1:
> >> +** orn w0, w0, w0, asr #31
> >> +** ret
> >> +*/
> >> +int
> >> +minm1 (int a)
> >> +{
> >> + return MIN (a, -1);
> >> +}
> >> +
> >> +/*
> >> +** maxm1l:
> >> +** orr x0, x0, x0, asr #63
> >> +** ret
> >> +*/
> >> +int64_t
> >> +maxm1l (int64_t a)
> >> +{
> >> + return MAX (a, -1);
> >> +}
> >> +
> >> +/*
> >> +** minm1l:
> >> +** orn x0, x0, x0, asr #63
> >> +** ret
> >> +*/
> >> +int64_t
> >> +minm1l (int64_t a)
> >> +{
> >> + return MIN (a, -1);
> >> +}
> >> +
> >> +/*
> >> +** ior_splat:
> >> +** orr w0, w0, w0, asr #31
> >> +** ret
> >> +*/
> >> +int
> >> +ior_splat (int a)
> >> +{
> >> + return a | (a >> 31);
> >> +}
> >> +
> >> +/*
> >> +** ior_not_splat:
> >> +** orn w0, w0, w0, asr #31
> >> +** ret
> >> +*/
> >> +int
> >> +ior_not_splat (int a)
> >> +{
> >> + return a | ~(a >> 31);
> >> +}
> >> diff --git a/gcc/testsuite/gcc.target/aarch64/sminmax-asr_3.c
> >> b/gcc/testsuite/gcc.target/aarch64/sminmax-asr_3.c
> >> new file mode 100644
> >> index 00000000000..1fe85fbd74f
> >> --- /dev/null
> >> +++ b/gcc/testsuite/gcc.target/aarch64/sminmax-asr_3.c
> >> @@ -0,0 +1,36 @@
> >> +/* PR target/125640 */
> >> +/* { dg-do run } */
> >> +/* { dg-options "-O2" } */
> >> +
> >> +#pragma GCC target "+nocssc"
> >> +
> >> +#define TEST(T, N) \
> >> + __attribute__ ((noipa)) T max_##N (T a) { return a > -1 ? a : -1; } \
> >> + __attribute__ ((noipa)) T min_##N (T a) { return a < -1 ? a : -1; }
> >> +
> >> +TEST (int, si)
> >> +TEST (long long, di)
> >> +
> >> +#define IMIN (-0x7fffffff - 1)
> >> +#define LMIN (-0x7fffffffffffffffLL - 1)
> >> +
> >> +static const int si[][3]
> >> + = { { 0, 0, -1 }, { 1, 1, -1 }, { -1, -1, -1 }, { -2, -1, -2 },
> >> + { 0x7fffffff, 0x7fffffff, -1 }, { IMIN, -1, IMIN } };
> >> +static const long long di[][3]
> >> + = { { 0, 0, -1 }, { 1, 1, -1 }, { -1, -1, -1 }, { -2, -1, -2 },
> >> + { 0x7fffffff, 0x7fffffff, -1 }, { IMIN, -1, IMIN },
> >> + { 0x80000000LL, 0x80000000LL, -1 },
> >> + { 0x7fffffffffffffffLL, 0x7fffffffffffffffLL, -1 }, { LMIN, -1,
> >> LMIN } };
> >> +
> >> +int
> >> +main (void)
> >> +{
> >> + for (unsigned i = 0; i < sizeof si / sizeof si[0]; i++)
> >> + if (max_si (si[i][0]) != si[i][1] || min_si (si[i][0]) != si[i][2])
> >> + __builtin_abort ();
> >> + for (unsigned i = 0; i < sizeof di / sizeof di[0]; i++)
> >> + if (max_di (di[i][0]) != di[i][1] || min_di (di[i][0]) != di[i][2])
> >> + __builtin_abort ();
> >> + return 0;
> >> +}
> >
>
