On Sat, Feb 14, 2026 at 12:28 PM Andrew Pinski
<[email protected]> wrote:
>
> On Sat, Feb 14, 2026 at 3:48 AM Daniel Barboza
> <[email protected]> wrote:
> >
> > Add a new helper that handles mispredicts in the following bit ops
> > scenarios:
> >
> > - checking if a bitmask is not set, and in this case set it: always set
> > the bitmask;
> > - checking if a bitmask is set, and in this case clear it: always clear
> > the bitmask.
> >
> > Bootstrapped and tested with x86_64-pc-linux-gnu.
>
> This is NOT a full review, just something which caught my eye.
>
>
> >
> > PR tree-optimization/64567
> >
> > gcc/ChangeLog:
> >
> > * tree-ssa-phiopt.cc (block_has_single_assignment): simple
> > helper that verifies in a block has a single statement.
> > (cond_removal_mispredict_bitop): helper that verifies if we have
> > a bitmask check that leads to the same bitmask being
> > set/cleared, and make the set/clear unconditional.
> > (pass_phiopt::execute): use the new helper.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.dg/tree-ssa/PR64567.c: New test.
> > ---
> > gcc/testsuite/gcc.dg/tree-ssa/PR64567.c | 23 ++++
> > gcc/tree-ssa-phiopt.cc | 170 ++++++++++++++++++++++++
> > 2 files changed, 193 insertions(+)
> > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/PR64567.c
> >
> > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/PR64567.c
> > b/gcc/testsuite/gcc.dg/tree-ssa/PR64567.c
> > new file mode 100644
> > index 00000000000..09e8cee62ee
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/tree-ssa/PR64567.c
> > @@ -0,0 +1,23 @@
> > +/* { dg-additional-options -O2 } */
> > +/* { dg-additional-options -fdump-tree-phiopt } */
> > +
> > +#define F1 0x04
> > +#define F2 0x08
> > +
> > +int bar(unsigned flags);
> > +
> > +int foo(unsigned flags)
> > +{
> > + if (flags & (F1 | F2))
> > + flags &= ~(F1 | F2);
> > + return bar(flags);
> > +}
> > +
> > +int baz(unsigned flags)
> > +{
> > + if (!(flags & F1))
> > + flags |= F1;
> > + return bar(flags);
> > +}
> > +
> > +/* { dg-final { scan-tree-dump-times " PHI " 0 phiopt2 } } */
> > diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
> > index fcf44136d0a..ef1c9eb8b19 100644
> > --- a/gcc/tree-ssa-phiopt.cc
> > +++ b/gcc/tree-ssa-phiopt.cc
> > @@ -2703,6 +2703,172 @@ cond_removal_in_builtin_zero_pattern (basic_block
> > cond_bb,
> > return true;
> > }
> >
> > +/* Check if a BB has a single assignment or a single assignment
> > + and a GOTO. Return the gimple assignment or NULL if
> > + the BB does not match the criteria. */
> > +
> > +static gimple*
> > +block_has_single_assignment (basic_block bb)
> > +{
> > + gimple_stmt_iterator gsi = gsi_start_nondebug_after_labels_bb (bb);
>
> if (gsi_end_p (gsi))
> return NULL;
>
> > + gimple *stmt = gsi_stmt (gsi);
> > +
> > + if (!stmt || !is_gimple_assign (stmt))
> > + return NULL;
>
> And then you can remove the check for null stmt here.
>
> > +
> > + gsi = gsi_last_nondebug_bb (bb);
> > + gimple *last_stmt = gsi_stmt (gsi);
> > +
> > + if (!last_stmt)
> > + return NULL;
> > +
> > + if (gimple_code (last_stmt) == GIMPLE_GOTO)
> > + {
> > + gsi_prev (&gsi);
> > + last_stmt = gsi_stmt (gsi);
> > + }
>
> GIMPLE_GOTO only shows up at this stage for computed gotos.
> So it should not matter here.
>
> Also maybe it is easier to reuse empty_bb_or_one_feeding_into_p.
> So something like:
> static gassign*
> block_has_single_assignment (basic_block bb, gimple phi)
> {
> gimple *stmt;
> if (!empty_bb_or_one_feeding_into_p (bb, phi, stmt))
> return null_ptr;
> return safe_dyn_cast<gassign*>(stmt);
> }
>
>
>
> > +
> > + if (last_stmt != stmt)
> > + return NULL;
> > +
> > + return stmt;
> > +}
> > +
> > +/* Optimize the case where we have a bitmask check and
> > + a bitmask set/clear of the same mask, making the
> > + set/clear unconditional. E.g for a bitmask set case:
> > +
> > + _1 = flags_3 & bitmask;
> > + if (_1 == 0)
> > + goto <bb 3>; [INV]
> > + else
> > + goto <bb 4>; [INV]
> > +
> > + ;; basic block 3, loop depth 0, maybe hot
> > + flags_4 = flags_3 | bitmask;
> > + ;; succ: 4 (FALLTHRU,EXECUTABLE)
> > +
> > + ;; basic block 4, loop depth 0, maybe hot
> > + # flags_2 = PHI <flags_3, flags_4>
>
>
> So we start out with:
> a = (a & bitmask) == 0 ? (a | bitmask) : a;
> And we want to convert that to:
> (a | bitmask)
> ?
>
> So isn't this just this match pattern:
> ```
> (for (neeq ne eq)
> (bitop bit_and bit_ior)
> (simplify
> (cond (neeq (bit_and @0 @1) integer_zerop)
> (bitop@2 @0 @1) @0)
> @2))
> ```
>
> Am I missing something here?
Well I did miss part of something here.
The above only works for bit_ior not bit_and.
For bit_and, you need
(simplify
(cond (ne (bit_and @0 INTEGER_CST@1) integer_zerop)
(bit_and@3 @0 INTEGER_CST@2) @0)
(if (...)
@3))
Thanks,
Andrew
>
>
> > +
> > + We'll make the gcond always true, always setting the
> > + bitmask. The gcond disappears, and the BIT_AND op
> > + used by it also goes away if it's not being used elsewhere.
> > +
> > + Likewise for the bitmask clear case. */
> > +
> > +static bool
> > +cond_removal_mispredict_bitop (basic_block cond_bb,
> > + basic_block middle_bb,
> > + tree arg0, tree arg1)
> > +{
> > + /* Check if the middle_bb has a single stmt or a
> > + single stmt + a goto. */
> > + gimple *mid_stmt = block_has_single_assignment (middle_bb);
> > + if (!mid_stmt)
> > + return false;
> > +
> > + /* mid_stmt constraints:
> > + - Must be either an IOR or an AND;
> > + - RHS1 is a SSA_NAME with integral type. */
> > + if (gimple_assign_rhs_class (mid_stmt) != GIMPLE_BINARY_RHS)
> > + return false;
> > +
> > + tree rhs1 = gimple_assign_rhs1 (mid_stmt);
> > + if (TREE_CODE (rhs1) != SSA_NAME
> > + || !INTEGRAL_TYPE_P (TREE_TYPE (rhs1)))
> > + return false;
> > +
> > + tree_code mid_code = gimple_assign_rhs_code (mid_stmt);
> > + if (mid_code != BIT_AND_EXPR && mid_code != BIT_IOR_EXPR)
> > + return false;
>
> Check mid_code instead of also checking GIMPLE_BINARY_RHS.
>
> > +
> > + if (arg0 != gimple_assign_lhs (mid_stmt)
> > + || arg1 != gimple_assign_rhs1 (mid_stmt))
> > + return false;
> > +
> > + /* 'cond' must be the format: SSA_NAME EQ|NE integer_zerop,
> > + where cond_code varies with mid_stmt OP:
> > +
> > + - SSA_NAME == 0 and mid_stmt = BIT_AND;
> > + - SSA_NAME != 0 and mid_stmt = BIT_IOR.
> > +
> > + We're assuming that SSA_NAME is a suitable BIT_AND
> > + for now. */
> > + gcond *cond = safe_dyn_cast <gcond *> (*gsi_last_bb (cond_bb));
>
> Shouldn't this always be a gcond here?
>
> > + if (!cond)
> > + return false;
> > +
> > + if (TREE_CODE (gimple_cond_lhs (cond)) != SSA_NAME
> > + || !integer_zerop (gimple_cond_rhs (cond)))
> > + return false;
> > +
> > + tree_code cond_code = gimple_cond_code (cond);
> > + if (cond_code != EQ_EXPR && cond_code != NE_EXPR)
> > + return false;
> > +
> > + if ((cond_code == NE_EXPR && mid_code != BIT_AND_EXPR)
> > + || (cond_code == EQ_EXPR && mid_code != BIT_IOR_EXPR))
> > + return false;
> > +
> > + gimple *cond_stmt = SSA_NAME_DEF_STMT (gimple_cond_lhs (cond));
> > + if (!cond_stmt || !is_gimple_assign (cond_stmt)
> > + || gimple_assign_rhs_class (cond_stmt) != GIMPLE_BINARY_RHS
> > + || gimple_assign_rhs_code (cond_stmt) != BIT_AND_EXPR)
> > + return false;
>
> You don't need to check both rhs_class and rhs_code here. class is
> based on the code.
>
> > +
> > + /* RHS1 for both cond_stmt and mid_stmt must be the same.
> > + RHS2 will depend of what we're trying to map. For a
> > + "check if set, if not set it":
> > +
> > + _1 = SSA_NAME & imm
> > + if (_1 == 0) goto 3 else goto 4
> > + 3: _4 = SSA_NAME | imm
> > + (fallthrough to 4)
> > +
> > + RHS2 must be the same for both. However for a "if set, clear it"
> > + case:
> > +
> > + _1 = SSA_NAME & imm
> > + if (_1 != 0) goto 3 else goto 4
> > + 3: _4 = SSA_NAME & (mask ~imm)
> > + (fallthrough to 4)
> > +
> > + mid_stmt RHS2 must clear 'imm', meaning that it must be a
> > + mask format. Both RHS2 must also be INTEGER_CST. */
> > + if (gimple_assign_rhs1 (cond_stmt) != gimple_assign_rhs1 (mid_stmt))
> > + return false;
> > +
> > + if (mid_code == BIT_IOR_EXPR
> > + && gimple_assign_rhs2 (cond_stmt) != gimple_assign_rhs2 (mid_stmt))
> > + return false;
> > + else if (mid_code == BIT_AND_EXPR)
> > + {
> > + tree cond_stmt_imm = gimple_assign_rhs2 (cond_stmt);
> > + tree mid_stmt_imm = gimple_assign_rhs2 (mid_stmt);
> > +
> > + if (TREE_CODE (cond_stmt_imm) != INTEGER_CST
> > + || TREE_CODE (mid_stmt_imm) != INTEGER_CST)
> > + return false;
> > +
> > + wide_int cond_imm = wi::to_wide (cond_stmt_imm);
> > + wide_int mid_imm = wi::to_wide (mid_stmt_imm);
> > +
> > + if (!wi::ltu_p (cond_imm, mid_imm)
> > + || wi::ne_p (mid_imm, wi::bit_not (cond_imm)))
> > + return false;
> > + }
> > +
> > + /* Finally set 'cond' to always execute mid_stmt. */
> > + edge e = single_pred_edge (middle_bb);
> > + if (e->flags & EDGE_TRUE_VALUE)
> > + gimple_cond_make_true (cond);
> > + else
> > + gimple_cond_make_false (cond);
>
> There is a better way of doing this here.
> If you move the statement from the middle_bb to right before the conditional,
> you could just use replace_phi_edge_with_variable which will do the
> right thing.
>
>
>
>
> Thanks,
> Andrew Pinski
>
> > +
> > + return true;
> > +}
> > +
> > /* Auxiliary functions to determine the set of memory accesses which
> > can't trap because they are preceded by accesses to the same memory
> > portion. We do that for MEM_REFs, so we only need to track
> > @@ -4073,6 +4239,10 @@ pass_phiopt::execute (function *)
> > && !diamond_p
> > && spaceship_replacement (bb, bb1, e1, e2, phi, arg0, arg1))
> > cfgchanged = true;
> > + else if (single_pred_p (bb1)
> > + && !diamond_p
> > + && cond_removal_mispredict_bitop (bb, bb1, arg0, arg1))
> > + cfgchanged = true;
> > };
> >
> > execute_over_cond_phis (phiopt_exec);
> > --
> > 2.51.1
> >
