diff mbox

[gomp-4_0-branch] misc reduction clause bug fixes

Message ID 53BEDEBF.7020106@codesourcery.com
State New
Headers show

Commit Message

Cesar Philippidis July 10, 2014, 6:43 p.m. UTC 
Hi Thomas,

This patch addresses two bugs openacc reduction clause bugs. The first
bug occurred because I didn't anticipate a GIMPLE_BIND stmt to be passed
to process_reduction_data. Turns out, this could happen with the
collapse clause. That's because the variables which were declared inside
the inner loops need to be declared outside of the collapsed loop nest.

The second issue is that process_reduction_clause adds unnecessary code
if a reduction clause isn't present. The patch prevents that from happening.

Is this OK for gomp-4_0-branch? I didn't include any test cases, because
these bugs were exposed by the collapse clause patch.

Thanks,
Cesar

Comments

Thomas Schwinge July 11, 2014, 9:15 a.m. UTC | #1 
Hi Cesar!

On Thu, 10 Jul 2014 11:43:11 -0700, Cesar Philippidis <cesar@codesourcery.com> wrote:
> This patch addresses two bugs openacc reduction clause bugs.

Thanks!  OK; one question/suggestion, though:

> --- a/gcc/omp-low.c
> +++ b/gcc/omp-low.c
> @@ -9679,11 +9679,23 @@ process_reduction_data (gimple_seq *body, gimple_seq *in_stmt_seqp,
>    gcc_assert (is_gimple_omp_oacc_specifically (ctx->stmt));
>  
>    gimple_stmt_iterator gsi;
> +  gimple_seq inner;
> +  gimple stmt;
> +
> +  /* A collapse clause may have inserted a new bind block.  */
> +  stmt = gimple_seq_first (*body);
> +  if (stmt && gimple_code (stmt) == GIMPLE_BIND)
> +    {
> +      inner = gimple_bind_body (gimple_seq_first (*body));
> +      body = &inner;
> +    }
>  
>    for (gsi = gsi_start (*body); !gsi_end_p (gsi); gsi_next (&gsi))
>      {
>        gimple stmt = gsi_stmt (gsi);

Can get rid of this shadow variable stmt?


Grüße,
 Thomas
diff mbox

Patch

diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index 227ff1b..eb078b6 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -9679,11 +9679,23 @@  process_reduction_data (gimple_seq *body, gimple_seq *in_stmt_seqp,
   gcc_assert (is_gimple_omp_oacc_specifically (ctx->stmt));
 
   gimple_stmt_iterator gsi;
+  gimple_seq inner;
+  gimple stmt;
+
+  /* A collapse clause may have inserted a new bind block.  */
+  stmt = gimple_seq_first (*body);
+  if (stmt && gimple_code (stmt) == GIMPLE_BIND)
+    {
+      inner = gimple_bind_body (gimple_seq_first (*body));
+      body = &inner;
+    }
 
   for (gsi = gsi_start (*body); !gsi_end_p (gsi); gsi_next (&gsi))
     {
       gimple stmt = gsi_stmt (gsi);
       tree call;
+      tree clauses, nthreads, t, c;
+      bool reduction_found = false;
 
       switch (gimple_code (stmt))
 	{
@@ -9691,6 +9703,18 @@  process_reduction_data (gimple_seq *body, gimple_seq *in_stmt_seqp,
 	  tree clauses, nthreads, t;
 
 	  clauses = gimple_omp_for_clauses (stmt);
+
+	  /* Search for a reduction clause.  */
+	  for (c = clauses; c; c = OMP_CLAUSE_CHAIN (c))
+	    if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_REDUCTION)
+	      {
+		reduction_found = true;
+		break;
+	      }
+
+	  if (!reduction_found)
+	    break;
+
 	  ctx = maybe_lookup_ctx (stmt);
 	  t = NULL_TREE;
 
@@ -9698,8 +9722,6 @@  process_reduction_data (gimple_seq *body, gimple_seq *in_stmt_seqp,
 	     Scan for the innermost vector_length clause.  */
 	  for (omp_context *oc = ctx; oc; oc = oc->outer)
 	    {
-	      tree c;
-
 	      switch (gimple_code (oc->stmt))
 		{
 		case GIMPLE_OACC_PARALLEL: