Re: [PATCH v4] match.pd: turn more conditional bit_iors unconditional [PR124667]

On 8/8/2026 5:55 PM, Andrea Pinski wrote:
> On Tue, Jul 28, 2026 at 4:30 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
>>
>> Following Andrea's suggestion we're also handling the variants that
>> results in just 'A' if we switch the conditionals:
>>
>> (A & BIT) EQ 0 ? A | BIT : A , BIT = 1 << val => A | BIT
>> (A & BIT) NE 0 ? A | BIT : A , BIT = 1 << val => A
>>
>> (A & BIT) NE 0 ? A : A | BIT , BIT = 1 << val => A | BIT
>> (A & BIT) EQ 0 ? A : A | BIT , BIT = 1 << val => A
>>
>> 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 : A | MASK => A
>>
>> (A & MASK) NE MASK ? A | MASK : A => A | MASK
>> (A & MASK) EQ MASK ? A | MASK : A => A
>>
>> 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|NE 0 ? A | BIT : A`): New
>>          pattern.
>>          (`(A & BIT) EQ|NE 0 ? A : A | BIT`): New pattern.
>>          (`(A & MASK) EQ|NE MASK ? A : A | MASK`): New pattern.
>>          (`(A & MASK) EQ|NE MASK ? A | MASK : A`): New pattern.
>>          (`A | (((A >> N) & 1) << N)`): New pattern.
> 
> 
> Ok.
> 
>>
>> gcc/testsuite/ChangeLog:
>>
>>          * gcc.dg/tree-ssa/pr124667-2.c: New test.
>>          * gcc.dg/tree-ssa/pr124667.c: New test.
>> ---
>>
>> Changes from v3, both suggested by Andrea:
>> - Do a partial match instead of a full match to cover more cases when
>>    dealing with the rshift + lshift pattern;
>> - Rename all @A instances to @0;
>> - v3 link: https://gcc.gnu.org/pipermail/gcc-patches/2026-July/723884.html
>>
>>   gcc/match.pd                               | 100 +++++++++++++++++++
>>   gcc/testsuite/gcc.dg/tree-ssa/pr124667-2.c |  67 +++++++++++++
>>   gcc/testsuite/gcc.dg/tree-ssa/pr124667.c   | 107 +++++++++++++++++++++
>>   3 files changed, 274 insertions(+)
>>   create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr124667-2.c
>>   create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr124667.c
>>
>> diff --git a/gcc/match.pd b/gcc/match.pd
>> index 723780b8c38..1bb9ad9c7e8 100644
>> --- a/gcc/match.pd
>> +++ b/gcc/match.pd
>> @@ -6193,6 +6193,106 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>>     (if (wi::to_wide (@2) == ~wi::to_wide (@1))
>>      @3))
>>
>> +/* PR124667: 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.
>> +
>> +   The same pattern, if using '!=' instead of '==', will
>> +   result in just 'A'.
>> +
>> +   (A & (1 << val)) == 0 ? A |= (1 << val) : A => A |= (1 << val)
>> +   (A & (1 << val)) != 0 ? A |= (1 << val) : A => A;  */
>> +(for neeq (eq ne)
>> + (simplify
>> +  (cond
>> +   (neeq (bit_and:c @0 (lshift@1 integer_onep @2)) integer_zerop)
>> +        (bit_ior:c@3 @0 @1)
>> +        @0)
>> +    (if (neeq == EQ_EXPR)
>> +     @3
>> +     @0))
>> +
>> +/* PR124667: NE variant of the previous match with switched true/false
>> +   legs:
>> +
>> +   (A & (1 << val)) != 0 ? A : A |= (1 << val) => A |= (1 << val)
>> +   (A & (1 << val)) == 0 ? A : A |= (1 << val) => A;  */
>> + (simplify
>> +  (cond
>> +   (neeq (bit_and:c @0 (lshift@1 integer_onep @2)) integer_zerop)
>> +        @0
>> +        (bit_ior:c@3 @0 @1))
>> +    (if (neeq == NE_EXPR)
>> +     @3
>> +     @0))
>> +
>> +/* PR124667: a bitmask set simplification that is possible as
>> +   long as the whole bitmask is being checked.  Switching the
>> +   comparison from '==' to '!=' results in 'A':
>> +
>> +   (A & B) == B ? A : A |= B  => A |= B
>> +   (A & B) != B ? A : A |= B  => A;  */
>> + (simplify
>> +  (cond
>> +   (neeq:c (bit_and @0 @1) @1)
>> +          @0
>> +          (bit_ior@2 @0 @1))
>> +    (if (neeq == EQ_EXPR)
>> +     @2
>> +     @0))
>> +
>> +/* PR124667: similar from above with true/false legs switched:
>> +
>> +   (A & B) != B ? A |= B : A => A | B.
>> +   (A & B) == B ? A |= B : A => A;  */
>> +(simplify
>> + (cond
>> +  (neeq:c (bit_and @0 @1) @1)
>> +         (bit_ior@2 @0 @1)
>> +         @0)
>> +   (if (neeq == NE_EXPR)
>> +    @2
>> +    @0)))
>> +
>> +/* PR124667: these also reduces to just "A":
>> +   ((A >> val) & 1) == 0 ?  A : (A | (1 << val))
>> +   ((A >> val) & 1) != 0 ?  (A | (1 << val)) : A;
>> +
>> +   For both cases the zero_one patterns in lines 4860-4890
>> +   will turn both into the following 'original':
>> +
>> +   intD.7 bitshiftD.4489 = 1 << bitD.4485;
>> +   arrD.4488 = arrD.4488 >> bitD.4485 & 1) * bitshiftD.4489 | arrD.4488;
>> +
>> +   And it'll appear back in phiopt as:
>> +
>> +   _1 = arrD.4482;
>> +   _2 = _1 >> bit_7;
>> +   _3 = _2 & 1;
>> +   _4 = _3 << bit_7;
>> +   _6 = _1 | _4;
>> +
>> +   I.e. it'll drop the 'mult' and lshift the zero_one result from the
>> +   bit check.  The 3 middle stmts can be reduced to:
>> +
>> +   _3 = 1 << bit_7;
>> +   _4 = _1 & _3;
>> +
>> +   Resulting in:
>> +
>> +   _1 = arrD.4482;
>> +   _3 = 1 << bit_7;
>> +   _4 = _1 & _3
>> +   _6 = _1 | _4;
>> +
>> +   Which will reduce to just _1.  Simplifying the middle stmts has the
>> +   advantage of also benefitting other optimizations as well.  */
>> +(simplify
>> +  (lshift
>> +    (bit_and:c (rshift @0 @1) integer_onep)
>> +     @1)
>> +  (bit_and @0 (lshift { build_one_cst (type); } @1)))
> 
> We could do slightly better here of a generic non 1 simplification but
> I think 1 is the most common case.

Oh, I believe you're correct.  The "1" here can be generalized to
any INTEGER_CST.

I'll make the change and see if something bad happens during bootstrap.  In
case we're good I'll send a v5.


Thanks,
Daniel

> 
> 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-2.c b/gcc/testsuite/gcc.dg/tree-ssa/pr124667-2.c
>> new file mode 100644
>> index 00000000000..aa7ac22e07d
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr124667-2.c
>> @@ -0,0 +1,67 @@
>> +/* { dg-additional-options -O2 } */
>> +/* { dg-additional-options -fdump-tree-optimized } */
>> +
>> +int test1 (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 test1_alt (int n, int bit)
>> +{
>> +  int arr[16];
>> +  int bitshift = 1 << bit;
>> +
>> +  arr[n] = (arr[n] & bitshift) != 0 ? arr[n] | bitshift : arr[n];
>> +
>> +  return arr[n];
>> +}
>> +
>> +int test2 (int n, int bit)
>> +{
>> +  int arr[16];
>> +  int bitshift = 1 << bit;
>> +
>> +  arr[n] = ((arr[n] >> bit) & 1) == 0 ? arr[n] : arr[n] | bitshift;
>> +
>> +  return arr[n];
>> +}
>> +
>> +int test2_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];
>> +}
>> +
>> +int test3 (int n)
>> +{
>> +  int arr[16];
>> +  int bits = 0xF;
>> +
>> +  arr[n] = (arr[n] & bits) == bits ? arr[n] | bits : arr[n];
>> +
>> +  return arr[n];
>> +}
>> +
>> +int test3_alt (int n)
>> +{
>> +  int arr[16];
>> +  int bits = 0xF;
>> +
>> +  arr[n] = (arr[n] & bits) != bits ? arr[n] : arr[n] | bits;
>> +
>> +  return arr[n];
>> +}
>> +
>> +/* { dg-final { scan-tree-dump-times " \\| " 0 optimized } } */
>> +/* { dg-final { scan-tree-dump-times " \& " 0 optimized } } */
>> +/* { dg-final { scan-tree-dump-times " << " 0 optimized } } */
>> +/* { dg-final { scan-tree-dump-times " >> " 0 optimized } } */
>> 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
>>