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