This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [gomp4] Remove device-specific filtering during parsing for OpenACC


On Fri, 17 Jul 2015 14:57:14 +0200
Thomas Schwinge <thomas@codesourcery.com> wrote:

> In combination with the equivant change to
> gcc/cp/parser.c:cp_parser_oacc_all_clauses,
> gcc/c-family/c-omp.c:c_oacc_filter_device_types, and transitively also
> the struct identifier_hasher and c_oacc_extract_device_id function
> preceding it, are now unused.  (Not an exhaustive list; have not
> checked which other auxilliary functions etc. Cesar has added in his
> device_type changes.)  Does it make any sense to keep these for
> later, or dump them now?

The attached patch removes this dead code...

> > --- a/gcc/c/c-typeck.c
> > +++ b/gcc/c/c-typeck.c
> > @@ -12568,6 +12568,10 @@ c_finish_omp_clauses (tree clauses, bool
> > oacc) pc = &OMP_CLAUSE_CHAIN (c);
> >  	  continue;
> >  
> > +        case OMP_CLAUSE_DEVICE_TYPE:
> > +	  pc = &OMP_CLAUSE_DEVICE_TYPE_CLAUSES (c);
> > +	  continue;
> > +
> >  	case OMP_CLAUSE_INBRANCH:
> >  	case OMP_CLAUSE_NOTINBRANCH:
> >  	  if (branch_seen)
> 
> From a quick glance only, this seems to be different from the C++
> front end (have not checked Fortran).
> 
> I have not looked at what the front end parsing is now actually
> doing; is it just attaching any clauses following a device_type
> clause to the latter?  (The same should be done for all front ends,
> obviously.  Even if it's not important right now, because of the
> sorry diagnostic that will be emitted later on as soon as there is
> one device_type clause, this should best be addressed now, while you
> still remember what's going on here ;-) so that there will be no bad
> surprises once we actually implement the handling in OMP
> lowering/streaming/device compilers.)
> 
> Do we need manually need to take care to
> "finalize" (c_finish_omp_clauses et al.) such "masked" clause chains,
> or will the right thing happen automatically?

...and fixes the C and C++ frontend to "finalize" parsed
device_type clauses properly (although so far "finalization" doesn't do
anything for the clauses that can be associated with a device_type
clause anyway, so there's no actual change in behaviour).

I haven't moved the "sorry" reporting for the unsupported device_type
clause to scan_sharing_clauses because it doesn't seem to be
particularly a more logical place, and doing so breaks the tests that
scan the omp-low dumps.

I will apply to gomp4 branch as obvious, shortly.

Thanks,

Julian

ChangeLog

    gcc/
    * c-family/c-omp.c (c_oacc_extract_device_id, identifier_hasher)
    (c_oacc_filter_device_types): Remove dead code.
    * c/c-typeck.c (c_finish_omp_clauses): Add scanning for sub-clauses
    of device_type clause.
    * cp/semantics.c (finish_omp_clauses): Likewise.
commit e24a9cd14d4b8b5dab8b37218b29844787809648
Author: Julian Brown <julian@codesourcery.com>
Date:   Mon Jul 27 07:31:10 2015 -0700

    Clause finalization cleanups and dead code removal.

diff --git a/gcc/c-family/c-omp.c b/gcc/c-family/c-omp.c
index b76de69..10190d7 100644
--- a/gcc/c-family/c-omp.c
+++ b/gcc/c-family/c-omp.c
@@ -1081,132 +1081,6 @@ c_omp_predetermined_sharing (tree decl)
   return OMP_CLAUSE_DEFAULT_UNSPECIFIED;
 }
 
-/* Return a numerical code representing the device_type.  Currently,
-   only device_type(nvidia) is supported.  All device_type parameters
-   are treated as case-insensitive keywords.  */
-
-static int
-c_oacc_extract_device_id (const char *device)
-{
-  if (!strcasecmp (device, "nvidia"))
-    return GOMP_DEVICE_NVIDIA_PTX;
-  else if (!strcmp (device, "*"))
-    return GOMP_DEVICE_DEFAULT;
-  return GOMP_DEVICE_NONE;
-}
-
-struct identifier_hasher : ggc_cache_ptr_hash<tree_node>
-{
-  static hashval_t hash (tree t) { return htab_hash_pointer (t); }
-  static bool equal (tree a, tree b)
-  {
-    return !strcmp(IDENTIFIER_POINTER (a), IDENTIFIER_POINTER (b));
-  }
-};
-
-/* Filter out the list of unsupported OpenACC device_types.  */
-
-tree
-c_oacc_filter_device_types (tree clauses)
-{
-  tree c, prev;
-  tree dtype = NULL_TREE;
-  tree seen_nvidia = NULL_TREE;
-  tree seen_default = NULL_TREE;
-  hash_table<identifier_hasher> *dt_htab
-    = hash_table<identifier_hasher>::create_ggc (10);
-
-  /* First scan for all device_type clauses.  */
-  for (c = clauses; c; c = OMP_CLAUSE_CHAIN (c))
-    {
-      if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_DEVICE_TYPE)
-	{
-	  tree t;
-
-	  for (t = OMP_CLAUSE_DEVICE_TYPE_DEVICES (c); t; t = TREE_CHAIN (t))
-	    {
-	      if (dt_htab->find (t))
-		{
-		  error_at (OMP_CLAUSE_LOCATION (c),
-			    "duplicate device_type (%s)",
-			    IDENTIFIER_POINTER (t));
-		  goto filter_dtype;
-		}
-
-	      int code = c_oacc_extract_device_id (IDENTIFIER_POINTER (t));
-
-	      if (code == GOMP_DEVICE_DEFAULT)
-		seen_default = OMP_CLAUSE_DEVICE_TYPE_CLAUSES (c);
-	      else if (code == GOMP_DEVICE_NVIDIA_PTX)
-		seen_nvidia = OMP_CLAUSE_DEVICE_TYPE_CLAUSES (c);
-	      else
-		{
-		  /* The OpenACC technical committee advises compilers
-		     to silently ignore unknown devices.  */
-		}
-
-	      tree *slot = dt_htab->find_slot (t, INSERT);
-	      *slot = t;
-	    }
-	}
-    }
-
-  /* Don't do anything if there aren't any device_type clauses.  */
-  if (dt_htab->elements () == 0)
-    return clauses;
-
-  if (seen_nvidia)
-    dtype = seen_nvidia;
-  else if (seen_default)
-    dtype = seen_default;
-  else
-    goto filter_dtype;
-
-  dtype = seen_nvidia ? seen_nvidia : seen_default;
-
-  /* Now filter out clauses if necessary.  */
-  for (c = clauses; c && OMP_CLAUSE_CODE (c) != OMP_CLAUSE_DEVICE_TYPE;
-       c = OMP_CLAUSE_CHAIN (c))
-    {
-      tree t;
-
-      prev = NULL_TREE;
-
-      for (t = dtype; t; t = OMP_CLAUSE_CHAIN (t))
-	{
-	  if (OMP_CLAUSE_CODE (t) == OMP_CLAUSE_CODE (c))
-	    {
-	      /* Remove c from clauses.  */
-	      tree next = OMP_CLAUSE_CHAIN (c);
-
-	      if (prev)
-	        OMP_CLAUSE_CHAIN (prev) = next;
-
-	      break;
-	    }
-	}
-
-      prev = c;
-    }
-  
- filter_dtype:
-  /* Remove all device_type clauses.  Those clauses are located at the
-     beginning of the clause list.  */
-  for (c = clauses; c && OMP_CLAUSE_CODE (c) == OMP_CLAUSE_DEVICE_TYPE;
-       c = OMP_CLAUSE_CHAIN (c))
-    ;
-
-  if (c == NULL_TREE)
-    return dtype;
-
-  clauses = c;
-  for (prev = c, c = OMP_CLAUSE_CHAIN (c); c; c = OMP_CLAUSE_CHAIN (c))
-    prev = c;
-
-  OMP_CLAUSE_CHAIN (prev) = dtype;
-  return clauses;
-}
-
 /* Split the 'clauses' into a set of 'loop' clauses and a set of
    'not-loop' clauses.  */
 
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index dcc246c..36d027e 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -12569,7 +12569,9 @@ c_finish_omp_clauses (tree clauses, bool oacc)
 	  continue;
 
         case OMP_CLAUSE_DEVICE_TYPE:
-	  pc = &OMP_CLAUSE_DEVICE_TYPE_CLAUSES (c);
+	  OMP_CLAUSE_DEVICE_TYPE_CLAUSES (c)
+	    = c_finish_omp_clauses (OMP_CLAUSE_DEVICE_TYPE_CLAUSES (c), oacc);
+	  pc = &OMP_CLAUSE_CHAIN (c);
 	  continue;
 
 	case OMP_CLAUSE_INBRANCH:
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 1ce1dfa..c1983d4 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -5951,9 +5951,14 @@ finish_omp_clauses (tree clauses, bool oacc)
 	case OMP_CLAUSE_BIND:
 	case OMP_CLAUSE_NOHOST:
 	case OMP_CLAUSE_TILE:
-	case OMP_CLAUSE_DEVICE_TYPE:
 	  break;
 
+	case OMP_CLAUSE_DEVICE_TYPE:
+	  OMP_CLAUSE_DEVICE_TYPE_CLAUSES (c)
+	    = finish_omp_clauses (OMP_CLAUSE_DEVICE_TYPE_CLAUSES (c), oacc);
+	  pc = &OMP_CLAUSE_CHAIN (c);
+	  continue;
+
 	case OMP_CLAUSE_INBRANCH:
 	case OMP_CLAUSE_NOTINBRANCH:
 	  if (branch_seen)

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]