Re: [patch,gomp-4_0-branch] openacc parallel reduction part 1

[Thomas Schwinge <thomas at codesourcery dot com>]

Hi!

On Wed, 12 Nov 2014 16:58:54 +0100, I wrote:
> Hi Casar!
> 
> Can you please help me trying to understand the following change, that
> you've done a long time ago:
> 
> On Tue, 8 Jul 2014 07:28:24 -0700, Cesar Philippidis <cesar_philippidis@mentor.com> wrote:
> > On 07/07/2014 02:55 AM, Thomas Schwinge wrote:
> > 
> > > On Sun, 6 Jul 2014 16:10:56 -0700, Cesar Philippidis <cesar_philippidis@mentor.com> wrote:
> > >> This patch is the first step to enabling parallel reductions in openacc.
> 
> > --- a/gcc/omp-low.c
> > +++ b/gcc/omp-low.c
> 
> > @@ -1750,7 +1808,16 @@ scan_sharing_clauses (tree clauses, omp_context *ctx)
> >  		      && TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE)
> >  		    install_var_field (decl, true, 7, ctx);
> >  		  else
> > -		    install_var_field (decl, true, 3, ctx);
> > +		    {
> > +		      if (!is_gimple_omp_oacc_specifically (ctx->stmt))
> > +			install_var_field (decl, true, 3, ctx);
> > +		      else
> > +		    {
> > +		      /* decl goes heres.  */
> > +		      omp_context *c = (ctx->field_map ? ctx : ctx->outer);
> > +		      install_var_field (decl, true, 3, c);
> > +		    }
> > +		    }
> >  		  if (is_gimple_omp_offloaded (ctx->stmt))
> >  		    install_var_local (decl, ctx);
> >  		}
> 
> Writing your change differently, easier to read:
> 
>                       if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
>                           && OMP_CLAUSE_MAP_KIND (c) == OMP_CLAUSE_MAP_POINTER
>                           && !OMP_CLAUSE_MAP_ZERO_BIAS_ARRAY_SECTION (c)
>                           && TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE)
>                         install_var_field (decl, true, 7, ctx);
>     +                 else if (is_gimple_omp_oacc_specifically (ctx->stmt))
>     +                   {
>     +                     /* decl goes heres.  */
>     +                     omp_context *c = (ctx->field_map ? ctx : ctx->outer);
>     +                     install_var_field (decl, true, 3, c);
>     +                   }
>                       else
>                         install_var_field (decl, true, 3, ctx);
> 
> It is a generic OpenACC (that is, not only OpenACC reductions) code path
> that you're changing here.  Can you still come up with a rationale for
> that change, or should this possibly be restricted to OpenACC reductions
> processing only?

Cesar could neither remember, nor make up a new rationale on the spot;
trusting in regression testing, and that showing no issues, I have now
reverted this in r217462 on gomp-4_0-branch:

commit 389a8054007e619a917c16234cc697cae711de02
Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Thu Nov 13 08:13:33 2014 +0000

    Middle end: Revert earlier change.
    
    This change in OMP_CLAUSE_MAP handling had originally been applied for
    reductions support, but is now obsolete.
    
    	gcc/
    	* omp-low.c (scan_sharing_clauses) <OMP_CLAUSE_MAP>: Revert
    	earlier change.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/gomp-4_0-branch@217462 138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/ChangeLog.gomp |  5 +++++
 gcc/omp-low.c      | 11 +----------
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git gcc/ChangeLog.gomp gcc/ChangeLog.gomp
index 7501679..174235d 100644
--- gcc/ChangeLog.gomp
+++ gcc/ChangeLog.gomp
@@ -1,3 +1,8 @@
+2014-11-13  Thomas Schwinge  <thomas@codesourcery.com>
+
+	* omp-low.c (scan_sharing_clauses) <OMP_CLAUSE_MAP>: Revert
+	earlier change.
+
 2014-11-12  Cesar Philippidis  <cesar@codesourcery.com>
 
 	* tree.c (omp_clause_code_name): Add missing comma
diff --git gcc/omp-low.c gcc/omp-low.c
index 44e14b4..e511846 100644
--- gcc/omp-low.c
+++ gcc/omp-low.c
@@ -1902,16 +1902,7 @@ scan_sharing_clauses (tree clauses, omp_context *ctx)
 		      && TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE)
 		    install_var_field (decl, true, 7, ctx);
 		  else
-		    {
-		      if (!is_gimple_omp_oacc_specifically (ctx->stmt))
-			install_var_field (decl, true, 3, ctx);
-		      else
-		    {
-		      /* decl goes heres.  */
-		      omp_context *c = (ctx->field_map ? ctx : ctx->outer);
-		      install_var_field (decl, true, 3, c);
-		    }
-		    }
+		    install_var_field (decl, true, 3, ctx);
 		  if (is_gimple_omp_offloaded (ctx->stmt))
 		    install_var_local (decl, ctx);
 		}


GrÃÃe,
 Thomas

Attachment: pgppAILzYXULX.pgp
Description: PGP signature