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] Output DIEs for outlined OpenMP functions in correct lexical scope


On Fri, 5 May 2017 14:23:14 +0300 (MSK)
Alexander Monakov <amonakov@ispras.ru> wrote:

> On Thu, 4 May 2017, Kevin Buettner wrote:
> > diff --git a/gcc/omp-expand.c b/gcc/omp-expand.c
> > index 5c48b78..7029951 100644
> > --- a/gcc/omp-expand.c
> > +++ b/gcc/omp-expand.c
> > @@ -667,6 +667,25 @@ expand_parallel_call (struct omp_region *region, basic_block bb,  
> 
> Outlined functions are also used for 'omp task' and 'omp target' regions, but
> here only 'omp parallel' is handled. Will this code need to be duplicated for
> those region types?

For 'omp task' and 'omp target', I think it's possible or even likely
that the original context which started these parallel tasks will no
longer exist.  So, it might not make sense to do something equivalent
for 'task' and 'target'.

That said, I have not yet given the matter much study.  There may be
cases where having scoped debug info might still prove useful.

The short answer is, "I don't know."

> >    tree child_fndecl = gimple_omp_parallel_child_fn (entry_stmt);
> >    t2 = build_fold_addr_expr (child_fndecl);
> >  
> > +  if (gimple_block (entry_stmt) != NULL_TREE
> > +      && TREE_CODE (gimple_block (entry_stmt)) == BLOCK)  
> 
> Here and also below, ...
> 
> > +    {
> > +      tree b = BLOCK_SUPERCONTEXT (gimple_block (entry_stmt));
> > +
> > +      /* Add child_fndecl to var chain of the supercontext of the
> > +        block corresponding to entry_stmt.  This ensures that debug
> > +        info for the outlined function will be emitted for the correct
> > +        lexical scope.  */
> > +      if (b != NULL_TREE && TREE_CODE (b) == BLOCK)  
> 
> ... here, I'm curious why the conditionals are necessary -- I don't see why the
> conditions can be sometimes true and sometimes false.  Sorry if I'm missing
> something obvious.

I'm not especially knowledgeable about gcc internals.  It may be the
case these conditionals that you noted are unnecessary and might
perhaps better be handled via the use of an assert.

I will note that when I originally coded it, I had fewer tests.  The code
still worked for the cases that I tried.  Later, when I reviewed it
for posting here, I decided to add some more checks.

I'll explain my reasoning for each of them...

> > +  if (gimple_block (entry_stmt) != NULL_TREE

If we have NULL_TREE here, dereferencing gimple_block (entry_stmt) further
won't work.

> > +      && TREE_CODE (gimple_block (entry_stmt)) == BLOCK)  

It seemed to me that a having a BLOCK is necessary in order to later
use BLOCK_SUPERCONTEXT.

> > +      if (b != NULL_TREE && TREE_CODE (b) == BLOCK)  

I check to make sure that b is a block so that I can later refer to
BLOCK_VARS (b).

Again, it may be the case that these should always evaluate to true.
If so, then use of an assert might be better here.

Kevin


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