[debug] Handle debug references to skipped params

Message ID 20180708092241.ysxxwnxk757uuspy@delia
State New
Headers show
Series
  • [debug] Handle debug references to skipped params
Related show

Commit Message

Tom de Vries July 8, 2018, 9:22 a.m.
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.

First patch OK for trunk?

[debug] Handle debug references to skipped params

When compiling guality/vla-1.c with -O3 -g, vla a in f1 is optimized away, but
f1 still contains a debug expression describing the upper bound of the vla
(D.1914):
...
     __attribute__((noinline))
     f1 (intD.6 iD.1900)
     {
       <bb 2>
       saved_stack.1_2 = __builtin_stack_save ();
       # DEBUG BEGIN_STMT
       # DEBUG D#3 => i_1(D) + 1
       # DEBUG D#2 => (long intD.8) D#3
       # DEBUG D#1 => D#2 + -1
       # DEBUG D.1914 => (sizetype) D#1
...

Then f1 is cloned to a version f1.constprop with no parameters, eliminating
parameter i, and 'DEBUG D#3 => i_1(D) + 1' turns into 'D#3 => NULL'.

This patch fixes that by defining debug expressions for default defs of
eliminated parameters in remap_ssa_name:
...
     __attribute__((noinline))
     f1.constprop ()
     {
       intD.6 iD.1949;

       <bb 3>
       # DEBUG D#8 s=> iD.1900
       # DEBUG iD.1949 => D#8

       <bb 2>
+      # DEBUG D#6 s=> iD.1949
       saved_stack.1_1 = __builtin_stack_save ();
       # DEBUG BEGIN_STMT
-      # DEBUG D#3 => NULL
+      # DEBUG D#3 => D#6 + 1
       # DEBUG D#2 => (long intD.8) D#3
       # DEBUG D#1 => D#2 + -1
       # DEBUG D.1951 => (sizetype) D#1
...

Bootstrapped and reg-tested on x86_64.

2018-07-07  Tom de Vries  <tdevries@suse.de>

	* cfgexpand.c (expand_debug_source_expr): Handle VAR_DECL.
	* tree-inline.c (remap_ssa_name): Handle default def ssa_name mapping
	onto VAR_DECL with abstract origin.

	* gcc.dg/vla-1.c: New test.

---
 gcc/cfgexpand.c              |  4 ++++
 gcc/testsuite/gcc.dg/vla-1.c | 25 +++++++++++++++++++++++++
 gcc/tree-inline.c            |  4 +++-
 3 files changed, 32 insertions(+), 1 deletion(-)

Comments

Jakub Jelinek July 8, 2018, 12:22 p.m. | #1
On Sun, Jul 08, 2018 at 11:22:41AM +0200, Tom de Vries wrote:
> --- a/gcc/cfgexpand.c
> +++ b/gcc/cfgexpand.c
> @@ -5141,6 +5141,10 @@ expand_debug_source_expr (tree exp)
>  
>    switch (TREE_CODE (exp))
>      {
> +    case VAR_DECL:
> +      if (DECL_ABSTRACT_ORIGIN (exp))
> +	return expand_debug_source_expr (DECL_ABSTRACT_ORIGIN (exp));
> +      break;
>      case PARM_DECL:
>        {
>  	mode = DECL_MODE (exp);

This is ok.

> diff --git a/gcc/testsuite/gcc.dg/vla-1.c b/gcc/testsuite/gcc.dg/vla-1.c
> new file mode 100644
> index 00000000000..0c19feffd2b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vla-1.c
> @@ -0,0 +1,25 @@
> +/* { dg-do compile } */
> +/* { dg-options "-g -O3 -fdump-tree-optimized" } */
> +
> +
> +/* One debug source bind is generated for the parameter, and two to describe the
> +   sizes of a and b.  */
> +/* { dg-final { scan-tree-dump-times " s=> i" 3 "optimized" } } */

I think you at least need explicit
-fvar-tracking-assignments -fno-selective-scheduling -fno-selective-scheduling2
and perhaps some guard to ignore the test on nvptx which disables
-fvar-tracking unconditionally?

> diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
> index 427ef959740..6fbd8c3ca61 100644
> --- a/gcc/tree-inline.c
> +++ b/gcc/tree-inline.c
> @@ -208,7 +208,9 @@ remap_ssa_name (tree name, copy_body_data *id)
>  	  n = id->decl_map->get (val);
>  	  if (n != NULL)
>  	    val = *n;
> -	  if (TREE_CODE (val) != PARM_DECL)
> +	  if (TREE_CODE (val) != PARM_DECL
> +	      && !(TREE_CODE (val) == VAR_DECL
> +		   && DECL_ABSTRACT_ORIGIN (val)))
>  	    {
>  	      processing_debug_stmt = -1;
>  	      return name;

Please use VAR_P macro.

	Jakub

Patch

diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 9b91279282e..d6e3c382085 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -5141,6 +5141,10 @@  expand_debug_source_expr (tree exp)
 
   switch (TREE_CODE (exp))
     {
+    case VAR_DECL:
+      if (DECL_ABSTRACT_ORIGIN (exp))
+	return expand_debug_source_expr (DECL_ABSTRACT_ORIGIN (exp));
+      break;
     case PARM_DECL:
       {
 	mode = DECL_MODE (exp);
diff --git a/gcc/testsuite/gcc.dg/vla-1.c b/gcc/testsuite/gcc.dg/vla-1.c
new file mode 100644
index 00000000000..0c19feffd2b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vla-1.c
@@ -0,0 +1,25 @@ 
+/* { dg-do compile } */
+/* { dg-options "-g -O3 -fdump-tree-optimized" } */
+
+int __attribute__((noinline))
+f1 (int i)
+{
+  char a[i + 1];
+  char b[i + 2];
+  b[1] = 3;
+  a[0] = 5;
+  return a[0] + b[1];
+}
+
+int
+main ()
+{
+  volatile int j;
+  int x = 5;
+  j = f1 (x);
+  return 0;
+}
+
+/* One debug source bind is generated for the parameter, and two to describe the
+   sizes of a and b.  */
+/* { dg-final { scan-tree-dump-times " s=> i" 3 "optimized" } } */
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index 427ef959740..6fbd8c3ca61 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -208,7 +208,9 @@  remap_ssa_name (tree name, copy_body_data *id)
 	  n = id->decl_map->get (val);
 	  if (n != NULL)
 	    val = *n;
-	  if (TREE_CODE (val) != PARM_DECL)
+	  if (TREE_CODE (val) != PARM_DECL
+	      && !(TREE_CODE (val) == VAR_DECL
+		   && DECL_ABSTRACT_ORIGIN (val)))
 	    {
 	      processing_debug_stmt = -1;
 	      return name;