On Sat, Jun 27, 2026 at 8:01 PM Roger Sayle <[email protected]> wrote: > > > Doh! Forgot to attach the patch. Sorry. > > > -----Original Message----- > > From: Roger Sayle <[email protected]> > > Sent: 27 June 2026 16:46 > > To: 'Richard Biener' <[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. > > > > > > 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?
The simplify-rtx.cc part is OK. Thanks, Richard. > > > > > > 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 > > > > > > > >
