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: [PATCH 2/n] OpenMP 4.0 offloading infrastructure: LTO streaming


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


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