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.5] Handle #pragma omp declare target link


On Mon, Nov 30, 2015 at 11:29:34PM +0300, Ilya Verbin wrote:
> > This looks wrong, both of these clearly could affect anything with
> > DECL_HAS_VALUE_EXPR_P, not just the link vars.
> > So, if you need to handle the "omp declare target link" vars specially,
> > you should only handle those specially and nothing else.  And please try to
> > explain why.
> 
> Actually these ifndefs are not needed, because assemble_decl never will be
> called by accel compiler for original link vars.  I've added a check into
> output_in_order, but missed a second place where assemble_decl is called -
> symbol_table::output_variables.  So, fixed now.

Great.

> > Do we need to do anything in gomp_unload_image_from_device ?
> > I mean at least in questionable programs that for link vars don't decrement
> > the refcount of the var that replaced the link var to 0 first before
> > dlclosing the library.
> > At least host_var_table[j * 2 + 1] will have the MSB set, so we need to
> > handle it differently.  Perhaps for that case perform a lookup, and if we
> > get something which has link_map non-NULL, first perform as if there is
> > target exit data delete (var) on it first?
> 
> You're right, it doesn't deallocate memory on the device if DSO leaves nonzero
> refcount.  And currently host compiler doesn't set MSB in host_var_table, it's
> set only by accel compiler.  But it's possible to do splay_tree_lookup for each
> var to determine whether is it linked or not, like in the patch bellow.
> Or do you prefer to set the bit in host compiler too?  It requires
> lookup_attribute ("omp declare target link") for all vars in the table during
> compilation, but allows to do splay_tree_lookup at run-time only for vars with
> MSB set in host_var_table.
> Unfortunately, calling gomp_exit_data from gomp_unload_image_from_device works
> only for DSO, but it crashed when an executable leaves nonzero refcount, because
> target device may be already uninitialized from plugin's __run_exit_handlers
> (and it is in case of intelmic), so gomp_exit_data cannot run free_func.
> Is it possible do add some atexit (...) to libgomp, which will set shutting_down
> flag, and just do nothing in gomp_unload_image_from_device if it is set?

Sorry, I didn't mean you should call gomp_exit_data, what I meant was that
you perform the same action as would delete(var) do in that case.
Calling gomp_exit_data e.g. looks it up again etc.
Supposedly having the MSB in host table too is useful, so if you could
handle that, it would be nice.  And splay_tree_lookup only if the MSB is
set.
So,
    if (!host_data_has_msb_set)
      splay_tree_remove (&devicep->mem_map, &k);
    else
      {
        splay_tree_key n = splay_tree_lookup (&devicep->mem_map, &k);
        if (n->link_key)
	  {
	    n->refcount = 0;
	    n->link_key = NULL;
	    splay_tree_remove (&devicep->mem_map, n);
	    if (n->tgt->refcount > 1)
	      n->tgt->refcount--;
	    else
	      gomp_unmap_tgt (n->tgt);
	  }
	else
	  splay_tree_remove (&devicep->mem_map, n);
      }
or so.

	Jakub


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