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