diff mbox

Fix PR tree-optimization/49539

Message ID 201106290017.25664.ebotcazou@adacore.com
State New
Headers show

Commit Message

Eric Botcazou June 28, 2011, 10:17 p.m. UTC 
Hi,

this is an ICE building the gnattools on ARM, a regression present on the
mainline (and reproducible on x86/Linux by switching to SJLJ exceptions).

For the reduced testcase compiled at -O:

Unable to coalesce ssa_names 2 and 174 which are marked as MUST COALESCE.
comp_last_2(ab) and  comp_last_174(ab)
+===========================GNAT BUG DETECTED==============================+
| 4.7.0 20110626 (experimental) [trunk revision 175408] (i586-suse-linux-gnu) 
GCC error:|
| SSA corruption                                                           |
| Error detected around p.adb:3:4

The SSA names (or rather 2 related ones) have overlapping lifetimes.  The
problem is created by forwprop1.  Before:

<bb 23>:
  # comp_last_1(ab) = PHI <comp_last_159(ab)(20), comp_last_2(ab)(22)>

[...]

  comp_last_174(ab) = comp_last_1(ab) + 1;
  D.2425_175 = args.P_BOUNDS;
  D.2426_176 = D.2425_175->LB0;
  if (D.2426_176 > comp_last_174(ab))
    goto <bb 39>;
  else
    goto <bb 38>;

<bb 38>:
  D.2425_177 = args.P_BOUNDS;
  D.2427_178 = D.2425_177->UB0;
  if (D.2427_178 < comp_last_174(ab))
    goto <bb 39>;
  else
    goto <bb 40>;

[...]

  comp_last_185(ab) = comp_last_174(ab) + 1;
  D.2425_186 = args.P_BOUNDS;
  D.2426_187 = D.2425_186->LB0;
  if (D.2426_187 > comp_last_185(ab))
    goto <bb 43>;
  else
    goto <bb 42>;


After:

  comp_last_185(ab) = comp_last_1(ab) + 2;
  D.2425_186 = args.P_BOUNDS;
  D.2426_187 = D.2425_186->LB0;
  if (D.2426_187 > comp_last_185(ab))
    goto <bb 43>;
  else
    goto <bb 42>;


The pass already contains a check for this situation in can_propagate_from but 
it isn't applied in this case.

Tested on x86_64-suse-linux, OK for the mainline?


2011-06-28  Eric Botcazou  <ebotcazou@adacore.com>

	PR tree-optimization/49539
	* tree-ssa-forwprop.c (can_propagate_from): Check for abnormal SSA
	by means of stmt_references_abnormal_ssa_name.
	(associate_plusminus): Call can_propagate_from before propagating
	from definition statements.
	(ssa_forward_propagate_and_combine): Remove superfluous newline.

Comments

Richard Biener June 29, 2011, 9:09 a.m. UTC | #1 
On Wed, Jun 29, 2011 at 12:17 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> this is an ICE building the gnattools on ARM, a regression present on the
> mainline (and reproducible on x86/Linux by switching to SJLJ exceptions).
>
> For the reduced testcase compiled at -O:
>
> Unable to coalesce ssa_names 2 and 174 which are marked as MUST COALESCE.
> comp_last_2(ab) and  comp_last_174(ab)
> +===========================GNAT BUG DETECTED==============================+
> | 4.7.0 20110626 (experimental) [trunk revision 175408] (i586-suse-linux-gnu)
> GCC error:|
> | SSA corruption                                                           |
> | Error detected around p.adb:3:4
>
> The SSA names (or rather 2 related ones) have overlapping lifetimes.  The
> problem is created by forwprop1.  Before:
>
> <bb 23>:
>  # comp_last_1(ab) = PHI <comp_last_159(ab)(20), comp_last_2(ab)(22)>
>
> [...]
>
>  comp_last_174(ab) = comp_last_1(ab) + 1;
>  D.2425_175 = args.P_BOUNDS;
>  D.2426_176 = D.2425_175->LB0;
>  if (D.2426_176 > comp_last_174(ab))
>    goto <bb 39>;
>  else
>    goto <bb 38>;
>
> <bb 38>:
>  D.2425_177 = args.P_BOUNDS;
>  D.2427_178 = D.2425_177->UB0;
>  if (D.2427_178 < comp_last_174(ab))
>    goto <bb 39>;
>  else
>    goto <bb 40>;
>
> [...]
>
>  comp_last_185(ab) = comp_last_174(ab) + 1;
>  D.2425_186 = args.P_BOUNDS;
>  D.2426_187 = D.2425_186->LB0;
>  if (D.2426_187 > comp_last_185(ab))
>    goto <bb 43>;
>  else
>    goto <bb 42>;
>
>
> After:
>
>  comp_last_185(ab) = comp_last_1(ab) + 2;
>  D.2425_186 = args.P_BOUNDS;
>  D.2426_187 = D.2425_186->LB0;
>  if (D.2426_187 > comp_last_185(ab))
>    goto <bb 43>;
>  else
>    goto <bb 42>;
>
>
> The pass already contains a check for this situation in can_propagate_from but
> it isn't applied in this case.
>
> Tested on x86_64-suse-linux, OK for the mainline?

Ok.  This is also latent on the 4.6 branch, so feel free to backport it.

Thanks,
Richard.

>
> 2011-06-28  Eric Botcazou  <ebotcazou@adacore.com>
>
>        PR tree-optimization/49539
>        * tree-ssa-forwprop.c (can_propagate_from): Check for abnormal SSA
>        by means of stmt_references_abnormal_ssa_name.
>        (associate_plusminus): Call can_propagate_from before propagating
>        from definition statements.
>        (ssa_forward_propagate_and_combine): Remove superfluous newline.
>
>
> --
> Eric Botcazou
>
diff mbox

Patch

Index: tree-ssa-forwprop.c
===================================================================
--- tree-ssa-forwprop.c	(revision 175408)
+++ tree-ssa-forwprop.c	(working copy)
@@ -260,9 +260,6 @@  get_prop_source_stmt (tree name, bool si
 static bool
 can_propagate_from (gimple def_stmt)
 {
-  use_operand_p use_p;
-  ssa_op_iter iter;
-
   gcc_assert (is_gimple_assign (def_stmt));
 
   /* If the rhs has side-effects we cannot propagate from it.  */
@@ -280,9 +277,8 @@  can_propagate_from (gimple def_stmt)
     return true;
 
   /* We cannot propagate ssa names that occur in abnormal phi nodes.  */
-  FOR_EACH_SSA_USE_OPERAND (use_p, def_stmt, iter, SSA_OP_USE)
-    if (SSA_NAME_OCCURS_IN_ABNORMAL_PHI (USE_FROM_PTR (use_p)))
-      return false;
+  if (stmt_references_abnormal_ssa_name (def_stmt))
+    return false;
 
   /* If the definition is a conversion of a pointer to a function type,
      then we can not apply optimizations as some targets require
@@ -1780,7 +1776,8 @@  associate_plusminus (gimple stmt)
 	{
 	  gimple def_stmt = SSA_NAME_DEF_STMT (rhs2);
 	  if (is_gimple_assign (def_stmt)
-	      && gimple_assign_rhs_code (def_stmt) == NEGATE_EXPR)
+	      && gimple_assign_rhs_code (def_stmt) == NEGATE_EXPR
+	      && can_propagate_from (def_stmt))
 	    {
 	      code = (code == MINUS_EXPR) ? PLUS_EXPR : MINUS_EXPR;
 	      gimple_assign_set_rhs_code (stmt, code);
@@ -1797,7 +1794,8 @@  associate_plusminus (gimple stmt)
 	{
 	  gimple def_stmt = SSA_NAME_DEF_STMT (rhs1);
 	  if (is_gimple_assign (def_stmt)
-	      && gimple_assign_rhs_code (def_stmt) == NEGATE_EXPR)
+	      && gimple_assign_rhs_code (def_stmt) == NEGATE_EXPR
+	      && can_propagate_from (def_stmt))
 	    {
 	      code = MINUS_EXPR;
 	      gimple_assign_set_rhs_code (stmt, code);
@@ -1840,7 +1838,7 @@  associate_plusminus (gimple stmt)
   if (TREE_CODE (rhs1) == SSA_NAME)
     {
       gimple def_stmt = SSA_NAME_DEF_STMT (rhs1);
-      if (is_gimple_assign (def_stmt))
+      if (is_gimple_assign (def_stmt) && can_propagate_from (def_stmt))
 	{
 	  enum tree_code def_code = gimple_assign_rhs_code (def_stmt);
 	  if (def_code == PLUS_EXPR
@@ -1940,7 +1938,7 @@  associate_plusminus (gimple stmt)
   if (rhs2 && TREE_CODE (rhs2) == SSA_NAME)
     {
       gimple def_stmt = SSA_NAME_DEF_STMT (rhs2);
-      if (is_gimple_assign (def_stmt))
+      if (is_gimple_assign (def_stmt) && can_propagate_from (def_stmt))
 	{
 	  enum tree_code def_code = gimple_assign_rhs_code (def_stmt);
 	  if (def_code == PLUS_EXPR
@@ -2262,8 +2260,7 @@  ssa_forward_propagate_and_combine (void)
 	      else
 		gsi_next (&gsi);
 	    }
-	  else if (code == POINTER_PLUS_EXPR
-		   && can_propagate_from (stmt))
+	  else if (code == POINTER_PLUS_EXPR && can_propagate_from (stmt))
 	    {
 	      if (TREE_CODE (gimple_assign_rhs2 (stmt)) == INTEGER_CST
 		  /* ???  Better adjust the interface to that function