Hi Richard,
Thanks for the feedback.  I agree that creating the helper function 
comparison_all_ones_result_p might be confusing, especially as
there are some cases (floating point results) that aren't currently
handled.  This revised patch implements your recommendation to
inline the relevant VECTOR_STORE_FLAG_VALUE functionality into
simplify_unary_operation_1 without creating the new function.

Additionally, to better support aarch64, I've also added the
related optimization "(not (neg (eq X Y))) -> (neg (ne X Y))" for
STORE_FLAG_VALUE/VECTOR_STORE_FLAG_VALUE == 1 targets.

I'll look into a store_flag_value(mode) as a follow-up patch, perhaps
exporting relational_result but that doesn't solve the is_all_ones_p
bit, and performs unnecessary memory allocations.

This revision has been tested on x86_64-pc-linux-gnu with make bootstrap
and make -k check, both with and without --target_board=unix{-m32}
with no new failures.  The i386 pieces have been approved (thanks).
Are the simplify-rtx.cc changes Ok for mainline?


2026-06-27  Roger Sayle  <[email protected]>

gcc/ChangeLog
        PR target/94871
        * config/i386/sse.md (*<sse>_imaskcmp<mode>3_comm): Clone
        of *<sse>_maskcmp<mode>3_comm pattern with integer vector
        result.

        * simplify-rtx.cc (simplify_unary_operation_1) <case NOT>:
        Update (not (eq X Y)) -> (ne X Y) to support vector comparions
        using VECTOR_STORE_FLAG_VALUE, and check SCALAR_INT_MODE_P
        when checking STORE_FLAG_VALUE.  Add equivalent optimization
        (not (neg (eq X Y))) -> (neg (ne X Y)) for targets with
        STORE_FLAG_VALUE == 1 or VECTOR_STORE_FLAG_VALUE is const1_rtx.

gcc/testsuite/ChangeLog
        PR target/94871
        * gcc.target/i386/pr94871.c: New test case.


Thanks in advance,
Roger
--

> -----Original Message-----
> From: Richard Biener <[email protected]>
> Sent: 24 June 2026 12:43
> To: Roger Sayle <[email protected]>
> Cc: GCC Patches <[email protected]>; Jeffrey Law
> <[email protected]>; Uros Bizjak <[email protected]>; Liu,
> Hongtao <[email protected]>; Hongtao Liu <[email protected]>
> Subject: Re: [PATCH] PR target/94871: Simplify (not (eq x y)) as (ne x y) for
> vectors.
> 
> On Wed, Jun 24, 2026 at 1:30 PM Roger Sayle <[email protected]>
> wrote:
> >
> >
> > Hi Richard,
> >
> > > -----Original Message-----
> > > From: Richard Biener <[email protected]>
> > > Sent: 24 June 2026 11:56
> > > To: Roger Sayle <[email protected]>
> > > Cc: GCC Patches <[email protected]>; Jeffrey Law
> > > <[email protected]>; Uros Bizjak <[email protected]>;
> > > Liu, Hongtao <[email protected]>; Hongtao Liu
> > > <[email protected]>
> > > Subject: Re: [PATCH] PR target/94871: Simplify (not (eq x y)) as (ne
> > > x y) for vectors.
> > >
> > > On Tue, Jun 23, 2026 at 8:42 PM Roger Sayle
> > > <[email protected]>
> > > wrote:
> > > >
> > > > This patch is my proposed fix for PR target/94871, a missed
> > > > optimization for simplifying vector comparisons in the RTL
> > > > optimizers
> > > > (combine) that includes both middle-end and i386 backend changes.
> > > >
> > > > The testcase from the original bugzilla PR is:
> > > >
> > > > typedef long long int64_t;
> > > > typedef signed char int8_t;
> > > >
> > > > typedef double v2df __attribute__((vector_size(16))); typedef
> > > > int64_t v2di __attribute__((vector_size(16))); typedef int8_t
> > > > v16qi __attribute__((vector_size(16)));
> > > >
> > > > inline v2di set1_epi8(int8_t a)
> > > > {
> > > >     return (v2di)(v16qi){a, a, a, a, a, a, a, a, a, a, a, a, a, a,
> > > > a, a}; }
> > > >
> > > > v2di foo(v2df a, v2df b)
> > > > {
> > > >     return ((v2di)__builtin_ia32_cmpeqpd(a, b) ^ set1_epi8(0xFF));
> > > > }
> > > >
> > > > where GCC with -O2 currently generates:
> > > >
> > > > foo:    cmpeqpd %xmm1, %xmm0
> > > >         pcmpeqd %xmm1, %xmm1
> > > >         pxor    %xmm1, %xmm0
> > > >         ret
> > > >
> > > > where with this pair of changes, we instead generate:
> > > >
> > > > foo:    cmpneqpd        %xmm1, %xmm0
> > > >         ret
> > > >
> > > > Basically, the usual optimizations (not (eq x y)) -> (ne x y) and
> > > > (not (ne x y)) -> (eq x y), but applied to vector mode comparisons
> > > > (and in this case floating point vector comparisons).
> > > >
> > > >
> > > > The first (i386 backend) part is to observe that the middle-end
> > > > RTL optimizers enjoy canonicalizing floating point vector
> > > > comparisons to return integer vector results.  Hence
> > > > -fdump-rtl-combine-all contains the lines:
> > > >
> > > > Failed to match this instruction:
> > > > (set (reg:V2DI 105)
> > > >     (eq:V2DI (reg:V2DF 106 [ aD.2972 ])
> > > >         (reg:V2DF 107 [ bD.2973 ])))
> > > >
> > > > Currently sse.md contains patterns for matching vector comparisons
> > > > for comparing V2DFs returning a V2DF, but not the equivalent
> > > > patterns for comparing V2DFs but returning a V2DI.  Very easily
> > > > handled by cloning the existing define_insn but substituting
> > > > <sseintvecmode> for the comparison result mode.
> > > >
> > > > The second (simplify-rtx.cc) part is to tweak the current (not (eq
> > > > X
> > > > Y)) -> (ne X Y) transformation in
> > > > simplify_unary_operation_1 to allow/handle vector modes.
> > > > In fact, this is a (latent) bug fix, as the current logic checks
> > > > STORE_FLAG_VALUE even for modes where this isn't relevant.  The
> > > > target's STORE_FLAG_VALUE is applicable for SCALAR_INT_MODE_P
> > > > comparisons; VECTOR_STORE_FLAG_VALUE should be used for VECTOR
> > > > modes, and FLOAT_STORE_FLAG_VALUE for comparisons that yield
> > > > floating point results.  In this patch I've split the logic into a
> > > > helper function, comparison_all_ones_result_p to make things clearer.
> > > >
> > > > I notice aarch64 returns a vector of 1s, not -1s, for vector mode
> > > > comparisons, so in theory similar RTL simplifications should be
> > > > possible there for (vx == vy) ^ 1 -> vx != vy, and likewise
> > > > 1 - (vx == yy) -> vx != vy.
> > > >
> > > >
> > > > This patch has been tested on x86_64-pc-linux-gnu with make
> > > > bootstrap and make -k check, both with and without
> > > > --target_board=unix{-m32} with no new failures.  Ok for mainline
> > > > (from middle-end and i386 backend maintainers)?
> > >
> > > +/* Test whether the result of a comparison in mode MODE is all ones.
> > > +*/
> > >
> > > of a elementwise comparison
> >
> > Thanks.
> >
> > > +static bool
> > > +comparison_all_ones_result_p (machine_mode mode) {
> > > +  if (mode == BImode)
> > > +    return true;
> > > +  if (SCALAR_INT_MODE_P (mode) && STORE_FLAG_VALUE == -1)
> > > +    return true;
> > > +#ifdef VECTOR_STORE_FLAG_VALUE
> > > +  if (GET_MODE_CLASS (mode) == MODE_VECTOR_INT
> > >
> > > why the check for MODE_VECTOR_INT?
> > >
> > > +      && VECTOR_STORE_FLAG_VALUE (mode) == constm1_rtx)
> > > +    return true;
> > > +#else
> > > +  if (GET_MODE_CLASS (mode) == MODE_VECTOR_INT)
> > > +    return true;
> > > +#endif
> > > +  return false;
> > > +}
> > >
> > > VECTOR_STORE_FLAG_VALUE documents
> > > "If there are no such operations, do not define this macro." which
> > > to me implies it's required to define the macro, so the fallback
> > > handling for MODE_VECTOR_INT above looks wrong and we should drop it?
> > >
> > > The function does look incomplete (FLOAT_STORE_FLAG_VALUE), so I
> > > wonder if amending the mode == BImode || STORE_FLAG_VALUE == -1 with
> > > the VECTOR_STORE_FLAG_VALUE case would be simpler and less
> > > contentious?  IMO we'd want to have a store_flag_value (mode)
> > > function that wraps all the macros and works on all modes, not necessarily
> also checking for constm1_rtx.
> >
> > This transformation is not safe for floating point, and floating point 
> > vector
> modes.
> > Consider nvptx, which has a
> > #define FLOAT_STORE_FLAG_VALUE(MODE) REAL_VALUE_ATOF("1.0", (MODE))
> >
> > So the result of comparison has the hex value, 0x3f8000000, and the
> > NOT of this value isn’t 0x00.  In theory, we could check for a
> > particular form of negative NaN, but it's much safer to limit this
> > transformation to sane targets that return integer 0/-1 masks from (floating
> point) comparisons.
> >
> > PRs 98251 and 112614 show that handling on -NaN is poorly defined by
> > IEEE, a backend that relies on -NaN as the result of a floating point
> > comparison is probably much better represented as a comparison with an
> > integer result.  I believe the only reason i386 has historically used 
> > V?[SD]F
> results is to match the Intel instrinsics APIs.
> >
> > Likewise, as mentioned above: aarch64 has:
> > #define VECTOR_STORE_FLAG_VALUE(MODE) CONST1_RTX
> (GET_MODE_INNER
> > (MODE))
> >
> > So the test for constm1_rtx is required.
> 
> Yes, I'm questioning general usefulness of an (incomplete)
> comparison_all_ones_result_p but I think a store_flag_value (mode) wrapper
> around the different macros would help cleanup code (and could be used here).
> 
> >  Your assumption and GCC's default behaviour that comparisons return
> > 0/-1 masks, is (I believe) the behaviour expected when
> > VECTOR_STORE_FLAG_VALUE hasn't been defined.
> 
> It's not my assumption - my assumption is that no default behavior is
> documented, so we shouldn't assume any?
> 
> That said, if you do not want to go down the store_flag_value (mode) route 
> then
> can you please inline the relevant VECTOR_STORE_FLAG_VALUE case to
> simplify_unary_operation_1 without creating the new function?
> 
> Thanks,
> Richard.
> 
> >
> > > Thanks,
> > > Richard.
> > >
> > > >
> > > > 2026-06-23  Roger Sayle  <[email protected]>
> > > >
> > > > gcc/ChangeLog
> > > >         PR target/94871
> > > >         * config/i386/sse.md (*<sse>_imaskcmp<mode>3_comm): Clone
> > > >         of *<sse>_maskcmp<mode>3_comm pattern with integer vector
> > > >         result.
> > > >
> > > >         * simplify-rtx.cc (comparison_all_ones_result_p): New
> > > >         helper function to check that the true result from a
> > > >         comparison with result mode MODE is a mask of all ones.
> > > >         (simplify_unary_operation_1): Use it when reversing
> > > >         comparisons, i.e. (not (eq X Y)) -> (ne X Y).
> > > >
> > > > gcc/testsuite/ChangeLog
> > > >         PR target/94871
> > > >         * gcc.target/i386/pr94871.c: New test case.
> > > >
> > > >
> > > > Thanks in advance (to you both),
> > > > Roger
> > > > --
> >
> > Cheers,
> > Roger
> >
> >

Reply via email to