https://gcc.gnu.org/g:999aad44aa86e64580d36e74bcc90b48f768cf55
commit r15-5852-g999aad44aa86e64580d36e74bcc90b48f768cf55
Author: Jovan Vukic <jovan.vu...@rt-rk.com>
Date: Sun Dec 1 11:57:41 2024 -0700
Thanks for the feedback on the first version of the patch. Accordingly:
I have corrected the code formatting as requested. I added new tests to
the existing file phi-opt-11.c, instead of creating a new one.
I performed testing before and after applying the patch on the x86
architecture, and I confirm that there are no new regressions.
The logic and general code of the patch itself have not been changed.
> So the A EQ/NE B expression, we can reverse A and B in the expression
> and still get the same result. But don't we have to be more careful for
> the TRUE/FALSE arms of the ternary? For BIT_AND we need ? a : b for
> BIT_IOR we need ? b : a.
>
> I don't see that gets verified in the existing code or after your
> change. I suspect I'm just missing something here. Can you clarify how
> we verify that BIT_AND gets ? a : b for the true/false arms and that
> BIT_IOR gets ? b : a for the true/false arms?
I did not communicate this clearly last time, but the existing optimization
simplifies the expression "(cond & (a == b)) ? a : b" to the simpler "b".
Similarly, the expression "(cond & (a == b)) ? b : a" simplifies to "a".
Thus, the existing and my optimization perform the following
simplifications:
(cond & (a == b)) ? a : b -> b
(cond & (a == b)) ? b : a -> a
(cond | (a != b)) ? a : b -> a
(cond | (a != b)) ? b : a -> b
For this reason, for BIT_AND_EXPR when we have A EQ B, it is sufficient to
confirm that one operand matches the true/false arm and the other matches
the false/true arm. In both cases, we simplify the expression to the third
operand of the ternary operation (i.e., OP0 ? OP1 : OP2 simplifies to OP2).
This is achieved in the value_replacement function after successfully
setting the value of *code within the rhs_is_fed_for_value_replacement
function to EQ_EXPR.
For BIT_IOR_EXPR, the same check is performed for A NE B, except now
*code remains NE_EXPR, and then value_replacement returns the second
operand (i.e., OP0 ? OP1 : OP2 simplifies to OP1).
2024-10-30 Jovan Vukic <jovan.vu...@rt-rk.com>
gcc/ChangeLog:
* tree-ssa-phiopt.cc (rhs_is_fed_for_value_replacement): Add a new
optimization opportunity for BIT_IOR_EXPR and a != b.
(operand_equal_for_value_replacement): Ditto.
gcc/testsuite/ChangeLog:
* gcc.dg/tree-ssa/phi-opt-11.c: Add more tests.
Diff:
---
gcc/testsuite/gcc.dg/tree-ssa/phi-opt-11.c | 31 ++++++++++++++++++-
gcc/tree-ssa-phiopt.cc | 48 +++++++++++++++++++-----------
2 files changed, 60 insertions(+), 19 deletions(-)
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-11.c
b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-11.c
index 14c82cd52165..d1e284c53252 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-11.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-11.c
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-options "-O1 -fdump-tree-optimized --param
logical-op-non-short-circuit=1" } */
+/* { dg-options "-O1 -fdump-tree-phiopt2 -fdump-tree-optimized --param
logical-op-non-short-circuit=1" } */
int f(int a, int b, int c)
{
@@ -22,4 +22,33 @@ int h(int a, int b, int c, int d)
return a;
}
+int i(int a, int b, int c)
+{
+ if ((a > c) & (a == b))
+ return a;
+ return b;
+}
+
+int j(int a, int b, int c)
+{
+ if ((a > c) & (a == b))
+ return b;
+ return a;
+}
+
+int k(int a, int b, int c)
+{
+ if ((a > c) | (a != b))
+ return b;
+ return a;
+}
+
+int l(int a, int b, int c)
+{
+ if ((a > c) | (a != b))
+ return a;
+ return b;
+}
+
+/* { dg-final { scan-tree-dump-times "if" 0 "phiopt2" } } */
/* { dg-final { scan-tree-dump-times "if" 0 "optimized" } } */
diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
index 0d9bc3eb4499..15651809d71b 100644
--- a/gcc/tree-ssa-phiopt.cc
+++ b/gcc/tree-ssa-phiopt.cc
@@ -1077,17 +1077,18 @@ jump_function_from_stmt (tree *arg, gimple *stmt)
return false;
}
-/* RHS is a source argument in a BIT_AND_EXPR which feeds a conditional
- of the form SSA_NAME NE 0.
+/* RHS is a source argument in a BIT_AND_EXPR or BIT_IOR_EXPR which feeds
+ a conditional of the form SSA_NAME NE 0.
- If RHS is fed by a simple EQ_EXPR comparison of two values, see if
- the two input values of the EQ_EXPR match arg0 and arg1.
+ If RHS is fed by a simple EQ_EXPR or NE_EXPR comparison of two values,
+ see if the two input values of the comparison match arg0 and arg1.
If so update *code and return TRUE. Otherwise return FALSE. */
static bool
rhs_is_fed_for_value_replacement (const_tree arg0, const_tree arg1,
- enum tree_code *code, const_tree rhs)
+ enum tree_code *code, const_tree rhs,
+ enum tree_code bit_expression_code)
{
/* Obviously if RHS is not an SSA_NAME, we can't look at the defining
statement. */
@@ -1095,11 +1096,15 @@ rhs_is_fed_for_value_replacement (const_tree arg0,
const_tree arg1,
{
gimple *def1 = SSA_NAME_DEF_STMT (rhs);
- /* Verify the defining statement has an EQ_EXPR on the RHS. */
- if (is_gimple_assign (def1) && gimple_assign_rhs_code (def1) == EQ_EXPR)
+ /* Verify the defining statement has an EQ_EXPR or NE_EXPR on the RHS.
*/
+ if (is_gimple_assign (def1)
+ && ((bit_expression_code == BIT_AND_EXPR
+ && gimple_assign_rhs_code (def1) == EQ_EXPR)
+ || (bit_expression_code == BIT_IOR_EXPR
+ && gimple_assign_rhs_code (def1) == NE_EXPR)))
{
- /* Finally verify the source operands of the EQ_EXPR are equal
- to arg0 and arg1. */
+ /* Finally verify the source operands of the EQ_EXPR or NE_EXPR
+ are equal to arg0 and arg1. */
tree op0 = gimple_assign_rhs1 (def1);
tree op1 = gimple_assign_rhs2 (def1);
if ((operand_equal_for_phi_arg_p (arg0, op0)
@@ -1118,8 +1123,9 @@ rhs_is_fed_for_value_replacement (const_tree arg0,
const_tree arg1,
/* Return TRUE if arg0/arg1 are equal to the rhs/lhs or lhs/rhs of COND.
- Also return TRUE if arg0/arg1 are equal to the source arguments of a
- an EQ comparison feeding a BIT_AND_EXPR which feeds COND.
+ Also return TRUE if arg0/arg1 are equal to the source arguments of an
+ EQ comparison feeding a BIT_AND_EXPR, or NE comparison feeding a
+ BIT_IOR_EXPR which feeds COND.
Return FALSE otherwise. */
@@ -1138,27 +1144,33 @@ operand_equal_for_value_replacement (const_tree arg0,
const_tree arg1,
return true;
/* Now handle more complex case where we have an EQ comparison
- which feeds a BIT_AND_EXPR which feeds COND.
+ feeding a BIT_AND_EXPR, or a NE comparison feeding a BIT_IOR_EXPR,
+ which then feeds into COND.
First verify that COND is of the form SSA_NAME NE 0. */
if (*code != NE_EXPR || !integer_zerop (rhs)
|| TREE_CODE (lhs) != SSA_NAME)
return false;
- /* Now ensure that SSA_NAME is set by a BIT_AND_EXPR. */
+ /* Now ensure that SSA_NAME is set by a BIT_AND_EXPR or BIT_OR_EXPR. */
def = SSA_NAME_DEF_STMT (lhs);
- if (!is_gimple_assign (def) || gimple_assign_rhs_code (def) != BIT_AND_EXPR)
+ if (!is_gimple_assign (def)
+ || (gimple_assign_rhs_code (def) != BIT_AND_EXPR
+ && gimple_assign_rhs_code (def) != BIT_IOR_EXPR))
return false;
- /* Now verify arg0/arg1 correspond to the source arguments of an
- EQ comparison feeding the BIT_AND_EXPR. */
+ /* Now verify arg0/arg1 correspond to the source arguments of an EQ
+ comparison feeding the BIT_AND_EXPR or a NE comparison feeding the
+ BIT_IOR_EXPR. */
tree tmp = gimple_assign_rhs1 (def);
- if (rhs_is_fed_for_value_replacement (arg0, arg1, code, tmp))
+ if (rhs_is_fed_for_value_replacement (arg0, arg1, code, tmp,
+ gimple_assign_rhs_code (def)))
return true;
tmp = gimple_assign_rhs2 (def);
- if (rhs_is_fed_for_value_replacement (arg0, arg1, code, tmp))
+ if (rhs_is_fed_for_value_replacement (arg0, arg1, code, tmp,
+ gimple_assign_rhs_code (def)))
return true;
return false;
