Re: Test distilled from -gdwarf-2 -O3 bootstrap failure

Re: Test distilled from -gdwarf-2 -O3 bootstrap failure

[Alexandre Oliva]

[-- Attachment #1: Type: text/plain, Size: 2746 bytes --]

On Feb 24, 2002, Alexandre Oliva <aoliva@redhat.com> wrote:

> I'm going to try and debug this problem, so this is just a heads up.

Ok, I've figured out what's going on.  In the first round of expanding
C bodies, f3() is deemed too big to be tree-inlined, so generation of
RTL is not deferred for it, but generation of assembly is.  f4() is
compiled to RTL and then assembly is generated for it, because it's
referenced (by itself :-(, so f3() has to be emitted out-of-line
because it couldn't be inlined.

In the second round(), we generate RTL for f1() and f2(), but they are
never emitted because they are inlined.  After generating assembly for
f3(), however, we get in trouble because:

- the inlined block from f1() contains the statement expression passed
  as an argument from f2

- the block copied from f1() was marked as ABSTRACT when we started
  generating debugging info for the inlined f2:
  dwarf2out_abstract_function() set the abstract flag in all blocks
  within f2, irregardless of whether they came from other functions.


I tried to skip blocks from f1(), but then we'd try to expand the
initializer of its argument and lose.  It appears to me that we should
be emitting debugging info for f1() just like for f2(), but the test
for the block abstractness prevents that: blocks of inlined functions
into inlined functions will always be marked as abstract.

I'm now testing two different patches for the problem.  The first
removes the test for abstractness of the block; the second simply
arranges for us to generate declarations of the block.  The latter is
enough to prevent the crash, and it seems reasonable under the
principle that, if we don't generate debugging info for the whole
block, we must at least generate it for the initializers of the
parameters, as we'd have done had we not chosen to generate debugging
info for the function.

However, it appears to me that the former should be the right
approach, since I don't see why handling f1 should be any different
from f2.  We might emit more debugging info than necessary, though
(but I'm not really sure about it).  Anyhow, it actually fails to
bootstrap, as some labels referenced in the debugging info don't
make it to the output.  So I'm leaning towards the second patch, but
if others agree the former would be better, I'll dig deeper :-)

For reference, here's the testcase I'm going to install in
gcc.dg/debug (thanks Jakub!) as soon as a patch for the bug is
approved, and the two candidate patches I'm testing (the first one
actually fails to bootstrap, while the second is doing well, but I
have doubts as to the correctness of the generated info).  ChangeLog
entries missing because this is not a patch submission, just a RFC.
So...  any comments?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 20020224-1.c --]
[-- Type: text/x-c, Size: 686 bytes --]

/* { dg-do compile } */

#define UNUSED __attribute__((unused))
#define EXT __extension__

int undef(void);

inline static void
f1 (int i UNUSED)
{
}

inline static void
f2 (void)
{
  f1 (EXT ({ int t1 UNUSED; undef (); }));
}

inline static void
f3 (void)
{
  int v1 UNUSED;
  int v2 UNUSED;

  EXT ({ int t2 UNUSED; if (0) undef (); 0; })
    && EXT ({ int t3 UNUSED; if (0) undef (); 0; });

  if (1)
    {
      undef ();
      if (1)
        f2 ();
    }

  {
    undef ();
  }
}

inline static void
f4 (void)
{
  EXT ({ undef (); 1; }) && EXT ({ int t4 UNUSED = ({ 1; }); 1; });

  { }

  EXT ({ int t5 UNUSED; if (0) undef (); 0; });

  f4 ();

  undef ();
  f3 ();

  return;
}

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: dwarf2-inlined-full.patch --]
[-- Type: text/x-patch, Size: 556 bytes --]

Index: gcc/dwarf2out.c
===================================================================
RCS file: /home/aoliva/cygnus/uberbaum/gcc/dwarf2out.c,v
retrieving revision 1.356
diff -u -p -r1.356 dwarf2out.c
--- gcc/dwarf2out.c 21 Feb 2002 23:03:14 -0000 1.356
+++ gcc/dwarf2out.c 25 Feb 2002 00:14:58 -0000
@@ -10554,7 +10554,7 @@ gen_inlined_subroutine_die (stmt, contex
      dw_die_ref context_die;
      int depth;
 {
-  if (! BLOCK_ABSTRACT (stmt))
+  if (1)
     {
       dw_die_ref subr_die
 	= new_die (DW_TAG_inlined_subroutine, context_die, stmt);

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: dwarf2-inlined-decls.patch --]
[-- Type: text/x-patch, Size: 590 bytes --]

Index: gcc/dwarf2out.c
===================================================================
RCS file: /home/aoliva/cygnus/uberbaum/gcc/dwarf2out.c,v
retrieving revision 1.356
diff -u -p -r1.356 dwarf2out.c
--- gcc/dwarf2out.c 21 Feb 2002 23:03:14 -0000 1.356
+++ gcc/dwarf2out.c 25 Feb 2002 00:06:30 -0000
@@ -10574,6 +10574,8 @@ gen_inlined_subroutine_die (stmt, contex
       decls_for_scope (stmt, subr_die, depth);
       current_function_has_inlines = 1;
     }
+  else
+    decls_for_scope (stmt, context_die, depth);
 }
 
 /* Generate a DIE for a field in a record, or structure.  */

[-- Attachment #5: Type: text/plain, Size: 289 bytes --]


-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                  aoliva@{cygnus.com, redhat.com}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist                Professional serial bug killer

Re: Test distilled from -gdwarf-2 -O3 bootstrap failure

[Alexandre Oliva]

[-- Attachment #1: Type: text/plain, Size: 3017 bytes --]

On Feb 24, 2002, Alexandre Oliva <aoliva@redhat.com> wrote:

> Ok, I've figured out what's going on.  In the first round of expanding
> C bodies, f3() is deemed too big to be tree-inlined, so generation of
> RTL is not deferred for it, but generation of assembly is.  f4() is
> compiled to RTL and then assembly is generated for it, because it's
> referenced (by itself :-(, so f3() has to be emitted out-of-line
> because it couldn't be inlined.

> In the second round(), we generate RTL for f1() and f2(), but they are
> never emitted because they are inlined.  After generating assembly for
> f3(), however, we get in trouble because:

> - the inlined block from f1() contains the statement expression passed
>   as an argument from f2

> - the block copied from f1() was marked as ABSTRACT when we started
>   generating debugging info for the inlined f2:
>   dwarf2out_abstract_function() set the abstract flag in all blocks
>   within f2, irregardless of whether they came from other functions.


> I tried to skip blocks from f1(), but then we'd try to expand the
> initializer of its argument and lose.  It appears to me that we should
> be emitting debugging info for f1() just like for f2(), but the test
> for the block abstractness prevents that: blocks of inlined functions
> into inlined functions will always be marked as abstract.

> I'm now testing two different patches for the problem.  The first
> removes the test for abstractness of the block; the second simply
> arranges for us to generate declarations of the block.  The latter is
> enough to prevent the crash, and it seems reasonable under the
> principle that, if we don't generate debugging info for the whole
> block, we must at least generate it for the initializers of the
> parameters, as we'd have done had we not chosen to generate debugging
> info for the function.

> However, it appears to me that the former should be the right
> approach, since I don't see why handling f1 should be any different
> from f2.  We might emit more debugging info than necessary, though
> (but I'm not really sure about it).  Anyhow, it actually fails to
> bootstrap, as some labels referenced in the debugging info don't
> make it to the output.  So I'm leaning towards the second patch, but
> if others agree the former would be better, I'll dig deeper :-)

> For reference, here's the testcase I'm going to install in
> gcc.dg/debug (thanks Jakub!) as soon as a patch for the bug is
> approved, and the two candidate patches I'm testing (the first one
> actually fails to bootstrap, while the second is doing well, but I
> have doubts as to the correctness of the generated info).  ChangeLog
> entries missing because this is not a patch submission, just a RFC.
> So...  any comments?

Here's another alternative I'm trying right now.  It appears to be the
better than both I posted earlier, and it has just completed bootstrap
on athlon-pc-linux-gnu with BOOT_CFLAGS='-O3 -g'.  Ok to install if no
regressions are introduced in the testsuite?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: dwarf2-inlined-lexblock.patch --]
[-- Type: text/x-patch, Size: 2921 bytes --]

Index: gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* dwarf2out.c (gen_inlined_subroutine_die): If block is abstract,
	generate a die for the lexical block.

Index: gcc/dwarf2out.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/dwarf2out.c,v
retrieving revision 1.356
diff -u -p -r1.356 dwarf2out.c
--- gcc/dwarf2out.c 2002/02/21 23:03:14 1.356
+++ gcc/dwarf2out.c 2002/02/25 02:57:09
@@ -10574,6 +10574,20 @@ gen_inlined_subroutine_die (stmt, contex
       decls_for_scope (stmt, subr_die, depth);
       current_function_has_inlines = 1;
     }
+  else
+    /* We may get here if we're the outer block of function A that was
+       inlined into function B that was inlined into function C.  When
+       generating debugging info for C, dwarf2out_abstract_function(B)
+       would mark all inlined blocks as abstract, including this one.
+       So, we wouldn't (and shouldn't) expect labels to be generated
+       for this one.  Instead, just emit debugging info for
+       declarations within the block.  This is particularly important
+       in the case of initializers of arguments passed from B to us:
+       if they're statement expressions containing declarations, we
+       wouldn't generate dies for their abstract variables, and then,
+       when generating dies for the real variables, we'd die (pun
+       intended :-)  */
+    gen_lexical_block_die (stmt, context_die, depth);
 }
 
 /* Generate a DIE for a field in a record, or structure.  */
Index: gcc/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* gcc.dg/debug/20020224-1.c: New.

Index: gcc/testsuite/gcc.dg/debug/20020224-1.c
===================================================================
RCS file: 20020224-1.c
diff -N 20020224-1.c
--- /dev/null	Tue May  5 13:32:27 1998
+++ gcc/testsuite/gcc.dg/debug/20020224-1.c Sun Feb 24 18:57:15 2002
@@ -0,0 +1,60 @@
+/* { dg-do compile } */
+
+/* Here's the deal: f3 is not inlined because it's too big, but f2 and
+   f1 are inlined into it.  We used to fail to emit debugging info for
+   t1, because it was moved inside the (inlined) block of f1, marked
+   as abstract, then we'd crash.  */
+
+#define UNUSED __attribute__((unused))
+#define EXT __extension__
+
+int undef(void);
+
+inline static void
+f1 (int i UNUSED)
+{
+}
+
+inline static void
+f2 (void)
+{
+  f1 (EXT ({ int t1 UNUSED; undef (); }));
+}
+
+inline static void
+f3 (void)
+{
+  int v1 UNUSED;
+  int v2 UNUSED;
+
+  EXT ({ int t2 UNUSED; if (0) undef (); 0; })
+    && EXT ({ int t3 UNUSED; if (0) undef (); 0; });
+
+  if (1)
+    {
+      undef ();
+      if (1)
+        f2 ();
+    }
+
+  {
+    undef ();
+  }
+}
+
+inline static void
+f4 (void)
+{
+  EXT ({ undef (); 1; }) && EXT ({ int t4 UNUSED = ({ 1; }); 1; });
+
+  { }
+
+  EXT ({ int t5 UNUSED; if (0) undef (); 0; });
+
+  f4 ();
+
+  undef ();
+  f3 ();
+
+  return;
+}

[-- Attachment #3: Type: text/plain, Size: 289 bytes --]


-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                  aoliva@{cygnus.com, redhat.com}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist                Professional serial bug killer

Re: Test distilled from -gdwarf-2 -O3 bootstrap failure

[Richard Henderson]

On Mon, Feb 25, 2002 at 12:35:06AM -0300, Alexandre Oliva wrote:
> 	* dwarf2out.c (gen_inlined_subroutine_die): If block is abstract,
> 	generate a die for the lexical block.

I guess this is ok.  I can't think of any way else to handle it.


r~