Fix PR80054 (SLSR dominance bug)

Submitted by William J. Schmidt on March 17, 2017, 5:44 p.m.

Details

Message ID b41a5614-a24b-eb5c-ebd6-1160453e9e07@linux.vnet.ibm.com
State New
Headers show

Commit Message

William J. Schmidt March 17, 2017, 5:44 p.m.
Hi,

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80054 reports an ICE
in SSA validation after SLSR (definition does not dominate use).
The problem occurs within conditional strength reduction candidate
processing.

PRE creates a situation where a conditional SLSR candidate depends
on a PHI that is not dominated by the basis for the candidate.
SLSR currently doesn't notice this and eventually creates a phi
basis that is not dominated by the true basis, leading to the error.

This patch eliminates the problem by detecting the dominance failure
and aborting the optimization when it occurs.

Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
regressions.  I'll plan to commit this on Monday if there are no
comments.

Thanks,
Bill


[gcc]

2017-03-17  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	PR tree-optimization/80054
	* gimple-ssa-strength-reduction.c (all_phi_incrs_profitable): Fail
	the optimization if a PHI or any of its arguments is not dominated
	by the candidate's basis.  Use gphi* rather than gimple* as
	appropriate.
	(replace_profitable_candidates): Clean up a gimple* variable that
	should be a gphi* variable.

[gcc/testsuite]

2017-03-17  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	PR tree-optimization/80054
	* g++.dg/torture/pr80054.C: New file.

Patch hide | download patch | download mbox

Index: gcc/gimple-ssa-strength-reduction.c
===================================================================
--- gcc/gimple-ssa-strength-reduction.c	(revision 246198)
+++ gcc/gimple-ssa-strength-reduction.c	(working copy)
@@ -3279,17 +3279,34 @@  insert_initializers (slsr_cand_t c)
 }
 
 /* Return TRUE iff all required increments for candidates feeding PHI
-   are profitable to replace on behalf of candidate C.  */
+   are profitable (and legal!) to replace on behalf of candidate C.  */
 
 static bool
-all_phi_incrs_profitable (slsr_cand_t c, gimple *phi)
+all_phi_incrs_profitable (slsr_cand_t c, gphi *phi)
 {
   unsigned i;
   slsr_cand_t basis = lookup_cand (c->basis);
   slsr_cand_t phi_cand = *stmt_cand_map->get (phi);
 
+  /* If the basis doesn't dominate the PHI (including when the PHI is
+     in the same block as the basis), we won't be able to create a PHI
+     using the basis here.  */
+  basic_block basis_bb = gimple_bb (basis->cand_stmt);
+  basic_block phi_bb = gimple_bb (phi);
+
+  if (phi_bb == basis_bb
+      || !dominated_by_p (CDI_DOMINATORS, phi_bb, basis_bb))
+    return false;
+
   for (i = 0; i < gimple_phi_num_args (phi); i++)
     {
+      /* If the PHI arg resides in a block not dominated by the basis,
+	 we won't be able to create a PHI using the basis here.  */
+      basic_block pred_bb = gimple_phi_arg_edge (phi, i)->src;
+
+      if (!dominated_by_p (CDI_DOMINATORS, pred_bb, basis_bb))
+	return false;
+
       tree arg = gimple_phi_arg_def (phi, i);
 
       if (!operand_equal_p (arg, phi_cand->base_expr, 0))
@@ -3298,7 +3315,7 @@  static bool
 
 	  if (gimple_code (arg_def) == GIMPLE_PHI)
 	    {
-	      if (!all_phi_incrs_profitable (c, arg_def))
+	      if (!all_phi_incrs_profitable (c, as_a <gphi *> (arg_def)))
 		return false;
 	    }
 	  else
@@ -3565,7 +3582,7 @@  replace_profitable_candidates (slsr_cand_t c)
 	{
 	  if (phi_dependent_cand_p (c))
 	    {
-	      gimple *phi = lookup_cand (c->def_phi)->cand_stmt;
+	      gphi *phi = as_a <gphi *> (lookup_cand (c->def_phi)->cand_stmt);
 
 	      if (all_phi_incrs_profitable (c, phi))
 		{
Index: gcc/testsuite/g++.dg/torture/pr80054.C
===================================================================
--- gcc/testsuite/g++.dg/torture/pr80054.C	(nonexistent)
+++ gcc/testsuite/g++.dg/torture/pr80054.C	(working copy)
@@ -0,0 +1,40 @@ 
+/* { dg-do compile } */
+
+/* Used to fail in SLSR because of a dominance violation.  PR80054.  */
+
+extern short var_2;
+extern short var_4;
+extern const bool var_32;
+extern short var_36;
+extern const bool var_37;
+extern bool var_46;
+extern unsigned int var_47;
+extern short var_49;
+extern unsigned int var_56;
+extern unsigned int var_62;
+extern unsigned int var_65;
+extern bool var_831;
+extern unsigned int var_843;
+extern short var_846;
+extern short var_889;
+
+void foo() {
+  if (var_36 * var_37)
+    var_831 = var_56 = 0;
+  else
+    var_65 = 0;
+
+  if (var_46)
+    var_843 = 0;
+
+  var_846 = 0;
+
+  if ((var_4 == 0) >> (var_32 | -(var_37 < var_46 || var_36)) + 8)
+    var_49 = 2032651381 * bool(var_2 * var_37);
+  else {
+    var_62 = 0;
+    var_47 = (var_46 || var_36) * (var_2 * var_37);
+  }
+
+  var_889 = bool(var_2 * var_37);
+}