On Fri, 22 Sep 2017 18:08:39 +0200
Jakub Jelinek <ja...@redhat.com> wrote:
> On Thu, Sep 21, 2017 at 01:08:01PM -0700, Kevin Buettner wrote:
> > Ping.
>
> Ok, thanks.
Committed.
Kevin
> > 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
