Re: [gomp4.1] Handle new form of #pragma omp declare target

[Ilya Verbin <iverbin@gmail.com>]

On Fri, Oct 30, 2015 at 20:12:25 +0100, Jakub Jelinek wrote:
> On Fri, Oct 30, 2015 at 08:44:07PM +0300, Ilya Verbin wrote:
> > On Wed, Oct 28, 2015 at 00:11:03 +0300, Ilya Verbin wrote:
> > > On Fri, Jul 17, 2015 at 15:05:59 +0200, Jakub Jelinek wrote:
> > > > As the testcases show, #pragma omp declare target has now a new form (well,
> > > > two; with some issues on it pending), where it is used just as a single
> > > > declarative directive rather than a pair of them and allows marking
> > > > vars and functions by name as "omp declare target" vars/functions (which the
> > > > middle-end etc. already handles),
> > > 
> > > There is an issue - such variables are not added to the offloading tables,
> > > because when varpool_node::get_create is called for the first time, the variable
> > > doesn't yet have "omp declare target" attribute, and when it's called for the
> > > second time, it just returns existing node.  Functions also aren't marked as
> > > offloadable.  I tried to fix this by moving the code from
> > > varpool_node::get_create to varpool_node::finalize_decl, but it helped only C,
> > > but doesn't fix C++.  Therefore, I decided to iterate through all functions and
> > > variables, like in the patch bellow.  But it doesn't work for static vars,
> > > declared inside functions, because they do not appear in symtab :(
> > 
> > Ping?  Where should I set node->offloadable for "omp declare target to (list)"
> > functions, global and static vars?
> 
> Perhaps already somewhere in the FEs?  I mean, when the varpool node is
> created after the decl has that attribute, it already should set offsetable
> itself, so perhaps when adding the attribute check if corresponding varpool
> node exists already (but don't create it) and if yes, set offloadable?

Here is the patch.
make check RUNTESTFLAGS=gomp.exp and check-target-libgomp passed.
OK for gomp-4_5-branch?


gcc/c/
	* c-parser.c: Include context.h.
	(c_parser_omp_declare_target): If decl has "omp declare target" or
	"omp declare target link" attribute, and cgraph or varpool node already
	exists, then set corresponding flags.
gcc/cp/
	* parser.c: Include context.h.
	(cp_parser_omp_declare_target): If decl has "omp declare target" or
	"omp declare target link" attribute, and cgraph or varpool node already
	exists, then set corresponding flags.
libgomp/
	* testsuite/libgomp.c++/target-13.C: Add global variable with "omp
	declare target (<list>)" directive, use it in foo.
	* testsuite/libgomp.c/target-28.c: Likewise.


diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index a169457..049417c 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -67,6 +67,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "gomp-constants.h"
 #include "c-family/c-indentation.h"
 #include "gimple-expr.h"
+#include "context.h"
 
 \f
 /* Initialization routine for this file.  */
@@ -15600,7 +15601,22 @@ c_parser_omp_declare_target (c_parser *parser)
 	  continue;
 	}
       if (!at1)
-	DECL_ATTRIBUTES (t) = tree_cons (id, NULL_TREE, DECL_ATTRIBUTES (t));
+	{
+	  symtab_node *node = symtab_node::get (t);
+	  DECL_ATTRIBUTES (t) = tree_cons (id, NULL_TREE, DECL_ATTRIBUTES (t));
+	  if (node != NULL)
+	    {
+	      node->offloadable = 1;
+#ifdef ENABLE_OFFLOADING
+	      g->have_offload = true;
+	      if (is_a <varpool_node *> (node))
+		{
+		  vec_safe_push (offload_vars, t);
+		  node->force_output = 1;
+		}
+#endif
+	    }
+	}
     }
 }
 
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index a374e6c..de77a4b 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -49,6 +49,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "omp-low.h"
 #include "gomp-constants.h"
 #include "c-family/c-indentation.h"
+#include "context.h"
 
 \f
 /* The lexer.  */
@@ -34773,7 +34774,22 @@ cp_parser_omp_declare_target (cp_parser *parser, cp_token *pragma_tok)
 	  continue;
 	}
       if (!at1)
-	DECL_ATTRIBUTES (t) = tree_cons (id, NULL_TREE, DECL_ATTRIBUTES (t));
+	{
+	  symtab_node *node = symtab_node::get (t);
+	  DECL_ATTRIBUTES (t) = tree_cons (id, NULL_TREE, DECL_ATTRIBUTES (t));
+	  if (node != NULL)
+	    {
+	      node->offloadable = 1;
+#ifdef ENABLE_OFFLOADING
+	      g->have_offload = true;
+	      if (is_a <varpool_node *> (node))
+		{
+		  vec_safe_push (offload_vars, t);
+		  node->force_output = 1;
+		}
+#endif
+	    }
+	}
     }
 }
 
diff --git a/libgomp/testsuite/libgomp.c++/target-13.C b/libgomp/testsuite/libgomp.c++/target-13.C
index 376672d..5279ac0 100644
--- a/libgomp/testsuite/libgomp.c++/target-13.C
+++ b/libgomp/testsuite/libgomp.c++/target-13.C
@@ -1,11 +1,14 @@
 extern "C" void abort (void);
 
+int g;
+#pragma omp declare target (g)
+
 #pragma omp declare target
 int
 foo (void)
 {
   static int s;
-  return ++s;
+  return ++s + g;
 }
 #pragma omp end declare target
 
diff --git a/libgomp/testsuite/libgomp.c/target-28.c b/libgomp/testsuite/libgomp.c/target-28.c
index c9a2999..96e9e05 100644
--- a/libgomp/testsuite/libgomp.c/target-28.c
+++ b/libgomp/testsuite/libgomp.c/target-28.c
@@ -1,11 +1,14 @@
 extern void abort (void);
 
+int g;
+#pragma omp declare target (g)
+
 #pragma omp declare target
 int
 foo (void)
 {
   static int s;
-  return ++s;
+  return ++s + g;
 }
 #pragma omp end declare target
 

  -- Ilya