On Fri, Jul 6, 2018 at 12:47 PM Tom de Vries <tdevr...@suse.de> wrote:
>
> On 07/05/2018 01:39 PM, Richard Biener wrote:
> > On Thu, Jul 5, 2018 at 1:25 PM Tom de Vries <tdevr...@suse.de> wrote:
> >>
> >> [ was: Re: [testsuite/guality, committed] Prevent optimization of local in
> >> vla-1.c ]
> >>
> >> On Wed, Jul 04, 2018 at 02:32:27PM +0200, Tom de Vries wrote:
> >>> On 07/03/2018 11:05 AM, Tom de Vries wrote:
> >>>> On 07/02/2018 10:16 AM, Jakub Jelinek wrote:
> >>>>> On Mon, Jul 02, 2018 at 09:44:04AM +0200, Richard Biener wrote:
>
> <SNIP>
>
> >> [debug] Handle references to skipped params in remap_ssa_name
> >>
> >> 2018-07-05 Tom de Vries <tdevr...@suse.de>
> >>
> >> * tree-inline.c (remap_ssa_name): Handle references to skipped
> >> params.
> >>
> >> ---
> >> gcc/tree-inline.c | 17 +++++++++++++++--
> >> 1 file changed, 15 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
> >> index 427ef959740..0fa996cab49 100644
> >> --- a/gcc/tree-inline.c
> >> +++ b/gcc/tree-inline.c
> >> @@ -204,11 +204,22 @@ remap_ssa_name (tree name, copy_body_data *id)
> >> gimple *def_temp;
> >> gimple_stmt_iterator gsi;
> >> tree val = SSA_NAME_VAR (name);
> >> + bool skipped_parm_decl = false;
> >>
> >> n = id->decl_map->get (val);
> >> if (n != NULL)
> >> - val = *n;
> >> - if (TREE_CODE (val) != PARM_DECL)
> >> + {
> >> + if (TREE_CODE (*n) == DEBUG_EXPR_DECL)
> >> + return *n;
> >> +
> >> + if (TREE_CODE (*n) == VAR_DECL
> >> + && DECL_ABSTRACT_ORIGIN (*n)
> >> + && TREE_CODE (DECL_ABSTRACT_ORIGIN (*n)) == PARM_DECL)
> >> + skipped_parm_decl = true;
> >> + else
> >> + val = *n;
> >> + }
> >> + if (TREE_CODE (val) != PARM_DECL && !skipped_parm_decl)
> >
> > I wonder if this cannot be more easily set up in
> > copy_arguments_for_versioning
> > which already does
> >
> > else if (!id->decl_map->get (arg))
> > {
> > /* Make an equivalent VAR_DECL. If the argument was used
> > as temporary variable later in function, the uses will be
> > replaced by local variable. */
> > tree var = copy_decl_to_var (arg, id);
> > insert_decl_map (id, arg, var);
> > /* Declare this new variable. */
> > DECL_CHAIN (var) = *vars;
> > *vars = var;
> > }
> >
> > which just misses to re-map the default def of the PARM_DECL (in case it
> > exists)
> > to the same(?) var?
>
> I've updated the patch to add a debug expr here in
> copy_arguments_for_versioning for every parameter that has a default
> def, and to use that debug expr in remap_ssa_name.
>
> > All remaining uses should be in debug stmts (I hope).
>
> I ran into a test-case where that was not the case, so I had to handle
> that in remap_ssa_name, the comment in the patch describes that in more
> detail.
I see.
I now also spotted the code in remap_ssa_name that is supposed to handle
this it seems and for the testcase we only give up because the PARM_DECL is
remapped to a VAR_DECL. So I suppose it is to be handled via the
debug-args stuff
which probably lacks in the area of versioning.
Your patch feels like it adds stuff ontop of existing mechanisms that
should "just work"
with the correct setup at the correct places...
Jakub, can you shed any light on how this is supposed to work?
Looking at rev. 175288
it's not entirely clear to me?
Richard.
> Bootstrapped and reg-tested on x86_64.
>
> OK for trunk?
>
> Thanks,
> - Tom
