PR57993 reports a scenario where a conditional candidate is incorrectly
replaced when its putative "hidden basis" (the basis hidden by one or
more PHI nodes) does not dominate all of the PHI nodes on which the
candidate depends.
This indicates that the hidden basis was incorrectly identified as a
useful basis for the candidate. There are two ways we can fix this.
One would be to add more complexity to the code that determines the
hidden basis. Currently that code does not recursively follow the PHI
definitions backwards to ensure that the basis dominates all the PHIs.
Adding code to do this would ensure we didn't identify an incorrect
basis in the first place, but would duplicate quite a bit of existing
logic in the later analysis phase, and increase compile time.
What I've done here is to instead delay detection of the problem until
the analysis phase. If we detect that we chose an invalid basis, we
assign an "infinite" cost to the replacement so that we won't pursue it
further.
This requires that we know which basic block contains the basis
statement. The basis statement may have itself been replaced by another
statement earlier, so we need to keep the candidate table up to date
with respect to such replacements.
Bootstrapped and tested on powerpc64-linux-gnu with no new regressions.
Ok for trunk?
Thanks,
Bill
gcc:
2013-07-27 Bill Schmidt <wschm...@vnet.linux.ibm.com>
* gimple-ssa-strength-reduction.c (replace_mult_candidate): Record
replaced statement in the candidate table.
(phi_add_costs): Return infinite cost when the hidden basis does
not dominate all phis on which the candidate is dependent.
(replace_one_candidate): Record replaced statement in the
candidate table.
gcc/testsuite:
2013-07-27 Bill Schmidt <wschm...@vnet.linux.ibm.com>
* gcc.dg/torture/pr57993.c: New test.
Index: gcc/testsuite/gcc.dg/torture/pr57993.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/pr57993.c (revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr57993.c (revision 0)
@@ -0,0 +1,30 @@
+/* This ICEd prior to fixing PR57993. */
+/* { dg-do compile } */
+
+int a, b, c, d;
+char e;
+unsigned g;
+
+void f(void)
+{
+ int h;
+
+ for(; d; d++)
+ if(d)
+lbl:
+ g + a || (d = 0);
+
+ b && (a = e);
+
+ for(h = 0; h < 1; ++h)
+ {
+ h = c ? : (d = 0);
+ g = a = (e | 0);
+ }
+
+ if(a)
+ goto lbl;
+
+ a = e = 0;
+ goto lbl;
+}
Index: gcc/gimple-ssa-strength-reduction.c
===================================================================
--- gcc/gimple-ssa-strength-reduction.c (revision 201267)
+++ gcc/gimple-ssa-strength-reduction.c (working copy)
@@ -1882,6 +1882,7 @@ replace_mult_candidate (slsr_cand_t c, tree basis_
gimple_stmt_iterator gsi = gsi_for_stmt (c->cand_stmt);
gimple_set_location (copy_stmt, gimple_location (c->cand_stmt));
gsi_replace (&gsi, copy_stmt, false);
+ c->cand_stmt = copy_stmt;
if (dump_file && (dump_flags & TDF_DETAILS))
stmt_to_print = copy_stmt;
}
@@ -2179,6 +2180,18 @@ phi_add_costs (gimple phi, slsr_cand_t c, int one_
int cost = 0;
slsr_cand_t phi_cand = base_cand_from_table (gimple_phi_result (phi));
+ /* If we work our way back to a phi that isn't dominated by the hidden
+ basis, this isn't a candidate for replacement. Indicate this by
+ returning an unreasonably high cost. It's not easy to detect
+ these situations when determining the basis, so we defer the
+ decision until now. */
+ basic_block phi_bb = gimple_bb (phi);
+ slsr_cand_t basis = lookup_cand (c->basis);
+ basic_block basis_bb = gimple_bb (basis->cand_stmt);
+
+ if (phi_bb == basis_bb || !dominated_by_p (CDI_DOMINATORS, phi_bb, basis_bb))
+ return COST_INFINITE;
+
for (i = 0; i < gimple_phi_num_args (phi); i++)
{
tree arg = gimple_phi_arg_def (phi, i);
@@ -3226,6 +3239,7 @@ replace_one_candidate (slsr_cand_t c, unsigned i,
gimple_stmt_iterator gsi = gsi_for_stmt (c->cand_stmt);
gimple_set_location (copy_stmt, gimple_location (c->cand_stmt));
gsi_replace (&gsi, copy_stmt, false);
+ c->cand_stmt = copy_stmt;
if (dump_file && (dump_flags & TDF_DETAILS))
stmt_to_print = copy_stmt;
@@ -3238,6 +3252,7 @@ replace_one_candidate (slsr_cand_t c, unsigned i,
NULL_TREE);
gimple_set_location (cast_stmt, gimple_location (c->cand_stmt));
gsi_replace (&gsi, cast_stmt, false);
+ c->cand_stmt = cast_stmt;
if (dump_file && (dump_flags & TDF_DETAILS))
stmt_to_print = cast_stmt;
