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
