[PATCH, PR 42006] Dont mark replacement variables for renaming

[PATCH, PR 42006] Dont mark replacement variables for renaming

[Martin Jambor]

Hi,

in PR 42006, IPA-SRA removes an unused scalar parameter because the
value passed to it is not used.  However, the parameter is used to
drive a loop and so it needs to be replaced by a variable.  IPA-SRA
currently creates that variable with make_rename_temp which marks it
for renaming but then proceeds with creating SSA_NAMEs on its own,
replacing one for one.  For some reason, the (unnecessary) renaming
then messes this up.  I even believe this might be a known "problem."

If I just use create_tmp_var (and then mark vectors and complex
numbers as gimple registers) the problem goes away and it is probably
the right thing to do anyway, so this is what the patch below does.

Bootstrapped and tested on x86_64-linux. OK for trunk?

Thanks,

Martin


2009-11-27  Martin Jambor  <mjambor@suse.cz>

	PR middle-end/42006
	* tree-sra.c (get_replaced_param_substitute): Call create_tmp_var
	instead of create_tmp_var.  Set DECL_GIMPLE_REG_P to one manually
	for vector and complex types.
	(get_adjustment_for_base): Describe return value in the comment.

	* testsuite/gcc.c-torture/execute/pr42006.c: New test.

Index: mine/gcc/tree-sra.c
===================================================================
--- mine.orig/gcc/tree-sra.c
+++ mine/gcc/tree-sra.c
@@ -3478,7 +3478,10 @@ get_replaced_param_substitute (struct ip
     {
       char *pretty_name = make_fancy_name (adj->base);
 
-      repl = make_rename_temp (TREE_TYPE (adj->base), "ISR");
+      repl = create_tmp_var (TREE_TYPE (adj->base), "ISR");
+      if (TREE_CODE (TREE_TYPE (repl)) == COMPLEX_TYPE
+	  || TREE_CODE (TREE_TYPE (repl)) == VECTOR_TYPE)
+	DECL_GIMPLE_REG_P (repl) = 1;
       DECL_NAME (repl) = get_identifier (pretty_name);
       obstack_free (&name_obstack, pretty_name);
 
@@ -3516,7 +3519,8 @@ get_adjustment_for_base (ipa_parm_adjust
 /* Callback for scan_function.  If the statement STMT defines an SSA_NAME of a
    parameter which is to be removed because its value is not used, replace the
    SSA_NAME with a one relating to a created VAR_DECL and replace all of its
-   uses too.  DATA is a pointer to an adjustments vector.  */
+   uses too and return true (update_stmt is then issued for the statement by
+   the caller).  DATA is a pointer to an adjustments vector.  */
 
 static bool
 replace_removed_params_ssa_names (gimple stmt, void *data)
Index: mine/gcc/testsuite/gcc.c-torture/execute/pr42006.c
===================================================================
--- /dev/null
+++ mine/gcc/testsuite/gcc.c-torture/execute/pr42006.c
@@ -0,0 +1,33 @@
+extern void abort (void);
+
+static unsigned int
+my_add(unsigned int si1, unsigned int si2)
+{
+  return (si1 > (50-si2)) ? si1 : (si1 + si2);
+}
+
+static unsigned int
+my_shift(unsigned int left, unsigned int right)
+{
+  return  (right > 100) ? left : (left >> right);
+}
+
+static int func_4(unsigned int p_6)
+{
+  int count = 0;
+  for (p_6 = 1; p_6 < 3; p_6 = my_add(p_6, 1))
+    {
+      if (count++ > 1)
+	abort ();
+
+      if (my_shift(p_6, p_6))
+	return 0;
+    }
+  return 0;
+}
+
+int main(void)
+{
+  func_4(0);
+  return 0;
+}

Re: [PATCH, PR 42006] Dont mark replacement variables for renaming

[Richard Guenther]

On Fri, 27 Nov 2009, Martin Jambor wrote:

> Hi,
> 
> in PR 42006, IPA-SRA removes an unused scalar parameter because the
> value passed to it is not used.  However, the parameter is used to
> drive a loop and so it needs to be replaced by a variable.  IPA-SRA
> currently creates that variable with make_rename_temp which marks it
> for renaming but then proceeds with creating SSA_NAMEs on its own,
> replacing one for one.  For some reason, the (unnecessary) renaming
> then messes this up.  I even believe this might be a known "problem."
> 
> If I just use create_tmp_var (and then mark vectors and complex
> numbers as gimple registers) the problem goes away and it is probably
> the right thing to do anyway, so this is what the patch below does.

Um, yeah.  You can't just arbitrarily rename things.

> Bootstrapped and tested on x86_64-linux. OK for trunk?

Ok.

Thanks,
Richard.

> Thanks,
> 
> Martin
> 
> 
> 2009-11-27  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR middle-end/42006
> 	* tree-sra.c (get_replaced_param_substitute): Call create_tmp_var
> 	instead of create_tmp_var.  Set DECL_GIMPLE_REG_P to one manually
> 	for vector and complex types.
> 	(get_adjustment_for_base): Describe return value in the comment.
> 
> 	* testsuite/gcc.c-torture/execute/pr42006.c: New test.
> 
> Index: mine/gcc/tree-sra.c
> ===================================================================
> --- mine.orig/gcc/tree-sra.c
> +++ mine/gcc/tree-sra.c
> @@ -3478,7 +3478,10 @@ get_replaced_param_substitute (struct ip
>      {
>        char *pretty_name = make_fancy_name (adj->base);
>  
> -      repl = make_rename_temp (TREE_TYPE (adj->base), "ISR");
> +      repl = create_tmp_var (TREE_TYPE (adj->base), "ISR");
> +      if (TREE_CODE (TREE_TYPE (repl)) == COMPLEX_TYPE
> +	  || TREE_CODE (TREE_TYPE (repl)) == VECTOR_TYPE)
> +	DECL_GIMPLE_REG_P (repl) = 1;
>        DECL_NAME (repl) = get_identifier (pretty_name);
>        obstack_free (&name_obstack, pretty_name);
>  
> @@ -3516,7 +3519,8 @@ get_adjustment_for_base (ipa_parm_adjust
>  /* Callback for scan_function.  If the statement STMT defines an SSA_NAME of a
>     parameter which is to be removed because its value is not used, replace the
>     SSA_NAME with a one relating to a created VAR_DECL and replace all of its
> -   uses too.  DATA is a pointer to an adjustments vector.  */
> +   uses too and return true (update_stmt is then issued for the statement by
> +   the caller).  DATA is a pointer to an adjustments vector.  */
>  
>  static bool
>  replace_removed_params_ssa_names (gimple stmt, void *data)
> Index: mine/gcc/testsuite/gcc.c-torture/execute/pr42006.c
> ===================================================================
> --- /dev/null
> +++ mine/gcc/testsuite/gcc.c-torture/execute/pr42006.c
> @@ -0,0 +1,33 @@
> +extern void abort (void);
> +
> +static unsigned int
> +my_add(unsigned int si1, unsigned int si2)
> +{
> +  return (si1 > (50-si2)) ? si1 : (si1 + si2);
> +}
> +
> +static unsigned int
> +my_shift(unsigned int left, unsigned int right)
> +{
> +  return  (right > 100) ? left : (left >> right);
> +}
> +
> +static int func_4(unsigned int p_6)
> +{
> +  int count = 0;
> +  for (p_6 = 1; p_6 < 3; p_6 = my_add(p_6, 1))
> +    {
> +      if (count++ > 1)
> +	abort ();
> +
> +      if (my_shift(p_6, p_6))
> +	return 0;
> +    }
> +  return 0;
> +}
> +
> +int main(void)
> +{
> +  func_4(0);
> +  return 0;
> +}
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex