On Thu, Sep 21, 2017 at 01:08:01PM -0700, Kevin Buettner wrote:
> Ping.
Ok, thanks.
> On Mon, 7 Aug 2017 17:51:38 -0700
> Kevin Buettner <kev...@redhat.com> wrote:
>
> > On Wed, 10 May 2017 17:24:27 +0200
> > Jakub Jelinek <ja...@redhat.com> wrote:
> >
> > > What I don't like is that the patch is inconsistent, it sets DECL_CONTEXT
> > > of the child function for all kinds of outlined functions, but then you
> > > just
> > > choose one of the many places and add it into the BLOCK tree. Any reason
> > > why the DECL_CONTEXT change can't be done in a helper function together
> > > with all the changes you've added into omp-expand.c, and then call it from
> > > expand_omp_parallel (with the child_fn and entry_stmt arguments) so that
> > > you can call it easily also for other constructs, either now or later on?
> > >
> >
> > I've worked out a way to do the DECL_CONTEXT and the scope change
> > together. The helper function should be usable for other constructs,
> > though I have not tested this yet.
> >
> > > Also, is there any rationale on appending the FUNCTION_DECL to BLOCK_VARS
> > > instead of prepending it there (which is cheaper)? Does the debugger
> > > care about relative order of those artificial functions vs. other
> > > variables in the lexical scope?
> >
> > To the best of my knowledge, the debugger doesn't care about the
> > order. I've changed the code to prepend the FUNCTION_DECL to
> > BLOCK_VARS instead.
> >
> > How does this new version (below) look?
> >
> > I've done a "make bootstrap" and "make -k check". No regressions found
> > for x86_64.
> >
> > gcc/ChangeLog:
> >
> > * omp-expand.c (adjust_context_scope): New function.
> > (expand_parallel_call): Call adjust_context_scope.
> > ---
> > gcc/omp-expand.c | 38 ++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 38 insertions(+)
> >
> > diff --git a/gcc/omp-expand.c b/gcc/omp-expand.c
> > index d6755cd..9eb0a89 100644
> > --- a/gcc/omp-expand.c
> > +++ b/gcc/omp-expand.c
> > @@ -498,6 +498,42 @@ parallel_needs_hsa_kernel_p (struct omp_region *region)
> > return false;
> > }
> >
> > +/* Change DECL_CONTEXT of CHILD_FNDECL to that of the parent function.
> > + Add CHILD_FNDECL to decl chain of the supercontext of the block
> > + ENTRY_BLOCK - this is the block which originally contained the
> > + code from which CHILD_FNDECL was created.
> > +
> > + Together, these actions ensure that the debug info for the outlined
> > + function will be emitted with the correct lexical scope. */
> > +
> > +static void
> > +adjust_context_and_scope (tree entry_block, tree child_fndecl)
> > +{
> > + if (entry_block != NULL_TREE && TREE_CODE (entry_block) == BLOCK)
> > + {
> > + tree b = BLOCK_SUPERCONTEXT (entry_block);
> > +
> > + if (TREE_CODE (b) == BLOCK)
> > + {
> > + tree parent_fndecl;
> > +
> > + /* Follow supercontext chain until the parent fndecl
> > + is found. */
> > + for (parent_fndecl = BLOCK_SUPERCONTEXT (b);
> > + TREE_CODE (parent_fndecl) == BLOCK;
> > + parent_fndecl = BLOCK_SUPERCONTEXT (parent_fndecl))
> > + ;
> > +
> > + gcc_assert (TREE_CODE (parent_fndecl) == FUNCTION_DECL);
> > +
> > + DECL_CONTEXT (child_fndecl) = parent_fndecl;
> > +
> > + DECL_CHAIN (child_fndecl) = BLOCK_VARS (b);
> > + BLOCK_VARS (b) = child_fndecl;
> > + }
> > + }
> > +}
> > +
> > /* Build the function calls to GOMP_parallel_start etc to actually
> > generate the parallel operation. REGION is the parallel region
> > being expanded. BB is the block where to insert the code. WS_ARGS
> > @@ -667,6 +703,8 @@ expand_parallel_call (struct omp_region *region,
> > basic_block bb,
> > tree child_fndecl = gimple_omp_parallel_child_fn (entry_stmt);
> > t2 = build_fold_addr_expr (child_fndecl);
> >
> > + adjust_context_and_scope (gimple_block (entry_stmt), child_fndecl);
> > +
> > vec_alloc (args, 4 + vec_safe_length (ws_args));
> > args->quick_push (t2);
> > args->quick_push (t1);
Jakub
