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 > >
