[PATCH] Fix PR49876: Continue code generation with integer_zero_node on gloog_error

[PATCH] Fix PR49876: Continue code generation with integer_zero_node on gloog_error

[Sebastian Pop]

When setting gloog_error, graphite should continue code generation
without early returns, as otherwise the SSA representation would not
be complete.  So set the new expression to integer_zero_node, that
would not require more SSA updates, and continue code generation as
nothing happened.

Regstrapping on amd64-linux.

2011-07-27  Sebastian Pop  <sebastian.pop@amd.com>

	PR tree-optimization/49876
	* sese.c (rename_uses): Do not return false on gloog_error: set
	the new_expr to integer_zero_node and continue code generation.
	(graphite_copy_stmts_from_block): Remove early exit on gloog_error.
---
 gcc/ChangeLog |    7 +++++++
 gcc/sese.c    |   18 ++++++++----------
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index b07d494..a565c18 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,5 +1,12 @@
 2011-07-27  Sebastian Pop  <sebastian.pop@amd.com>
 
+	PR tree-optimization/49876
+	* sese.c (rename_uses): Do not return false on gloog_error: set
+	the new_expr to integer_zero_node and continue code generation.
+	(graphite_copy_stmts_from_block): Remove early exit on gloog_error.
+
+2011-07-27  Sebastian Pop  <sebastian.pop@amd.com>
+
 	PR tree-optimization/49471
 	* tree-ssa-loop-manip.c (canonicalize_loop_ivs): Build an unsigned
 	iv only when the largest type is unsigned.  Do not call
diff --git a/gcc/sese.c b/gcc/sese.c
index ec96dfb..04a8e75 100644
--- a/gcc/sese.c
+++ b/gcc/sese.c
@@ -527,10 +527,10 @@ rename_uses (gimple copy, htab_t rename_map, gimple_stmt_iterator *gsi_tgt,
       if (chrec_contains_undetermined (scev))
 	{
 	  *gloog_error = true;
-	  return false;
+	  new_expr = integer_zero_node;
 	}
-
-      new_expr = chrec_apply_map (scev, iv_map);
+      else
+	new_expr = chrec_apply_map (scev, iv_map);
 
       /* The apply should produce an expression tree containing
 	 the uses of the new induction variables.  We should be
@@ -540,12 +540,13 @@ rename_uses (gimple copy, htab_t rename_map, gimple_stmt_iterator *gsi_tgt,
 	  || tree_contains_chrecs (new_expr, NULL))
 	{
 	  *gloog_error = true;
-	  return false;
+	  new_expr = integer_zero_node;
 	}
+      else
+	/* Replace the old_name with the new_expr.  */
+	new_expr = force_gimple_operand (unshare_expr (new_expr), &stmts,
+					 true, NULL_TREE);
 
-      /* Replace the old_name with the new_expr.  */
-      new_expr = force_gimple_operand (unshare_expr (new_expr), &stmts,
-				       true, NULL_TREE);
       gsi_insert_seq_before (gsi_tgt, stmts, GSI_SAME_STMT);
       replace_exp (use_p, new_expr);
 
@@ -621,9 +622,6 @@ graphite_copy_stmts_from_block (basic_block bb, basic_block new_bb,
 		       gloog_error))
 	fold_stmt_inplace (copy);
 
-      if (*gloog_error)
-	break;
-
       update_stmt (copy);
     }
 }
-- 
1.7.4.1

Re: [PATCH] Fix PR49876: Continue code generation with integer_zero_node on gloog_error

[Richard Guenther]

On Wed, Jul 27, 2011 at 8:49 PM, Sebastian Pop <sebpop@gmail.com> wrote:
> When setting gloog_error, graphite should continue code generation
> without early returns, as otherwise the SSA representation would not
> be complete.  So set the new expression to integer_zero_node, that
> would not require more SSA updates, and continue code generation as
> nothing happened.

I suppose you have to watch for correct types?  Or does the code get
discarded again before it eventually reaches the verifier?  Ok in that case.

Thanks,
Richard.

> Regstrapping on amd64-linux.
>
> 2011-07-27  Sebastian Pop  <sebastian.pop@amd.com>
>
>        PR tree-optimization/49876
>        * sese.c (rename_uses): Do not return false on gloog_error: set
>        the new_expr to integer_zero_node and continue code generation.
>        (graphite_copy_stmts_from_block): Remove early exit on gloog_error.
> ---
>  gcc/ChangeLog |    7 +++++++
>  gcc/sese.c    |   18 ++++++++----------
>  2 files changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index b07d494..a565c18 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,5 +1,12 @@
>  2011-07-27  Sebastian Pop  <sebastian.pop@amd.com>
>
> +       PR tree-optimization/49876
> +       * sese.c (rename_uses): Do not return false on gloog_error: set
> +       the new_expr to integer_zero_node and continue code generation.
> +       (graphite_copy_stmts_from_block): Remove early exit on gloog_error.
> +
> +2011-07-27  Sebastian Pop  <sebastian.pop@amd.com>
> +
>        PR tree-optimization/49471
>        * tree-ssa-loop-manip.c (canonicalize_loop_ivs): Build an unsigned
>        iv only when the largest type is unsigned.  Do not call
> diff --git a/gcc/sese.c b/gcc/sese.c
> index ec96dfb..04a8e75 100644
> --- a/gcc/sese.c
> +++ b/gcc/sese.c
> @@ -527,10 +527,10 @@ rename_uses (gimple copy, htab_t rename_map, gimple_stmt_iterator *gsi_tgt,
>       if (chrec_contains_undetermined (scev))
>        {
>          *gloog_error = true;
> -         return false;
> +         new_expr = integer_zero_node;
>        }
> -
> -      new_expr = chrec_apply_map (scev, iv_map);
> +      else
> +       new_expr = chrec_apply_map (scev, iv_map);
>
>       /* The apply should produce an expression tree containing
>         the uses of the new induction variables.  We should be
> @@ -540,12 +540,13 @@ rename_uses (gimple copy, htab_t rename_map, gimple_stmt_iterator *gsi_tgt,
>          || tree_contains_chrecs (new_expr, NULL))
>        {
>          *gloog_error = true;
> -         return false;
> +         new_expr = integer_zero_node;
>        }
> +      else
> +       /* Replace the old_name with the new_expr.  */
> +       new_expr = force_gimple_operand (unshare_expr (new_expr), &stmts,
> +                                        true, NULL_TREE);
>
> -      /* Replace the old_name with the new_expr.  */
> -      new_expr = force_gimple_operand (unshare_expr (new_expr), &stmts,
> -                                      true, NULL_TREE);
>       gsi_insert_seq_before (gsi_tgt, stmts, GSI_SAME_STMT);
>       replace_exp (use_p, new_expr);
>
> @@ -621,9 +622,6 @@ graphite_copy_stmts_from_block (basic_block bb, basic_block new_bb,
>                       gloog_error))
>        fold_stmt_inplace (copy);
>
> -      if (*gloog_error)
> -       break;
> -
>       update_stmt (copy);
>     }
>  }
> --
> 1.7.4.1
>
>

Re: [PATCH] Fix PR49876: Continue code generation with integer_zero_node on gloog_error

[Sebastian Pop]

On Thu, Jul 28, 2011 at 09:49, Richard Guenther <rguenther@suse.de> wrote:
> And it's always integers or pointers only?  Otherwise you'd probably
> want build_zero_cst (type) instead.

Ok, I started regstrapping again with build_zero_cst.

Thanks,
Sebastian

Re: [PATCH] Fix PR49876: Continue code generation with integer_zero_node on gloog_error

[Sebastian Pop]

[-- Attachment #1: Type: text/plain, Size: 754 bytes --]

On Thu, Jul 28, 2011 at 02:58, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Wed, Jul 27, 2011 at 8:49 PM, Sebastian Pop <sebpop@gmail.com> wrote:
>> When setting gloog_error, graphite should continue code generation
>> without early returns, as otherwise the SSA representation would not
>> be complete.  So set the new expression to integer_zero_node, that
>> would not require more SSA updates, and continue code generation as
>> nothing happened.
>
> I suppose you have to watch for correct types?  Or does the code get
> discarded again before it eventually reaches the verifier?  Ok in that case.

Attached is the amended patch using build_int_cst.
Thanks for pointing this type problem.
Regstrapping again.

Sebastian

[-- Attachment #2: 0001-Fix-PR49876-Continue-code-generation-with-integer_ze.patch --]
[-- Type: text/x-diff, Size: 3077 bytes --]

From 1ce1367e3a0439b374025104eb2e7558c9b957fc Mon Sep 17 00:00:00 2001
From: Sebastian Pop <sebpop@gmail.com>
Date: Wed, 27 Jul 2011 13:42:29 -0500
Subject: [PATCH] Fix PR49876: Continue code generation with integer_zero_node on gloog_error

When setting gloog_error, graphite should continue code generation
without early returns, as otherwise the SSA representation would not
be complete.  So set the new expression to integer_zero_node, that
would not require more SSA updates, and continue code generation as
nothing happened.

Regstrapping on amd64-linux.

2011-07-27  Sebastian Pop  <sebastian.pop@amd.com>

	PR tree-optimization/49876
	* sese.c (rename_uses): Do not return false on gloog_error: set
	the new_expr to integer_zero_node and continue code generation.
	(graphite_copy_stmts_from_block): Remove early exit on gloog_error.
---
 gcc/ChangeLog |    7 +++++++
 gcc/sese.c    |   18 ++++++++----------
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index b07d494..a565c18 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,5 +1,12 @@
 2011-07-27  Sebastian Pop  <sebastian.pop@amd.com>
 
+	PR tree-optimization/49876
+	* sese.c (rename_uses): Do not return false on gloog_error: set
+	the new_expr to integer_zero_node and continue code generation.
+	(graphite_copy_stmts_from_block): Remove early exit on gloog_error.
+
+2011-07-27  Sebastian Pop  <sebastian.pop@amd.com>
+
 	PR tree-optimization/49471
 	* tree-ssa-loop-manip.c (canonicalize_loop_ivs): Build an unsigned
 	iv only when the largest type is unsigned.  Do not call
diff --git a/gcc/sese.c b/gcc/sese.c
index ec96dfb..cd92527 100644
--- a/gcc/sese.c
+++ b/gcc/sese.c
@@ -527,10 +527,10 @@ rename_uses (gimple copy, htab_t rename_map, gimple_stmt_iterator *gsi_tgt,
       if (chrec_contains_undetermined (scev))
 	{
 	  *gloog_error = true;
-	  return false;
+	  new_expr = build_int_cst (TREE_TYPE (old_name), 0);
 	}
-
-      new_expr = chrec_apply_map (scev, iv_map);
+      else
+	new_expr = chrec_apply_map (scev, iv_map);
 
       /* The apply should produce an expression tree containing
 	 the uses of the new induction variables.  We should be
@@ -540,12 +540,13 @@ rename_uses (gimple copy, htab_t rename_map, gimple_stmt_iterator *gsi_tgt,
 	  || tree_contains_chrecs (new_expr, NULL))
 	{
 	  *gloog_error = true;
-	  return false;
+	  new_expr = build_int_cst (TREE_TYPE (old_name), 0);
 	}
+      else
+	/* Replace the old_name with the new_expr.  */
+	new_expr = force_gimple_operand (unshare_expr (new_expr), &stmts,
+					 true, NULL_TREE);
 
-      /* Replace the old_name with the new_expr.  */
-      new_expr = force_gimple_operand (unshare_expr (new_expr), &stmts,
-				       true, NULL_TREE);
       gsi_insert_seq_before (gsi_tgt, stmts, GSI_SAME_STMT);
       replace_exp (use_p, new_expr);
 
@@ -621,9 +622,6 @@ graphite_copy_stmts_from_block (basic_block bb, basic_block new_bb,
 		       gloog_error))
 	fold_stmt_inplace (copy);
 
-      if (*gloog_error)
-	break;
-
       update_stmt (copy);
     }
 }
-- 
1.7.4.1