procyberian/gcc
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Thanks for the feedback on the first version of the patch. Accordingly:

Browse source 
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.Vukic@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.
This commit is contained in:
Jovan Vukic 2024-12-01 11:57:41 -07:00 • committed by Jeff Law
parent 113e902e84
commit 999aad44aa
2 changed files with 60 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
gcc/testsuite/gcc.dg/tree-ssa/phi-opt-11.c
View file

@ -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" } } */

48
gcc/tree-ssa-phiopt.cc
View file

@ -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;