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)
So you propose to set global have_offload flag somewhere in expand_omp_target,
etc. where functions and global variables are created?
> > static void
> > ipa_passes (void)
> > {
> > + bool have_offload = false;
> > gcc::pass_manager *passes = g->get_passes ();
> >
> > set_cfun (NULL);
> > @@ -2004,6 +2036,14 @@ ipa_passes (void)
> > gimple_register_cfg_hooks ();
> > bitmap_obstack_initialize (NULL);
> >
> > + if (!in_lto_p && flag_openmp)
>
> As -fopenmp is not generally available it's odd to test
> flag_openmp (though that is available everywhere as
> implementation detail). Doesn't offloading work
> without -fopenmp?
In this patch series offloading is implemented only for OpenMP.
OpenACC guys will add flag_openacc here.
> > /* If LTO is enabled, initialize the streamer hooks needed by GIMPLE. */
> > - if (flag_lto)
> > + if (flag_lto || flag_openmp)
>
> flag_generate_lto?
>
> > /* When not optimizing, do not bother to analyze. Inlining is still done
> > because edge redirection needs to happen there. */
> > - if (!optimize && !flag_lto && !flag_wpa)
> > + if (!optimize && !flag_lto && !flag_wpa && !flag_openmp)
> > return;
>
> Likewise !flag_generate_lto
Currently this is not working, since symbol_table::compile is executed before
ipa_passes. But with global have_offload it should work.
> > +/* 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.
Thanks,
-- Ilya
