From patchwork Sat Sep 25 21:33:06 2010
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: Fix thinko in GIMPLE inliner
X-Patchwork-Submitter: Eric Botcazou <ebotcazou@adacore.com>
X-Patchwork-Id: 65767
Message-Id: <201009252333.06163.ebotcazou@adacore.com>
To: gcc-patches@gcc.gnu.org
Date: Sat, 25 Sep 2010 23:33:06 +0200
From: Eric Botcazou <ebotcazou@adacore.com>
List-Id: <gcc-patches.gcc.gnu.org>

I've attached a testcase that, when compiled with our 4.5-based compiler at -O 
or above, runs into a thinko in the GIMPLE inliner.  However, because of new 
code on the mainline (mem-ref branch) and too ancient code in gigi on the 4.5 
branch, it doesn't run into this thinko with FSF compilers.  But the issue is 
latent in them too.

tree-inline.c:copy_bb reads:

      seq_gsi = copy_gsi;

      /* With return slot optimization we can end up with
	 non-gimple (foo *)&this->m, fix that here.  */
      if (is_gimple_assign (stmt)
	  && gimple_assign_rhs_code (stmt) == NOP_EXPR
	  && !is_gimple_val (gimple_assign_rhs1 (stmt)))
	{
	  tree new_rhs;
	  new_rhs = force_gimple_operand_gsi (&seq_gsi,
					      gimple_assign_rhs1 (stmt),
					      true, NULL, false, GSI_NEW_STMT);
	  gimple_assign_set_rhs1 (stmt, new_rhs);
	  id->regimplify = false;
	}

      gsi_insert_after (&seq_gsi, stmt, GSI_NEW_STMT);


The problem is the first GSI_NEW_STMT because:

  GSI_NEW_STMT,		/* Only valid when single statement is added, move
			   iterator to it.  */

so if force_gimple_operand_gsi happens to create more than one statement (this 
will occur when the LHS of the return statement is a VIEW_CONVERT_EXPR), the 
seq_gsi iterator will be moved to the first statement.  As a consequence, the 
modified STMT will be inserted between the first and the second newly created 
statements, wreaking havoc in the SSA form.

Fixed by using GSI_CONTINUE_LINKING instead of GSI_NEW_STMT.  Bootstrapped and 
regtested on x86_64-suse-linux, applied on mainline and 4.5 branch as obvious.


2010-09-25  Eric Botcazou  <ebotcazou@adacore.com>

	* tree-inline.c (copy_bb): Use GSI_CONTINUE_LINKING when inserting new
	statements because of the return slot optimization.


2010-09-25  Eric Botcazou  <ebotcazou@adacore.com>

	* gnat.dg/return2.ad[sb]: New test.
	* gnat.dg/return2_pkg.ads: New helper.

Index: tree-inline.c
===================================================================
--- tree-inline.c	(revision 164617)
+++ tree-inline.c	(working copy)
@@ -1558,7 +1558,8 @@ copy_bb (copy_body_data *id, basic_block
 	  tree new_rhs;
 	  new_rhs = force_gimple_operand_gsi (&seq_gsi,
 					      gimple_assign_rhs1 (stmt),
-					      true, NULL, false, GSI_NEW_STMT);
+					      true, NULL, false,
+					      GSI_CONTINUE_LINKING);
 	  gimple_assign_set_rhs1 (stmt, new_rhs);
 	  id->regimplify = false;
 	}