Ping.
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);
