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

Andrea Pinski <[email protected]>
Newsgroups gmane.comp.gcc.patches
Message-ID <CALvbMcCLkCTLaVRwogQyQ_iW7yfzFy6xBiu6NaHke_HOWy=mhg@mail.gmail.com>
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.

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
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.