On 2/14/2026 5:35 PM, Andrew Pinski wrote:
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))
Hm, I tried to do this transform with match.pd but my match was off the
mark, matching way more stuff than I intended:
+(simplify
+ (cond (eq (bit_and @0 @1) integer_zerop) (bit_ior @0 @1) @2)
+ (cond { boolean_true_node; } (bit_ior @0 @1) @2))
+
+(simplify
+ (cond (ne (bit_and @0 INTEGER_CST@1) integer_zerop)
+ (bit_and @0 INTEGER_CST@2) @3)
+ (with
+ {
+ wide_int cond_imm = wi::to_wide (@1);
+ wide_int mid_imm = wi::to_wide (@2);
+ }
+ (if (wi::ltu_p (cond_imm, mid_imm)
+ && wi::eq_p (mid_imm, wi::bit_not (cond_imm)))
+ (cond { boolean_true_node; } (bit_and @0 @2) @3))))
It didn't occur to me that I also needed to match the 'else' leg of the
gcond. Makes since, given that I'm also checking the PHI results in phiopt.
I was also trying to set the gcond to 'true' instead of just returning
the result, emulating what I was doing manually in phiopt. Not sure why
I did that since it's just a flashy way of returning the 'true' leg ...
I'll make another try with match.pd in v2. Thanks,
Daniel
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
