diff mbox

Regimplification enhancements 3/3

Message ID 539ECD9C.3020302@codesourcery.com
State New
Headers show

Commit Message

Bernd Schmidt June 16, 2014, 10:57 a.m. UTC 
There's code in regimplification that makes us use an extra temporary
when we encounter a call returning a non-BLKmode structure. This seems
somewhat inefficient and unnecessary, and when used from the
lower-addr-spaces pass I'm working on it leads to problems further
down that look like tree-ssa bugs that I wasn't able to clearly
disentangle.

Here's what happens on compile/pr51761.c.  Regimplification has the
following effect, creating an extra temporary _6:

-  D.1378 = fooD.1373 (aD.1377);
+  _6 = fooD.1373 (aD.1377);
+  # .MEMD.1382 = VDEF <.MEMD.1382>
+  D.1378 = _6;

SRA turns this into:

   _6 = fooD.1373 (aD.1377);
   # VUSE <.MEM_3>
   SR$2_7 = MEM[(struct S *)&_6];

Somehow, the address of &_6 doesn't count as a use, and the DCE pass 
decides it is unused:

   Eliminating unnecessary statements:
   Deleting LHS of call: _6 = foo (a);

However, the statement
   SR$2_7 = MEM[(struct S *)&_6];
is still present, and we have an SSA name without a definition, leading 
to a crash.

Rather than figure all this out, I decided to try making the 
regimplification not generate the extra copy in the first place. The 
testsuite seems to agree with me that it's unnecessary. Bootstrapped and 
tested on x86_64-linux, ok?


Bernd

Comments

Richard Biener June 16, 2014, 11:38 a.m. UTC | #1 
On Mon, Jun 16, 2014 at 12:57 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
> There's code in regimplification that makes us use an extra temporary
> when we encounter a call returning a non-BLKmode structure. This seems
> somewhat inefficient and unnecessary, and when used from the
> lower-addr-spaces pass I'm working on it leads to problems further
> down that look like tree-ssa bugs that I wasn't able to clearly
> disentangle.
>
> Here's what happens on compile/pr51761.c.  Regimplification has the
> following effect, creating an extra temporary _6:
>
> -  D.1378 = fooD.1373 (aD.1377);
> +  _6 = fooD.1373 (aD.1377);
> +  # .MEMD.1382 = VDEF <.MEMD.1382>
> +  D.1378 = _6;
>
> SRA turns this into:
>
>   _6 = fooD.1373 (aD.1377);
>   # VUSE <.MEM_3>
>   SR$2_7 = MEM[(struct S *)&_6];

clearly bogus - _6 is a register, you can't use a MEM on it.

> Somehow, the address of &_6 doesn't count as a use, and the DCE pass decides
> it is unused:
>
>   Eliminating unnecessary statements:
>   Deleting LHS of call: _6 = foo (a);
>
> However, the statement
>   SR$2_7 = MEM[(struct S *)&_6];
> is still present, and we have an SSA name without a definition, leading to a
> crash.
>
> Rather than figure all this out, I decided to try making the
> regimplification not generate the extra copy in the first place. The
> testsuite seems to agree with me that it's unnecessary. Bootstrapped and
> tested on x86_64-linux, ok?

Ok.  The code looks bogus anyway in that it generates a SSA name
for sth not is_gimple_reg_type ().

Thanks,
Richard.

>
> Bernd
diff mbox

Patch

    	* gimplify-me.c (gimple_regimplify_operands): Avoid unnecessary
    	temporary for struct returns.

diff --git a/gcc/gimplify-me.c b/gcc/gimplify-me.c
index 05eaeb0..2d1181b 100644
--- a/gcc/gimplify-me.c
+++ b/gcc/gimplify-me.c
@@ -304,25 +304,9 @@  gimple_regimplify_operands (gimple stmt, gimple_stmt_iterator *gsi_p)
 		    need_temp = true;
 		}
 	    }
-	  else
-	    {
-	      if (is_gimple_reg_type (TREE_TYPE (lhs)))
-		need_temp = true;
-	      else if (TYPE_MODE (TREE_TYPE (lhs)) != BLKmode)
-		{
-		  if (is_gimple_call (stmt))
-		    {
-		      tree fndecl = gimple_call_fndecl (stmt);
+	  else if (is_gimple_reg_type (TREE_TYPE (lhs)))
+	    need_temp = true;
 
-		      if (!aggregate_value_p (TREE_TYPE (lhs), fndecl)
-			  && !(fndecl && DECL_RESULT (fndecl)
-			       && DECL_BY_REFERENCE (DECL_RESULT (fndecl))))
-			need_temp = true;
-		    }
-		  else
-		    need_temp = true;
-		}
-	    }
 	  if (need_temp)
 	    {
 	      tree temp = create_tmp_reg (TREE_TYPE (lhs), NULL);