Re: [PATCH] Fix tree inlining ICE (take 2)

Re: [PATCH] Fix tree inlining ICE (take 2)

[Jakub Jelinek]

On Tue, Nov 20, 2001 at 09:18:14AM -0800, Richard Henderson wrote:
> On Tue, Nov 20, 2001 at 12:35:30PM +0100, Jakub Jelinek wrote:
> > Here is a better one. The DECL_RESULT check is there so that we're sure
> > start_function has been called and not undone by duplicate_decls.
> 
> Hum.  What is DECL_RESULT for a void function?

Something like:

<result_decl 0x401b4828 type <void_type 0x4019e7b4 void>
        VOID file x.c line 2
        align 8 context <function_decl 0x401b47b4 foo>>

	Jakub

[PATCH] Fix tree inlining ICE (take 2)

[Jakub Jelinek]

Hi!

My patch from yesterday did not work with the 20011119-1.c testcase I
included at -O3.
Here is a better one. The DECL_RESULT check is there so that we're sure
start_function has been called and not undone by duplicate_decls.
Bootstrapped, no regressions. Ok to commit?

2001-11-20  Jakub Jelinek  <jakub@redhat.com>

	* c-decl.c (c_expand_deferred_function): Only call c_expand_body
	if fndecl is still DECL_INLINE and has DECL_RESULT.

	* gcc.c-torture/compile/20011119-1.c: New test.
	* gcc.c-torture/compile/20011119-2.c: New test.

--- gcc/testsuite/gcc.c-torture/compile/20011119-1.c.jj	Tue Nov 20 00:32:50 2001
+++ gcc/testsuite/gcc.c-torture/compile/20011119-1.c	Tue Nov 20 00:32:50 2001
@@ -0,0 +1,5 @@
+extern inline int foo (void)
+{
+  return 23;
+}
+extern int foo (void) __attribute__ ((weak, alias ("xxx")));
--- gcc/testsuite/gcc.c-torture/compile/20011119-2.c.jj	Tue Nov 20 00:32:50 2001
+++ gcc/testsuite/gcc.c-torture/compile/20011119-2.c	Tue Nov 20 00:32:50 2001
@@ -0,0 +1,13 @@
+extern inline int foo (void)
+{
+  return 23;
+}
+int bar (void)
+{
+  return foo ();
+}
+extern int foo (void) __attribute__ ((weak, alias ("xxx")));
+int baz (void)
+{
+  return foo ();
+}
--- gcc/c-decl.c.jj	Tue Nov 20 00:31:17 2001
+++ gcc/c-decl.c	Tue Nov 20 13:25:43 2001
@@ -6768,8 +6768,13 @@ void
 c_expand_deferred_function (fndecl)
      tree fndecl;
 {
-  c_expand_body (fndecl, 0, 0);
-  current_function_decl = NULL;
+  /* DECL_INLINE or DECL_RESULT might got cleared after the inline
+     function was deferred, e.g. in duplicate_decls.  */
+  if (DECL_INLINE (fndecl) && DECL_RESULT (fndecl))
+    {
+      c_expand_body (fndecl, 0, 0);
+      current_function_decl = NULL;
+    }
 }
 
 /* Generate the RTL for the body of FNDECL.  If NESTED_P is non-zero,


	Jakub

Re: [PATCH] Fix tree inlining ICE (take 2)

[Richard Henderson]

On Tue, Nov 20, 2001 at 12:35:30PM +0100, Jakub Jelinek wrote:
> 	* c-decl.c (c_expand_deferred_function): Only call c_expand_body
> 	if fndecl is still DECL_INLINE and has DECL_RESULT.
> 
> 	* gcc.c-torture/compile/20011119-1.c: New test.
> 	* gcc.c-torture/compile/20011119-2.c: New test.

Ok.


r~

[PATCH] Fix tree inlining ICE

[Jakub Jelinek]

Hi!

The following patch fixes
http://gcc.gnu.org/ml/gcc-bugs/2001-11/msg00428.html
The issue is that if duplicate_decls overrides an inline function which has
been deferred, c_expand_body is called on a fndecl which might not have
finish_function called at all.
Ok to commit, provided it bootstraps and creates no regressions?

2001-11-19  Jakub Jelinek  <jakub@redhat.com>

	* c-decl.c (c_expand_deferred_function): Only call c_expand_body
	if fndecl is still DECL_INLINE.

	* gcc.c-torture/compile/20011119-1.c: New test.
	* gcc.c-torture/compile/20011119-2.c: New test.

--- gcc/testsuite/gcc.c-torture/compile/20011119-1.c.jj	Mon Nov 19 18:04:04 2001
+++ gcc/testsuite/gcc.c-torture/compile/20011119-1.c	Mon Nov 19 18:00:30 2001
@@ -0,0 +1,5 @@
+extern inline int foo (void)
+{
+  return 23;
+}
+extern int foo (void) __attribute__ ((weak, alias ("xxx")));
--- gcc/testsuite/gcc.c-torture/compile/20011119-2.c.jj	Mon Nov 19 18:04:04 2001
+++ gcc/testsuite/gcc.c-torture/compile/20011119-2.c	Mon Nov 19 18:00:30 2001
@@ -0,0 +1,13 @@
+extern inline int foo (void)
+{
+  return 23;
+}
+int bar (void)
+{
+  return foo ();
+}
+extern int foo (void) __attribute__ ((weak, alias ("xxx")));
+int baz (void)
+{
+  return foo ();
+}
--- gcc/c-decl.c.jj	Thu Nov 15 11:37:54 2001
+++ gcc/c-decl.c	Mon Nov 19 17:58:43 2001
@@ -6765,8 +6765,13 @@ void
 c_expand_deferred_function (fndecl)
      tree fndecl;
 {
-  c_expand_body (fndecl, 0, 0);
-  current_function_decl = NULL;
+  /* DECL_INLINE might got cleared since the inline function was deferred,
+     e.g. in duplicate_decls.  */
+  if (DECL_INLINE (fndecl))
+    {
+      c_expand_body (fndecl, 0, 0);
+      current_function_decl = NULL;
+    }
 }
 
 /* Generate the RTL for the body of FNDECL.  If NESTED_P is non-zero,


	Jakub

Re: [PATCH] Fix tree inlining ICE (take 2)

[Richard Henderson]

On Tue, Nov 20, 2001 at 12:35:30PM +0100, Jakub Jelinek wrote:
> Here is a better one. The DECL_RESULT check is there so that we're sure
> start_function has been called and not undone by duplicate_decls.

Hum.  What is DECL_RESULT for a void function?


r~

Re: [PATCH] Fix tree inlining ICE

[Gerald Pfeifer]

On Tue, 23 Oct 2001, Jakub Jelinek wrote:
> BTW: I have another issue with C tree inlining. The default
> -finline-limit=600 is too low for glibc and causes glibc ld.so not to work
> at all (glibc in one place relies on some nested static inline functions
> beeing inlined). Experimentation showed that -finline-limit=1500 works (both
> elf_get_dynamic_info and elf_do_machine_rel get inlined).
> Now, what do you think is a good solution for this? Hardcoding
> -finline-limit=1500 into glibc CFLAGS is IMHO not the right solution, since
> the meaning of this switch keeps changing (was number of RTL insns, now
> something completely else, etc.). Either -finline-limit could have different
> defaults for each language, or the lang hook which is used currently for not
> honouring inline-limit in some cases at all could be changed [...]

The proper fix is a completely different one: Fix GCC so that the default
limit for *all* languages can be increased substantially.

The limit is as low as it currently is because compile-time went through
the roof due to inefficient code generation/optimization passes.

Gerald
-- 
Gerald "Jerry" pfeifer@dbai.tuwien.ac.at http://www.dbai.tuwien.ac.at/~pfeifer/

Re: [PATCH] Fix tree inlining ICE

[Neil Booth]

Jakub Jelinek wrote:-

> Hi!

Hi Jakub!

> +extern void foo (char *x);
> +void bar (void);
> +void bar (void)
> +{
> +  auto void baz (void);
> +  void baz (void)
> +    {
> +      char tmp[2];
> +      foo (tmp);
> +    }
> +  baz ();
> +}

Would you please put a one or two line comment in your testcases
explaining what they are testing?  It's pretty opaque otherwise.

Thanks,

Neil.

Re: [PATCH] Fix tree inlining ICE

[Richard Henderson]

On Tue, Oct 23, 2001 at 02:55:03PM +0200, Jakub Jelinek wrote:
> 	* c-decl.c (finish_decl): Don't add DECL_STMTs for nested function
> 	prototypes.
> 	* gcc.c-torture/compile/20011023-1.c: New test.

Ok.


r~

Re: [PATCH] Fix tree inlining ICE

[Richard Henderson]

On Tue, Oct 23, 2001 at 02:55:03PM +0200, Jakub Jelinek wrote:
> BTW: I have another issue with C tree inlining. The default
> -finline-limit=600 is too low for glibc and causes glibc ld.so not to work
> at all (glibc in one place relies on some nested static inline functions
> beeing inlined).

FWIW, "extern inline" is currently set to disable the inline limit.
Whether that's a good idea or not, I'm not sure.


r~

[PATCH] Fix tree inlining ICE

[Jakub Jelinek]

Hi!

The following testcase ICEs with -O3 (but goes away when the auto nested
function prototype is removed; but it cannot be removed because otherwise
-Wmissing-prototypes warns about it). The problem is we end up
with two different DECL_STMTs with the same DECL_STMT_EXPR (baz) and tree
inliner is very confused about it. This patch makes sure DECL_STMT is added
for the actual nested fn definition only.
What was broken before and keeps being broken is:
void bar (void) { auto void baz (void); baz ();  }
which IMHO should generate error, the question is where exactly the error
should be diagnosed.
Ok to commit if it bootstraps and creates no regressions?

BTW: I have another issue with C tree inlining. The default
-finline-limit=600 is too low for glibc and causes glibc ld.so not to work
at all (glibc in one place relies on some nested static inline functions
beeing inlined). Experimentation showed that -finline-limit=1500 works (both
elf_get_dynamic_info and elf_do_machine_rel get inlined).
Now, what do you think is a good solution for this? Hardcoding
-finline-limit=1500 into glibc CFLAGS is IMHO not the right solution, since
the meaning of this switch keeps changing (was number of RTL insns, now
something completely else, etc.). Either -finline-limit could have different
defaults for each language, or the lang hook which is used currently for not
honouring inline-limit in some cases at all could be changed, so that it
could return a new limit computed from the -finline-limit parameter and
properties of function being inlined. I think it makes sense to say that
functions marked by user with inline keyword should be attempted to inline
harder than just some arbitrary short function at -O3, and C nested
inline functions should be attempted even harder (e.g. due to all the
bookkeeping needed for nested function calls if they are not inlined).

2001-10-23  Jakub Jelinek  <jakub@redhat.com>

	* c-decl.c (finish_decl): Don't add DECL_STMTs for nested function
	prototypes.

	* gcc.c-torture/compile/20011023-1.c: New test.

--- gcc/c-decl.c.jj	Mon Oct 22 11:06:59 2001
+++ gcc/c-decl.c	Tue Oct 23 14:27:45 2001
@@ -3731,7 +3731,8 @@ finish_decl (decl, init, asmspec_tree)
 		SET_DECL_ASSEMBLER_NAME (decl, get_identifier (asmspec));
 	    }
 
-	  add_decl_stmt (decl);
+	  if (TREE_CODE (decl) != FUNCTION_DECL)
+	    add_decl_stmt (decl);
 	}
 
       if (DECL_CONTEXT (decl) != 0)
--- gcc/testsuite/gcc.c-torture/compile/20011023-1.c.jj	Tue Oct 23 13:58:40 2001
+++ gcc/testsuite/gcc.c-torture/compile/20011023-1.c	Tue Oct 23 14:38:38 2001
@@ -0,0 +1,12 @@
+extern void foo (char *x);
+void bar (void);
+void bar (void)
+{
+  auto void baz (void);
+  void baz (void)
+    {
+      char tmp[2];
+      foo (tmp);
+    }
+  baz ();
+}

	Jakub