| Message ID | 20180708092725.c7ahp6ggfh6d3mqz@delia |
|---|---|
| State | New |
| Headers | show |
| Series | [debug] Reuse debug exprs generated in remap_ssa_name | expand |
On Sun, Jul 8, 2018 at 11:27 AM Tom de Vries <tdevries@suse.de> wrote: > > On Sun, Jul 08, 2018 at 11:22:41AM +0200, Tom de Vries wrote: > > On Fri, Jul 06, 2018 at 04:38:50PM +0200, Richard Biener wrote: > > > On Fri, Jul 6, 2018 at 12:47 PM Tom de Vries <tdevries@suse.de> wrote: > > > > On 07/05/2018 01:39 PM, Richard Biener wrote: > > > > <SNIP> > > > > > 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... > > > > > > > Hmm, I realized that I may be complicating things, by trying to do an > > optimal fix in a single patch, so I decided to write two patches, one > > with a fix, and then one improving the fix to be more optimal. > > > > Also, I suspect that the "just work" approach is this: > > ... > > # DEBUG D#8 s=> iD.1900 > > # DEBUG iD.1949 => D#8 > > # DEBUG D#6 s=> iD.1949 > > ... > > whereas previously I tried to map 'D#6' on iD.1900 directly. > > > > Second patch OK for trunk? OK, though I wonder how it doesn't fail with that testcase with the mismatching type where the removed param-decl is mapped to a local var-decl. Richard. > Thanks, > - Tom > > [debug] Reuse debug exprs generated in remap_ssa_name > > When compiling gcc.dg/vla-1.c with -O3 -g, vla a and b in f1 are optimized > away, and f1 is cloned to a version f1.constprop with no parameters, eliminating > parameter i. Debug info is generated to describe the sizes of a and b, but > that process generates debug expressions that are not reused. > > Fix the duplication by saving and reusing the generated debug expressions in > remap_ssa_name. Concretely: reuse D#7 here instead of generating D#8: > ... > __attribute__((noinline)) > f1.constprop () > { > intD.6 iD.1935; > > <bb 3> > # DEBUG D#10 s=> iD.1897 > # DEBUG iD.1935 => D#10 > > <bb 2> > - # DEBUG D#8 s=> iD.1935 > # DEBUG D#7 s=> iD.1935 > saved_stack.2_1 = __builtin_stack_save (); > # DEBUG BEGIN_STMT > # DEBUG D#6 => D#7 + 1 > # DEBUG D#5 => (long intD.8) D#6 > # DEBUG D#4 => D#5 + -1 > # DEBUG D.1937 => (sizetype) D#4 > # DEBUG a.0D.1942 => NULL > # DEBUG BEGIN_STMT > - # DEBUG D#3 => D#8 + 2 > + # DEBUG D#3 => D#7 + 2 > # DEBUG D#2 => (long intD.8) D#3 > # DEBUG D#1 => D#2 + -1 > # DEBUG D.1944 => (sizetype) D#1 > # DEBUG b.1D.1949 => NULL > ... > > Bootstrapped and reg-tested on x86_64. > > 2018-07-07 Tom de Vries <tdevries@suse.de> > > * tree-inline.c (remap_ssa_name): Save and reuse debug exprs generated > in remap_ssa_name. > > * gcc.dg/vla-1.c: Update. > > --- > gcc/testsuite/gcc.dg/vla-1.c | 5 +++-- > gcc/tree-inline.c | 4 ++++ > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/gcc/testsuite/gcc.dg/vla-1.c b/gcc/testsuite/gcc.dg/vla-1.c > index 0c19feffd2b..94db23d1336 100644 > --- a/gcc/testsuite/gcc.dg/vla-1.c > +++ b/gcc/testsuite/gcc.dg/vla-1.c > @@ -20,6 +20,7 @@ main () > return 0; > } > > -/* One debug source bind is generated for the parameter, and two to describe the > +/* One debug source bind is generated for the parameter, and one to describe the > sizes of a and b. */ > -/* { dg-final { scan-tree-dump-times " s=> i" 3 "optimized" } } */ > +/* { dg-final { scan-tree-dump-times " s=> i" 2 "optimized" } } */ > + > diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c > index 6fbd8c3ca61..164c7fff710 100644 > --- a/gcc/tree-inline.c > +++ b/gcc/tree-inline.c > @@ -215,12 +215,16 @@ remap_ssa_name (tree name, copy_body_data *id) > processing_debug_stmt = -1; > return name; > } > + n = id->decl_map->get (val); > + if (n && TREE_CODE (*n) == DEBUG_EXPR_DECL) > + return *n; > def_temp = gimple_build_debug_source_bind (vexpr, val, NULL); > DECL_ARTIFICIAL (vexpr) = 1; > TREE_TYPE (vexpr) = TREE_TYPE (name); > SET_DECL_MODE (vexpr, DECL_MODE (SSA_NAME_VAR (name))); > gsi = gsi_after_labels (single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun))); > gsi_insert_before (&gsi, def_temp, GSI_SAME_STMT); > + insert_decl_map (id, val, vexpr); > return vexpr; > } > >
On 07/09/2018 02:43 PM, Richard Biener wrote: > On Sun, Jul 8, 2018 at 11:27 AM Tom de Vries <tdevries@suse.de> wrote: >> >> On Sun, Jul 08, 2018 at 11:22:41AM +0200, Tom de Vries wrote: >>> On Fri, Jul 06, 2018 at 04:38:50PM +0200, Richard Biener wrote: >>>> On Fri, Jul 6, 2018 at 12:47 PM Tom de Vries <tdevries@suse.de> wrote: >>>>> On 07/05/2018 01:39 PM, Richard Biener wrote: >>> >>> <SNIP> >>> >>>> 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... >>>> >>> >>> Hmm, I realized that I may be complicating things, by trying to do an >>> optimal fix in a single patch, so I decided to write two patches, one >>> with a fix, and then one improving the fix to be more optimal. >>> >>> Also, I suspect that the "just work" approach is this: >>> ... >>> # DEBUG D#8 s=> iD.1900 >>> # DEBUG iD.1949 => D#8 >>> # DEBUG D#6 s=> iD.1949 >>> ... >>> whereas previously I tried to map 'D#6' on iD.1900 directly. >>> >> >> Second patch OK for trunk? > > OK, though I wonder how it doesn't fail with that testcase with > the mismatching type where the removed param-decl is mapped > to a local var-decl. Previously I mapped the default def ssa-name onto the debug expression, which meant I had to add special handling at the start of remap_ssa_name, where already mapped ssa-names are handled, for the mismatched argument/parameter type case. Now I map the local var-decl onto the debug expression, and the code that is added to look it up is already guarded with processing_debug_stmt. Thanks, - Tom >> diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c >> index 6fbd8c3ca61..164c7fff710 100644 >> --- a/gcc/tree-inline.c >> +++ b/gcc/tree-inline.c >> @@ -215,12 +215,16 @@ remap_ssa_name (tree name, copy_body_data *id) >> processing_debug_stmt = -1; >> return name; >> } >> + n = id->decl_map->get (val); >> + if (n && TREE_CODE (*n) == DEBUG_EXPR_DECL) >> + return *n; >> def_temp = gimple_build_debug_source_bind (vexpr, val, NULL); >> DECL_ARTIFICIAL (vexpr) = 1; >> TREE_TYPE (vexpr) = TREE_TYPE (name); >> SET_DECL_MODE (vexpr, DECL_MODE (SSA_NAME_VAR (name))); >> gsi = gsi_after_labels (single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun))); >> gsi_insert_before (&gsi, def_temp, GSI_SAME_STMT); >> + insert_decl_map (id, val, vexpr); >> return vexpr; >> } >> >>
On Fri, Jul 13, 2018 at 12:09 PM Tom de Vries <tdevries@suse.de> wrote: > > On 07/09/2018 02:43 PM, Richard Biener wrote: > > On Sun, Jul 8, 2018 at 11:27 AM Tom de Vries <tdevries@suse.de> wrote: > >> > >> On Sun, Jul 08, 2018 at 11:22:41AM +0200, Tom de Vries wrote: > >>> On Fri, Jul 06, 2018 at 04:38:50PM +0200, Richard Biener wrote: > >>>> On Fri, Jul 6, 2018 at 12:47 PM Tom de Vries <tdevries@suse.de> wrote: > >>>>> On 07/05/2018 01:39 PM, Richard Biener wrote: > >>> > >>> <SNIP> > >>> > >>>> 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... > >>>> > >>> > >>> Hmm, I realized that I may be complicating things, by trying to do an > >>> optimal fix in a single patch, so I decided to write two patches, one > >>> with a fix, and then one improving the fix to be more optimal. > >>> > >>> Also, I suspect that the "just work" approach is this: > >>> ... > >>> # DEBUG D#8 s=> iD.1900 > >>> # DEBUG iD.1949 => D#8 > >>> # DEBUG D#6 s=> iD.1949 > >>> ... > >>> whereas previously I tried to map 'D#6' on iD.1900 directly. > >>> > >> > >> Second patch OK for trunk? > > > > OK, though I wonder how it doesn't fail with that testcase with > > the mismatching type where the removed param-decl is mapped > > to a local var-decl. > > Previously I mapped the default def ssa-name onto the debug expression, > which meant I had to add special handling at the start of > remap_ssa_name, where already mapped ssa-names are handled, for the > mismatched argument/parameter type case. > > Now I map the local var-decl onto the debug expression, and the code > that is added to look it up is already guarded with processing_debug_stmt. Ah, thanks for the explanation! Richard. > Thanks, > - Tom > > >> diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c > >> index 6fbd8c3ca61..164c7fff710 100644 > >> --- a/gcc/tree-inline.c > >> +++ b/gcc/tree-inline.c > >> @@ -215,12 +215,16 @@ remap_ssa_name (tree name, copy_body_data *id) > >> processing_debug_stmt = -1; > >> return name; > >> } > >> + n = id->decl_map->get (val); > >> + if (n && TREE_CODE (*n) == DEBUG_EXPR_DECL) > >> + return *n; > >> def_temp = gimple_build_debug_source_bind (vexpr, val, NULL); > >> DECL_ARTIFICIAL (vexpr) = 1; > >> TREE_TYPE (vexpr) = TREE_TYPE (name); > >> SET_DECL_MODE (vexpr, DECL_MODE (SSA_NAME_VAR (name))); > >> gsi = gsi_after_labels (single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun))); > >> gsi_insert_before (&gsi, def_temp, GSI_SAME_STMT); > >> + insert_decl_map (id, val, vexpr); > >> return vexpr; > >> } > >> > >>
diff --git a/gcc/testsuite/gcc.dg/vla-1.c b/gcc/testsuite/gcc.dg/vla-1.c index 0c19feffd2b..94db23d1336 100644 --- a/gcc/testsuite/gcc.dg/vla-1.c +++ b/gcc/testsuite/gcc.dg/vla-1.c @@ -20,6 +20,7 @@ main () return 0; } -/* One debug source bind is generated for the parameter, and two to describe the +/* One debug source bind is generated for the parameter, and one to describe the sizes of a and b. */ -/* { dg-final { scan-tree-dump-times " s=> i" 3 "optimized" } } */ +/* { dg-final { scan-tree-dump-times " s=> i" 2 "optimized" } } */ + diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c index 6fbd8c3ca61..164c7fff710 100644 --- a/gcc/tree-inline.c +++ b/gcc/tree-inline.c @@ -215,12 +215,16 @@ remap_ssa_name (tree name, copy_body_data *id) processing_debug_stmt = -1; return name; } + n = id->decl_map->get (val); + if (n && TREE_CODE (*n) == DEBUG_EXPR_DECL) + return *n; def_temp = gimple_build_debug_source_bind (vexpr, val, NULL); DECL_ARTIFICIAL (vexpr) = 1; TREE_TYPE (vexpr) = TREE_TYPE (name); SET_DECL_MODE (vexpr, DECL_MODE (SSA_NAME_VAR (name))); gsi = gsi_after_labels (single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun))); gsi_insert_before (&gsi, def_temp, GSI_SAME_STMT); + insert_decl_map (id, val, vexpr); return vexpr; }