[PATCH 2/n] OpenMP 4.0 offloading infrastructure: LTO streaming

[Ilya Verbin]

On 20 Oct 15:19, Ilya Verbin wrote:
> On 15 Oct 16:23, Richard Biener wrote:
> > > +static bool
> > > +initialize_offload (void)
> > > +{
> > > +  bool have_offload = false;
> > > +  struct cgraph_node *node;
> > > +  struct varpool_node *vnode;
> > > +
> > > +  FOR_EACH_DEFINED_FUNCTION (node)
> > > +    if (lookup_attribute ("omp declare target", DECL_ATTRIBUTES (node->decl)))
> > > +      {
> > > +	have_offload = true;
> > > +	break;
> > > +      }
> > > +
> > > +  FOR_EACH_DEFINED_VARIABLE (vnode)
> > > +    {
> > > +      if (!lookup_attribute ("omp declare target",
> > > +			     DECL_ATTRIBUTES (vnode->decl))
> > > +	  || TREE_CODE (vnode->decl) != VAR_DECL
> > > +	  || DECL_SIZE (vnode->decl) == 0)
> > > +	continue;
> > > +      have_offload = true;
> > > +    }
> > > +
> > > +  return have_offload;
> > > +}
> > > +
> > 
> > I wonder if we can avoid the above by means of a global have_offload
> > flag?  (or inside gcc::context)
>
> > > +/* Select what needs to be streamed out.  In regular lto mode stream everything.
> > > +   In offload lto mode stream only stuff marked with an attribute.  */
> > > +void
> > > +select_what_to_stream (bool offload_lto_mode)
> > > +{
> > > +  struct symtab_node *snode;
> > > +  FOR_EACH_SYMBOL (snode)
> > > +    snode->need_lto_streaming
> > > +      = !offload_lto_mode || lookup_attribute ("omp declare target",
> > > +					       DECL_ATTRIBUTES (snode->decl));
> > 
> > I suppose I suggested this already earlier this year.  Why keep this
> > artificial attribute when you have a cgraph node flag?
> 
> > > +	  /* If '#pragma omp critical' is inside target region, the symbol must
> > > +	     have an 'omp declare target' attribute.  */
> > > +	  omp_context *octx;
> > > +	  for (octx = ctx->outer; octx; octx = octx->outer)
> > > +	    if (is_targetreg_ctx (octx))
> > > +	      {
> > > +		DECL_ATTRIBUTES (decl)
> > > +		  = tree_cons (get_identifier ("omp declare target"),
> > > +			       NULL_TREE, DECL_ATTRIBUTES (decl));
> > 
> > Here - why not set a flag on cgraph_get_node (decl) instead?
> 
> I thought that select_what_to_stream is exactly what you've suggested.
> Could you please clarify this?  You propose to replace "omp declare target"
> attribure with some cgraph node flag like need_offload?  But we'll need
> need_lto_streaming anyway, since for LTO it should be 1 for all nodes, but for
> offloading it should be equal to need_offload.

We have to set the global have_offload flag in few places in omp-low.c and in FE
(c/c-decl.c:c_decl_attributes, fortran/trans-common.c:build_common_decl,
fortran/trans-decl.c:add_attributes_to_decl).
This way looks for me a bit more complicated than the current approach.

Actually, we could follow Jakub's suggestion of caching the attribute in a bit
field, and set the global have_offload flag on the run without any changes in
FE.  However, I don't know a suitable place for it.  If you agree with the
approach, could you please specify the place?

Thanks,
  -- Ilya