Patchwork [gomp4] rewrite simd clone argument adjustment to avoid regimplification

login
register
mail settings
Submitter Aldy Hernandez
Date Nov. 7, 2013, 3:17 p.m.
Message ID <527BAEF9.2080905@redhat.com>
Download mbox | patch
Permalink /patch/289383/
State New
Headers show

Comments

Aldy Hernandez - Nov. 7, 2013, 3:17 p.m.
On 11/07/13 00:38, Jakub Jelinek wrote:
> On Wed, Nov 06, 2013 at 05:37:03PM -0700, Aldy Hernandez wrote:
>> Hmmm, good point.  I've moved update_stmt and company to the caller,
>> and modified the caller to call regimplify_operands only for
>> GIMPLE_RETURN which is the special case.
>
> Can't you (later) handle that without regimplification too?

Sure!  See attached patch.

But as discussed on IRC, I wonder whether we can do without the 
following in the attached patch:

+		    tree repl = make_ssa_name (TREE_TYPE (retval), NULL);
+		    stmt = gimple_build_assign (repl, retval);
+		    gsi_insert_before (&gsi, stmt, GSI_SAME_STMT);
+		    stmt = gimple_build_assign (ref, repl);

...and unconditionally do:

+		  stmt = gimple_build_assign (ref, retval);

...since it seems all the GIMPLE_RETURNs I see can be replaced by 
``retval_array[iter] = whatever'' without creating something non-gimple 
(thus avoiding an SSA variable).

Either way, I'm ok.  Let me know.

Aldy
gcc/ChangeLog.gomp

	* omp-low.c (ipa_simd_modify_function_body): Avoid
	regimplification of GIMPLE_RETURNs.
Jakub Jelinek - Nov. 7, 2013, 3:23 p.m.
On Thu, Nov 07, 2013 at 08:17:13AM -0700, Aldy Hernandez wrote:
> But as discussed on IRC, I wonder whether we can do without the
> following in the attached patch:
> 
> +		    tree repl = make_ssa_name (TREE_TYPE (retval), NULL);
> +		    stmt = gimple_build_assign (repl, retval);
> +		    gsi_insert_before (&gsi, stmt, GSI_SAME_STMT);
> +		    stmt = gimple_build_assign (ref, repl);
> 
> ...and unconditionally do:
> 
> +		  stmt = gimple_build_assign (ref, retval);

This should be sufficient.
> 
> ...since it seems all the GIMPLE_RETURNs I see can be replaced by
> ``retval_array[iter] = whatever'' without creating something
> non-gimple (thus avoiding an SSA variable).
> 
> Either way, I'm ok.  Let me know.

Thanks.

	Jakub

Patch

diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index 14b8571..6039de9 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -11175,26 +11175,31 @@  ipa_simd_modify_function_body (struct cgraph_node *node,
 	    {
 	    case GIMPLE_RETURN:
 	      {
-		tree old_retval = gimple_return_retval (stmt);
-		if (old_retval)
+		tree retval = gimple_return_retval (stmt);
+		if (!retval)
 		  {
-		    /* Replace `return foo' by `retval_array[iter] = foo'.  */
-		    stmt = gimple_build_assign (build4 (ARRAY_REF,
-							TREE_TYPE (old_retval),
-							retval_array, iter,
-							NULL, NULL),
-						old_retval);
-		    gsi_replace (&gsi, stmt, true);
-		    gimple_regimplify_operands (stmt, &gsi);
-		    info.modified = true;
+		    gsi_remove (&gsi, true);
+		    continue;
 		  }
+
+		/* Replace `return foo' with `retval_array[iter] = foo'.  */
+		tree ref = build4 (ARRAY_REF,
+				   TREE_TYPE (retval),
+				   retval_array, iter,
+				   NULL, NULL);
+		if (is_gimple_reg (retval))
+		  stmt = gimple_build_assign (ref, retval);
 		else
 		  {
-		    gsi_remove (&gsi, true);
-		    continue;
+		    tree repl = make_ssa_name (TREE_TYPE (retval), NULL);
+		    stmt = gimple_build_assign (repl, retval);
+		    gsi_insert_before (&gsi, stmt, GSI_SAME_STMT);
+		    stmt = gimple_build_assign (ref, repl);
 		  }
-		break;
+		gsi_replace (&gsi, stmt, true);
+		info.modified = true;
 	      }
+	      break;
 
 	    default:
 	      walk_gimple_op (stmt, ipa_simd_modify_stmt_ops, &wi);