[PATCH] more optimizations for tree-ssa-phiopt
law@redhat.com
law@redhat.com
Tue May 18 19:03:00 GMT 2004
In message <62865210-A6AE-11D8-9DC2-000393A6D2F2@physics.uc.edu>, Andrew Pinski
writes:
>This is patch is the equivalent tree optimization to the rtl one which
>fixed
>PR 10817. This is also the other part which is needed to fix PR 14135.
>
>This has been on the lno branch with no problems for a while now.
>
>OK?
>Bootstrapped on powerpc-apple-darwin7.3.0 with no regressions.
>
>Thanks,
>Andrew Pinski
>
>Testcase:
>/* { dg-do compile } */
>/* { dg-options "-O1 -fdump-tree-phiopt1-details" } */
>int f(int a, int b)
>{
> int c = b;
> if (a != b)
> c = a;
> return c;
>}
>
>/* Should have no ifs left after straightening. */
>/* { dg-final { scan-tree-dump-times "if " 0 "phiopt1"} } */
>
>ChangeLog:
> * tree-ssa-phiopt.c (value_replacement): New function.
> (tree_ssa_phiopt): Call it.
I made several changes to this patch.
First, there were two large hunks of code which were duplicated between
conditional_replacement and value_replacement. Those were factored into
subroutines that are used by conditional_replacement and value_replacement.
Second, I cleaned up the test for when this optimization applies. It really
ought to apply for any equality test as long as the arguments in the
equality test match those in the PHI node. The fact that it's not triggering
for EQ_EXPR is AFAICT an artifact of the order in which we happen to visit
blocks and how we record equivalences created by conditionals during the
dominator walk. A change in either of those implementation details would
likely result in this optimization triggering for EQ_EXPR.
Additionally I generalized the code so that it didn't care what order the
PHI alternatives are found. This allowed the optimization to trigger a
couple more times during a GCC bootstrap.
Finally, you added flags.h to the include list, but didn't update the
dependencies in Makefile.in. I fixed that.
Here's the resulting patch I'm checking in (bootstrapped and regression
tested on i686-pc-linux-gnu).
* Makefile.in (tree-ssa-phiopt.o): Depends on flags.h.
* tree-ssa-phiopt.c: Include flags.h.
(conditional_replacement): Remove argument names from prototype.
Minor formatting and comment fixes.
(tree_ssa_phiopt): If conditional_replacement returns false, then
call value_replacement.
(value_replacement): New function.
* gcc.dg/tree-ssa/20040518-1.c: New test.
Index: Makefile.in
===================================================================
RCS file: /cvs/gcc/gcc/gcc/Makefile.in,v
retrieving revision 1.1277
diff -c -p -r1.1277 Makefile.in
*** Makefile.in 17 May 2004 15:51:22 -0000 1.1277
--- Makefile.in 18 May 2004 17:19:56 -0000
*************** tree-ssa-forwprop.o : tree-ssa-forwprop.
*** 1581,1587 ****
$(TREE_FLOW_H) tree-pass.h $(TREE_DUMP_H)
tree-ssa-phiopt.o : tree-ssa-phiopt.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \
$(TM_H) errors.h $(GGC_H) $(TREE_H) $(RTL_H) $(TM_P_H) $(BASIC_BLOCK_H) \
! $(TREE_FLOW_H) tree-pass.h $(TREE_DUMP_H) langhooks.h
tree-nrv.o : tree-nrv.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \
$(TM_H) $(TREE_H) $(RTL_H) function.h $(BASIC_BLOCK_H) $(EXPR_H) \
diagnostic.h $(TREE_FLOW_H) $(TIMEVAR_H) $(TREE_DUMP_H) tree-pass.h \
--- 1581,1587 ----
$(TREE_FLOW_H) tree-pass.h $(TREE_DUMP_H)
tree-ssa-phiopt.o : tree-ssa-phiopt.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \
$(TM_H) errors.h $(GGC_H) $(TREE_H) $(RTL_H) $(TM_P_H) $(BASIC_BLOCK_H) \
! $(TREE_FLOW_H) tree-pass.h $(TREE_DUMP_H) langhooks.h flags.h
tree-nrv.o : tree-nrv.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \
$(TM_H) $(TREE_H) $(RTL_H) function.h $(BASIC_BLOCK_H) $(EXPR_H) \
diagnostic.h $(TREE_FLOW_H) $(TIMEVAR_H) $(TREE_DUMP_H) tree-pass.h \
Index: tree-ssa-phiopt.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-phiopt.c,v
retrieving revision 2.4
diff -c -p -r2.4 tree-ssa-phiopt.c
*** tree-ssa-phiopt.c 18 May 2004 16:23:25 -0000 2.4
--- tree-ssa-phiopt.c 18 May 2004 17:19:57 -0000
*************** Software Foundation, 59 Temple Place - S
*** 26,31 ****
--- 26,32 ----
#include "ggc.h"
#include "tree.h"
#include "rtl.h"
+ #include "flags.h"
#include "tm_p.h"
#include "basic-block.h"
#include "timevar.h"
*************** Software Foundation, 59 Temple Place - S
*** 36,50 ****
#include "langhooks.h"
static void tree_ssa_phiopt (void);
! static bool conditional_replacement (basic_block bb, tree phi, tree arg0,
! tree arg1);
static void replace_phi_with_stmt (block_stmt_iterator, basic_block,
basic_block, tree, tree);
static bool candidate_bb_for_phi_optimization (basic_block,
basic_block *,
basic_block *);
-
/* This pass eliminates PHI nodes which can be trivially implemented as
an assignment from a conditional expression. ie if we have something
like:
--- 37,50 ----
#include "langhooks.h"
static void tree_ssa_phiopt (void);
! static bool conditional_replacement (basic_block, tree, tree, tree);
! static bool value_replacement (basic_block, tree, tree, tree);
static void replace_phi_with_stmt (block_stmt_iterator, basic_block,
basic_block, tree, tree);
static bool candidate_bb_for_phi_optimization (basic_block,
basic_block *,
basic_block *);
/* This pass eliminates PHI nodes which can be trivially implemented as
an assignment from a conditional expression. ie if we have something
like:
*************** static bool candidate_bb_for_phi_optimiz
*** 64,70 ****
bb1 will become unreachable and bb0 and bb2 will almost always
be merged into a single block. This occurs often due to gimplification
! of conditionals. */
static void
tree_ssa_phiopt (void)
--- 64,88 ----
bb1 will become unreachable and bb0 and bb2 will almost always
be merged into a single block. This occurs often due to gimplification
! of conditionals.
!
! Also done is the following optimization:
!
! bb0:
! if (a != b) goto bb2; else goto bb1;
! bb1:
! bb2:
! x = PHI (a (bb1), b (bb0))
!
! We can rewrite that as:
!
! bb0:
! bb1:
! bb2:
! x = b;
!
! This can sometimes occur as a result of other optimizations. A
! similar transformation is done by the ifcvt RTL optimizer. */
static void
tree_ssa_phiopt (void)
*************** tree_ssa_phiopt (void)
*** 77,83 ****
{
tree arg0, arg1, phi;
-
/* We're searching for blocks with one PHI node which has two
arguments. */
phi = phi_nodes (bb);
--- 95,100 ----
*************** tree_ssa_phiopt (void)
*** 88,99 ****
arg1 = PHI_ARG_DEF (phi, 1);
/* Do the replacement of conditional if it can be done. */
! if (conditional_replacement (bb, phi, arg0, arg1))
! {
! /* We have done the replacement so we need to rebuild the cfg. */
! removed_phis = true;
! continue;
! }
}
}
--- 105,117 ----
arg1 = PHI_ARG_DEF (phi, 1);
/* Do the replacement of conditional if it can be done. */
! if (conditional_replacement (bb, phi, arg0, arg1)
! || value_replacement (bb, phi, arg0, arg1))
! {
! /* We have done the replacement so we need to rebuild the
! cfg when this pass is complete. */
! removed_phis = true;
! }
}
}
*************** conditional_replacement (basic_block bb,
*** 294,301 ****
TREE_OPERAND (old_result, 0),
TREE_OPERAND (old_result, 1));
! new1 = build (MODIFY_EXPR, TREE_TYPE (result),
! new_var, new1);
bsi_insert_after (&bsi, new1, BSI_NEW_STMT);
}
--- 312,318 ----
TREE_OPERAND (old_result, 0),
TREE_OPERAND (old_result, 1));
! new1 = build (MODIFY_EXPR, TREE_TYPE (result), new_var, new1);
bsi_insert_after (&bsi, new1, BSI_NEW_STMT);
}
*************** conditional_replacement (basic_block bb,
*** 330,336 ****
cond = cond1;
/* If what we get back is a conditional expression, there is no
! way that is can be gimple. */
if (TREE_CODE (cond) == COND_EXPR)
return false;
--- 347,353 ----
cond = cond1;
/* If what we get back is a conditional expression, there is no
! way that it can be gimple. */
if (TREE_CODE (cond) == COND_EXPR)
return false;
*************** conditional_replacement (basic_block bb,
*** 358,363 ****
--- 375,450 ----
/* Note that we optimized this PHI. */
return true;
+ }
+
+ /* The function value_replacement does the main work of doing the value
+ replacement. Return true if the replacement is done. Otherwise return
+ false.
+ BB is the basic block where the replacement is going to be done on. ARG0
+ is argument 0 from the PHI. Likewise for ARG1. */
+
+ static bool
+ value_replacement (basic_block bb, tree phi, tree arg0, tree arg1)
+ {
+ tree result;
+ basic_block other_block = NULL;
+ basic_block cond_block = NULL;
+ tree new, cond;
+ edge true_edge, false_edge;
+
+ /* If the type says honor signed zeros we cannot do this
+ optimization. */
+ if (HONOR_SIGNED_ZEROS (TYPE_MODE (TREE_TYPE (arg1))))
+ return false;
+
+ if (!candidate_bb_for_phi_optimization (bb, &cond_block, &other_block))
+ return false;
+
+ cond = COND_EXPR_COND (last_stmt (cond_block));
+ result = PHI_RESULT (phi);
+
+ /* This transformation is only valid for equality comparisons. */
+ if (TREE_CODE (cond) != NE_EXPR && TREE_CODE (cond) != EQ_EXPR)
+ return false;
+
+ /* We need to know which is the true edge and which is the false
+ edge so that we know if have abs or negative abs. */
+ extract_true_false_edges_from_block (cond_block, &true_edge, &false_edge);
+
+ /* At this point we know we have a COND_EXPR with two successors.
+ One successor is BB, the other successor is an empty block which
+ falls through into BB.
+
+ The condition for the COND_EXPR is known to be NE_EXPR or EQ_EXPR.
+
+ There is a single PHI node at the join point (BB) with two arguments.
+
+ We now need to verify that the two arguments in the PHI node match
+ the two arguments to the equality comparison. */
+
+ if ((operand_equal_p (arg0, TREE_OPERAND (cond, 0), 0)
+ && operand_equal_p (arg1, TREE_OPERAND (cond, 1), 0))
+ || (operand_equal_p (arg1, TREE_OPERAND (cond, 0), 0)
+ && operand_equal_p (arg0, TREE_OPERAND (cond, 1), 0)))
+ {
+ edge e;
+ tree arg;
+
+ e = (TREE_CODE (cond) == NE_EXPR ? true_edge : false_edge);
+ if (PHI_ARG_EDGE (phi, 0) == e)
+ arg = arg0;
+ else
+ arg = arg1;
+
+ /* Build the new assignment. */
+ new = build (MODIFY_EXPR, TREE_TYPE (result), result, arg);
+
+ replace_phi_with_stmt (bsi_start (bb), bb, cond_block, phi, new);
+
+ /* Note that we optimized this PHI. */
+ return true;
+ }
+ return false;
}
Index: testsuite//gcc.dg/tree-ssa/20040518-1.c
===================================================================
RCS file: testsuite//gcc.dg/tree-ssa/20040518-1.c
diff -N testsuite//gcc.dg/tree-ssa/20040518-1.c
*** /dev/null 1 Jan 1970 00:00:00 -0000
--- testsuite//gcc.dg/tree-ssa/20040518-1.c 18 May 2004 17:23:23 -0000
***************
*** 0 ****
--- 1,12 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O1 -fdump-tree-phiopt1-details" } */
+ int f(int a, int b)
+ {
+ int c = b;
+ if (a != b)
+ c = a;
+ return c;
+ }
+
+ /* Should have no ifs left after straightening. */
+ /* { dg-final { scan-tree-dump-times "if " 0 "phiopt1"} } */
More information about the Gcc-patches
mailing list