[gomp4.5] Handle #pragma omp declare target link

On Thu, Nov 19, 2015 at 16:31:15 +0100, Jakub Jelinek wrote:
> On Mon, Nov 16, 2015 at 06:40:43PM +0300, Ilya Verbin wrote:
> > @@ -2009,7 +2010,8 @@ scan_sharing_clauses (tree clauses, omp_context *ctx)
> >  	  decl = OMP_CLAUSE_DECL (c);
> >  	  /* Global variables with "omp declare target" attribute
> >  	     don't need to be copied, the receiver side will use them
> > -	     directly.  */
> > +	     directly.  However, global variables with "omp declare target link"
> > +	     attribute need to be copied.  */
> >  	  if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
> >  	      && DECL_P (decl)
> >  	      && ((OMP_CLAUSE_MAP_KIND (c) != GOMP_MAP_FIRSTPRIVATE_POINTER
> > @@ -2017,7 +2019,9 @@ scan_sharing_clauses (tree clauses, omp_context *ctx)
> >  		       != GOMP_MAP_FIRSTPRIVATE_REFERENCE))
> >  		  || TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE)
> >  	      && is_global_var (maybe_lookup_decl_in_outer_ctx (decl, ctx))
> > -	      && varpool_node::get_create (decl)->offloadable)
> > +	      && varpool_node::get_create (decl)->offloadable
> > +	      && !lookup_attribute ("omp declare target link",
> > +				    DECL_ATTRIBUTES (decl)))
> 
> I wonder if Honza/Richi wouldn't prefer to have this info also
> in cgraph, instead of looking up the attribute in each case.

So should I add a new flag into cgraph?
Also it is used in gimplify_adjust_omp_clauses.

> > +      if (var.link_ptr_decl == NULL_TREE)
> > +	addr = build_fold_addr_expr (var.decl);
> > +      else
> > +	{
> > +	  /* For "omp declare target link" var use address of the pointer
> > +	     instead of address of the var.  */
> > +	  addr = build_fold_addr_expr (var.link_ptr_decl);
> > +	  /* Most significant bit of the size marks such vars.  */
> > +	  unsigned HOST_WIDE_INT isize = tree_to_uhwi (size);
> > +	  isize |= 1ULL << (int_size_in_bytes (const_ptr_type_node) * 8 - 1);
> > +	  size = wide_int_to_tree (const_ptr_type_node, isize);
> > +
> > +	  /* FIXME: Remove varpool node of var?  */
> 
> There is varpool_node::remove (), but not sure if at this point all the
> references are already gone.

Actually removing varpool node here will not remove var from the target code, so
I've added a check in cgraphunit.c before assemble_decl ().

> > +class pass_omp_target_link : public gimple_opt_pass
> > +{
> > +public:
> > +  pass_omp_target_link (gcc::context *ctxt)
> > +    : gimple_opt_pass (pass_data_omp_target_link, ctxt)
> > +  {}
> > +
> > +  /* opt_pass methods: */
> > +  virtual bool gate (function *fun)
> > +    {
> > +#ifdef ACCEL_COMPILER
> > +      /* FIXME: Replace globals in target regions too or not?  */
> > +      return lookup_attribute ("omp declare target",
> > +			       DECL_ATTRIBUTES (fun->decl));
> 
> Certainly in "omp declare target entrypoint" regions too.

Done.

> > +unsigned
> > +pass_omp_target_link::execute (function *fun)
> > +{
> > +  basic_block bb;
> > +  FOR_EACH_BB_FN (bb, fun)
> > +    {
> > +      gimple_stmt_iterator gsi;
> > +      for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
> > +	{
> > +	  unsigned i;
> > +	  gimple *stmt = gsi_stmt (gsi);
> > +	  for (i = 0; i < gimple_num_ops (stmt); i++)
> > +	    {
> > +	      tree op = gimple_op (stmt, i);
> > +	      tree var = NULL_TREE;
> > +
> > +	      if (!op)
> > +		continue;
> > +	      if (TREE_CODE (op) == VAR_DECL)
> > +		var = op;
> > +	      else if (TREE_CODE (op) == ADDR_EXPR)
> > +		{
> > +		  tree op1 = TREE_OPERAND (op, 0);
> > +		  if (TREE_CODE (op1) == VAR_DECL)
> > +		    var = op1;
> > +		}
> > +	      /* FIXME: Support arrays.  What else?  */
> 
> We need to support all the references to the variables.
> So, I think this approach is not right.
> 
> > +
> > +	      if (var && lookup_attribute ("omp declare target link",
> > +					   DECL_ATTRIBUTES (var)))
> > +		{
> > +		  tree type = TREE_TYPE (var);
> > +		  tree ptype = build_pointer_type (type);
> > +
> > +		  /* Find var in offload table.  */
> > +		  omp_offload_var *table_entry = NULL;
> > +		  for (unsigned j = 0; j < vec_safe_length (offload_vars); j++)
> > +		    if ((*offload_vars)[j].decl == var)
> > +		      {
> > +			table_entry = &(*offload_vars)[j];
> > +			break;
> > +		      }
> 
> Plus this would be terribly expensive if there are many variables in
> offload_vars.
> So, what I think should be done instead is that you first somewhere, perhaps
> when streaming in the decls from LTO in ACCEL_COMPILER or so, create
> the artificial link ptr variables for the "omp declare target link"
> global vars and
>   SET_DECL_VALUE_EXPR (var, build_simple_mem_ref (link_ptr_var));
>   DECL_HAS_VALUE_EXPR_P (var) = 1;
> and then in this pass just walk_gimple_stmt each stmt, with a
> callback that would check for VAR_DECLs with DECL_HAS_VALUE_EXPR_P set
> and in that case check if they are "omp declare target link", and if found
> signal to the caller that the stmt needs to be regimplified, then just
> gimple_regimplify_operands those stmts.

Cool, it works :)  However I had to disable 2 checks in
varpool_node::assemble_decl for ACCEL_COMPILER.

> > +		  gcc_assert (table_entry);
> > +
> > +		  /* Get or create artificial pointer for the var.  */
> > +		  tree ptr_decl;
> > +		  if (table_entry->link_ptr_decl != NULL_TREE)
> > +		    ptr_decl = table_entry->link_ptr_decl;
> > +		  else
> > +		    {
> > +		      /* FIXME: Create a new node instead of copying?
> > +			 Which info to preserve?  */
> > +		      ptr_decl = copy_node (var);
> 
> I think you want a new node instead of copying.  You don't really want to
> copy anything, perhaps TREE_USED, and set DECL_NAME to something derived
> from the original name.  Make the ptr DECL_ARTIFICIAL and perhaps
> DECL_NAMELESS.

Done.

> > diff --git a/libgomp/target.c b/libgomp/target.c
> > index ef22329..195be43 100644
> > --- a/libgomp/target.c
> > +++ b/libgomp/target.c
> > @@ -78,6 +78,17 @@ static int num_devices;
> >  /* Number of GOMP_OFFLOAD_CAP_OPENMP_400 devices.  */
> >  static int num_devices_openmp;
> >  
> > +/* FIXME: Quick and dirty prototype of keeping correspondence between host
> > +   address of the object and target address of the artificial link pointer.
> > +   Move it to gomp_device_descr, or where?  */
> > +struct link_struct
> > +{
> > +  uintptr_t host_start;
> > +  uintptr_t tgt_link_ptr;
> > +};
> > +static struct link_struct links[100];
> > +static int link_num;
> 
> As for the representation, I think one possibility would be to say define
> REFCOUNT_LINK (~(uintptr_t) 1)
> and register at gomp_load_image_to_device time the link vars with that
> refcount instead of REFCOUNT_INFINITY.  If k->refcount == REFCOUNT_LINK
> then k->tgt_offset would be the pointer to the artificial pointer variable
> instead of actual mapping; for say pointer lookup purposes
> k->refcount == REFCOUNT_LINK would be treated as not mapped, and
> gomp_map_vars if mapping something over that would simply temporarily
> replace (remove the old splay tree key, add the new one) the REFCOUNT_LINK entry
> with the new mapping (and store the pointer).  Then for the even when the
> new mapping's refcount drops to zero we need to ensure that we readd the
> REFCOUNT_LINK entry.  For that we need to store the old splay_tree_key
> somewhere.  Either we can add it to splay_tree_key_s, but then it will be
> around unconditionally for all entries, and splay_tree_node right now is
> nicely power of 2-ish - 8 pointers.  Or stick it somewhere in
> struct target_mem_desc, say splay_tree_key *link; and if the tgt has tgt->array
> allocated and any of the mappings were previously REFCOUNT_LINK, then you could
> either allocate that link array with not_found_cnt elements, or allocate
> together with tgt->array and just point it after the last entry in
> tgt->array.  tgt->link[i] would be NULL if tgt->array[i] splay_tree_node_s
> did not replace REFCOUNT_LINK when created, and the old REFCOUNT_LINK entry
> otherwise.  When do_unmap or exit_data, before splay_tree_remove you'd
> find corresponding link entry (k should point to &k->tgt->array[X].key
> for some X, so (splay_tree_node) k - k->tgt->array should be X and thus
> splay_tree_key linkk = NULL;
> if (k->tgt->link)
>   linkk = k->tgt->link[(splay_tree_node) k - k->tgt->array];
> before
>   splay_tree_remove (&devicep->mem_map, k);
> should hopefully give you the splay_tree_key to insert again.

I implemented the first approach, because the second seems more complicated.
Or should I implement the second?

make check-target-libgomp passed, bootstrap in progress.  Is it OK?

gcc/c-family/
	* c-common.c (c_common_attribute_table): Handle "omp declare target
	link" attribute.
gcc/
	* cgraphunit.c (output_in_order): Do not assemble "omp declare target
	link" variables in ACCEL_COMPILER.
	* gimplify.c (gimplify_adjust_omp_clauses): Do not remove mapping of
	"omp declare target link" variables.
	* lto/lto.c: Include stringpool.h and fold-const.h.
	(offload_handle_link_vars): New static function.
	(lto_main): Call offload_handle_link_vars.
	* omp-low.c (scan_sharing_clauses): Do not remove mapping of "omp
	declare target link" variables.
	(add_decls_addresses_to_decl_constructor): For "omp declare target link"
	variables output address of the artificial pointer instead of address of
	the variable.  Set most significant bit of the size to mark them.
	(pass_data_omp_target_link): New pass_data.
	(pass_omp_target_link): New class.
	(find_link_var_op): New static function.
	(make_pass_omp_target_link): New function.
	* passes.def: Add pass_omp_target_link.
	* tree-pass.h (make_pass_omp_target_link): Declare.
	* varpool.c (varpool_node::assemble_decl): Allow decls with VALUE_EXPR
	in ACCEL_COMPILER.
libgomp/
	* libgomp.h (REFCOUNT_LINK): Define.
	(struct splay_tree_key_s): Add link_key.
	* target.c (gomp_map_vars): Treat REFCOUNT_LINK objects as not mapped.
	Replace target address of the pointer with target address of newly
	mapped object in the splay tree.  Set link pointer on target to the
	device address of the mapped object.
	(gomp_unmap_vars): Restore target address of the pointer in the splay
	tree for REFCOUNT_LINK objects after unmapping.
	(gomp_load_image_to_device): Set refcount to REFCOUNT_LINK for "omp
	declare target link" objects.
	(gomp_exit_data): Restore target address of the pointer in the splay
	tree for REFCOUNT_LINK objects after unmapping.
	* testsuite/libgomp.c/target-link-1.c: New file.

  -- Ilya

[gomp4.5] Handle #pragma omp declare target link

Commit Message

Comments

Patch