On Thu, Jul 9, 2026 at 10:11 AM Daniel Henrique Barboza
<[email protected]> wrote:
>
>
>
> On 7/9/2026 1:29 PM, Daniel Henrique Barboza wrote:
> > Hello!
> >
> > On 7/9/2026 5:32 AM, Andrea Pinski wrote:
> >> On Wed, Jul 8, 2026 at 4:35 AM Daniel Barboza
> >> <[email protected]> wrote:
> >>>
> >>> Add patterns to handle the case where we're checking if a bit is set and
> >>> setting it in case it's not. Similar to the work done in PR64567 but in
> >>> this case the bit is a result of a 1 << val lshift:
> >>>
> >>> (A & BIT) EQ 0 ? A | BIT : A , BIT = 1 << val => A | BIT
> >>> (A & BIT) NE 0 ? A : A | BIT , BIT = 1 << val => A | BIT
> >>>
> >>> We're also adding cases where a full bitmask is tested, i.e.:
> >>>
> >>> (A & MASK) EQ MASK ? A : A | MASK => A | MASK
> >>> (A & MASK) NE MASK ? A | MASK : A => A | MASK
> >>>
> >>> Note that for these simplifications we're not limited to a bit/pow2
> >>> value like the zero comparisons, which don't work with multiple bits
> >>> because there's no guarantees to preserve the 'non-zero' cases. E.g.:
> >>> "(A & 0xF) == 0 ? A | 0xF : A" can't be simplified to just "A | 0xF"
> >>> because there's a whole range of A lower bits (1,2...E) that would be
> >>> turned to 0xF in the simplification - bits that would be preserved in
> >>> the original pattern. This is the same scenario discussed before in
> >>> PR64567.
> >>>
> >>> Bootstrapped and regression tested in x86_64, aarch64 and riscv64.
> >>>
> >>> PR tree-optimization/124667
> >>>
> >>> gcc/ChangeLog:
> >>>
> >>> * match.pd(`(A & BIT) EQ 0 ? A | BIT : A`): New
> >>> pattern.
> >>> (`(A & BIT) NE 0 ? A : A | BIT`): New pattern.
> >>> (`(A & MASK) EQ MASK ? A : A | MASK`): New pattern.
> >>> (`(A & MASK) NE MASK ? A | MASK : A`): New pattern.
> >>>
> >>> gcc/testsuite/ChangeLog:
> >>>
> >>> * gcc.dg/tree-ssa/pr124667.c: New test.
> >>> ---
> >>>
> >>> Hi,
> >>>
> >>> This is a follow-up of:
> >>>
> >>> [PATCH v3] tree-ssa-phiopt.cc: add cond_removal_mispredict_memop
> >>> [PR124667]
> >>>
> >>> But now we're using match.pd instead of using cselim. There's a
> >>> bitclear case that we're not covering here since match.pd isn't
> >>> able (yet) to deal with more than one stmt in the middle_bb from
> >>> phiopt. As soon as we get to that point I'll either re-send this
> >>> patch or, in case this is already merged, send a follow-up.
> >>>
> >>>
> >>>
> >>> gcc/match.pd | 31 +++++++
> >>> gcc/testsuite/gcc.dg/tree-ssa/pr124667.c | 107 +++++++++++++++++++++++
> >>> 2 files changed, 138 insertions(+)
> >>> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr124667.c
> >>>
> >>> diff --git a/gcc/match.pd b/gcc/match.pd
> >>> index a7cec25dbad..a6580926ee3 100644
> >>> --- a/gcc/match.pd
> >>> +++ b/gcc/match.pd
> >>> @@ -6102,6 +6102,37 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >>> (if (wi::to_wide (@2) == ~wi::to_wide (@1))
> >>> @3))
> >>>
> >>> +/* PR124667: (A & (1 << val)) == 0 ? A |= (1 << val) : A
> >>> + Similar to PR64567 above but with the bit given
> >>> + by an (1 << val) lshift. Note that this shift pattern
> >>> + guarantees that this is a single bit operation. */
> >>> +(simplify
> >>> + (cond
> >>> + (eq (bit_and:c @0 (lshift@1 integer_onep @2)) integer_zerop)
> >>> + (bit_ior:c@3 @0 @1) @0)
> >>> + @3)
> >>> +
> >>> +/* PR124667: (A & (1 << val)) != 0 ? A : A |= (1 << val)
> >>> + NE variant of the previous match. */
> >>> +(simplify
> >>> + (cond
> >>> + (ne (bit_and:c @0 (lshift@1 integer_onep @2)) integer_zerop)
> >>> + @0 (bit_ior:c@3 @0 @1))
> >>> + @3)
> >>
> >> So it looks like you might need to recognize:
> >> ((a >> val) & 1) == 0 ? (A | (1 << val)) : A
> >>
> >> Which at least will be caught at the generic level.
> >
> > This pattern inserts an extra stmt in middle_bb (the 1 << val) and won't
> > work
> > at this moment. I checked it with your patch in https://gcc.gnu.org/PR25290
> > and it does work with that applied.
> >
> > I'll carry on without it for now but seems like you're almost done with that
> > patch. Maybe it's a good idea to hold this v2 while you push that upstream
> > and
> > then we can add this extra pattern too.
> >
> >>
> >> Can you also handle:
> >> (A & (1 << val)) == 0 ? A: (A| (1 << val) )
> >> (A & (1 << val)) != 0 ?(A| (1 << val) ) : A
> >>
> >> These are both just A.
> >> And the `((a >> val) & 1) == 0`/`((a >> val) & 1) != 0` versions where
> >> the result is just A.
> >
> > A quick note on these: depending on how I write the code I'll have an extra
> > stmt in middle_bb:
> >
> > int bitset21 (int n, int bit)
> > {
> > int arr[16];
> > int bitshift = 1 << bit;
> >
> > if (((arr[n] >> bit) & 1) != 0)
> > arr[n] |= bitshift;
> >
> > return arr[n];
> > }
> >
> > The above generates:
> >
> > -----
> > _3 = _2 & 1;
> > if (_3 != 0)
> > goto <bb 3>; [50.00%]
> > else
> > goto <bb 4>; [50.00%]
> >
> > ;; basic block 3
> > bitshift_7 = 1 << bit_6(D);
> > _4 = _1 | bitshift_7;
> >
> > ;; basic block 4
> > # cstore_14 = PHI <_1(2), _4(3)>
> > # .MEM_12 = VDEF <.MEM_8(D)>
> > -----
> >
> > But this one:
> >
> > int bitset21_alt (int n, int bit)
> > {
> > int arr[16];
> > int bitshift = 1 << bit;
> >
> > arr[n] = ((arr[n] >> bit) & 1) != 0 ? arr[n] | bitshift : arr[n];
> >
> > return arr[n];
> > }
> >
> > Generates:
> >
> > ;; basic block 2, loop depth 0, count 1073741824 (estimated locally, freq
> > 1.0000), maybe hot
> > ;; prev block 0, next block 1, flags: (NEW, REACHABLE, VISITED)
> > ;; pred: ENTRY [always] count:1073741824 (estimated locally, freq
> > 1.0000) (FALLTHRU,EXECUTABLE)
> > # VUSE <.MEM_4(D)>
> > _5 = arrD.4695[n_1(D)];
> > _6 = _5 >> bit_2(D);
> > # RANGE [irange] int [0, 1] MASK 0x1 VALUE 0x0
> > _7 = _6 & 1;
> > _8 = _7 << bit_2(D);
> > _9 = _5 | _8;
> > # .MEM_10 = VDEF <.MEM_4(D)>
> > arrD.4695 ={v} {CLOBBER(eos)};
> > # VUSE <.MEM_10>
> > return _9;
> >
> > The change starts way back in 'original' so I take this has to do with how
> > the
> > frontend generates code for the ternary.
> >
> > I'll carry on with them using the ternary testcase and, after your patch
> > lands,
> > we'll have the other form figured out too.
>
> That's not the whole story, is it? The pattern to match this ternary
> structure isn't
> the same "cond" base pattern for that other case ...
>
> I guess I'll hold these patterns with "(val >> bit) & 1" cmps until we have
> your change
> upstream. All other patterns you suggested are good to go as is.
As I mentioned even though it does not currently match for gimple; it
should match the generic trees (from the front-end).
So you should be able to look at the .original dump to see the change
rather than looking into phiopt.
Thanks,
Andrea
>
>
> Thanks,
> Daniel
>
> >
> >
> > Thanks,
> > Daniel
> >
> >>
> >>
> >>> +
> >>> +/* PR124667: a bitmask set simplification that is possible as
> >>> + long as the whole bitmask is being checked:
> >>> + (A & B) == B ? A : A |= B => A | B
> >>> + (A & B) != B ? A |= B : A => A | B. */
> >>> +(simplify
> >>> + (cond (eq (bit_and:c @0 @1) @1)
> >>> + @0 (bit_ior:c@2 @0 @1))
> >>> + @2)
> >>> +(simplify
> >>> + (cond (ne (bit_and:c @0 @1) @1)
> >>> + (bit_ior:c@2 @0 @1) @0)
> >>> + @2)
> >>
> >> These 2 patterns don't need `:c` on the bit_and/bit_ior as the order
> >> there is stable. But should have them on the eq/ne.
> >> Can you handle:
> >> (A & B) == B ? (A|B) : A
> >> (A & B) != B ? A : (A|B)
> >>
> >> too? They are both just A.
> >>
> >> Thanks,
> >> Andrea
> >>
> >>> +
> >>> #if GIMPLE
> >>> (match (nop_atomic_bit_test_and_p @0 @1 @4)
> >>> (bit_and (convert?@4 (ATOMIC_FETCH_OR_XOR_N @2 INTEGER_CST@0 @3))
> >>> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr124667.c
> >>> b/gcc/testsuite/gcc.dg/tree-ssa/pr124667.c
> >>> new file mode 100644
> >>> index 00000000000..9bdbe05a00d
> >>> --- /dev/null
> >>> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr124667.c
> >>> @@ -0,0 +1,107 @@
> >>> +/* { dg-additional-options -O2 } */
> >>> +/* { dg-additional-options -fdump-tree-optimized } */
> >>> +
> >>> +int bitset1 (int n, int bit)
> >>> +{
> >>> + int arr[16];
> >>> + int bitshift = 1 << bit;
> >>> +
> >>> + if ((arr[n] & bitshift) == 0)
> >>> + arr[n] |= bitshift;
> >>> +
> >>> + return arr[n];
> >>> +}
> >>> +
> >>> +int bitset1_alt (int n, int bit)
> >>> +{
> >>> + int arr[16];
> >>> + int bitshift = 1 << bit;
> >>> +
> >>> + arr[n] = (arr[n] & bitshift) != 0 ? arr[n] : arr[n] | bitshift;
> >>> +
> >>> + return arr[n];
> >>> +}
> >>> +
> >>> +int bitset1b (int n, int bit)
> >>> +{
> >>> + int arr[16];
> >>> + int bitshift = 1 << bit;
> >>> +
> >>> + if ((bitshift & arr[n]) == 0)
> >>> + arr[n] |= bitshift;
> >>> +
> >>> + return arr[n];
> >>> +}
> >>> +
> >>> +int bitset1b_alt (int n, int bit)
> >>> +{
> >>> + int arr[16];
> >>> + int bitshift = 1 << bit;
> >>> +
> >>> + arr[n] = (bitshift & arr[n]) != 0 ? arr[n] : bitshift | arr[n];
> >>> +
> >>> + return arr[n];
> >>> +}
> >>> +
> >>> +int bitset2 (int n)
> >>> +{
> >>> + int arr[16];
> >>> + int bits = 0xF;
> >>> +
> >>> + if ((arr[n] & bits) != bits)
> >>> + arr[n] |= bits;
> >>> +
> >>> + return arr[n];
> >>> +}
> >>> +
> >>> +int bitset2_alt (int n)
> >>> +{
> >>> + int arr[16];
> >>> + int bits = 0xF;
> >>> +
> >>> + arr[n] = (arr[n] & bits) == bits ? arr[n]: arr[n] | bits;
> >>> +
> >>> + return arr[n];
> >>> +}
> >>> +
> >>> +int bitset2b (int n)
> >>> +{
> >>> + int arr[16];
> >>> + int bits = 0xF;
> >>> +
> >>> + if ((bits & arr[n]) != bits)
> >>> + arr[n] |= bits;
> >>> +
> >>> + return arr[n];
> >>> +}
> >>> +
> >>> +int bitset2b_alt (int n)
> >>> +{
> >>> + int arr[16];
> >>> + int bits = 0xF;
> >>> +
> >>> + arr[n] = (bits & arr[n]) == bits ? arr[n]: bits | arr[n];
> >>> +
> >>> + return arr[n];
> >>> +}
> >>> +
> >>> +/* A negative test to ensure we're not optimizing something
> >>> + we shouldn't. */
> >>> +int bitset_nosimplify (int n)
> >>> +{
> >>> + int arr[16];
> >>> +
> >>> + int bits = 0xF;
> >>> +
> >>> + /* We can't make the bit_ior unconditional here because the
> >>> + cond matches for multiple values aside from '0xF' in
> >>> + which the value wouldn't be touched, e.g. for arr[n] = 0x1
> >>> + "arr[n] & bits" is also non-zero and the value wouldn't be
> >>> + changed. */
> >>> + if ((arr[n] & bits) == 0)
> >>> + arr[n] |= bits;
> >>> +
> >>> + return arr[n];
> >>> +}
> >>> +
> >>> +/* { dg-final { scan-tree-dump-times "goto" 2 optimized } } */
> >>> --
> >>> 2.43.0
> >>>
> >
>
