[Bug c/33763] [4.4/4.5/4.6/4.7 Regression] Bogus inlining failed in call to `xxx': redefined extern inline functions are not considered for inlining

["rguenth at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33763

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
                 CC|                            |jsm28 at gcc dot gnu.org
          Component|tree-optimization           |c

--- Comment #23 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-01-12 13:42:39 UTC ---
Shorter testcase

extern __inline __attribute__ ((__always_inline__,__gnu_inline__))
void open ()
{
}
void open ()
{
  open ();
}

fails on trunk like

> ./cc1 -quiet t.c -O
t.c: In function 'open':
t.c:5:6: error: inlining failed in call to always_inline 'open': function not
inlinable
t.c:7:8: error: called from here

and creates wrong code at -O0 (and probably would, at -On):

open:
.LFB1:
        .cfi_startproc
        pushq   %rbp
        .cfi_def_cfa_offset 16
        .cfi_offset 6, -16
        movq    %rsp, %rbp
        .cfi_def_cfa_register 6
        movl    $0, %eax
        call    open
        popq    %rbp
        .cfi_def_cfa 7, 8
        ret

as the always-inline body was not inlined.

Note that the initial callgraph is already wrong:

Initial callgraph:

open/0 @0x7ffff5a2b6c0 (asm: open) analyzed needed reachable body finalized
redefined_extern_inline
  called by: open/0 (1.00 per call)
  calls: open/0 (1.00 per call)
  References:
  Refering this function:
variable pool:

as said, the C frontend needs to create two decls for open for this to
properly work.

OTOH, it is time to deprecate this extension and warn about it (after
all we miscompile this since quite some time, GCC 3.3 and 4.1 already produce
the recursive open - how was this intended to work? ...)  I don't have
3.2 (and 2.95 does not have always_inline).

The "proper" way to write the testcase is

extern __inline __attribute__ ((__always_inline__,__gnu_inline__))
void open ()
{
}
void open_1 () asm("open");
void open_1 ()
{
  open ();
}

So, Joseph - would you be fine with changing behavior for this testcase
from ICEing at -On, miscompiling at -O0 to rejecting it?  Thus,

Index: c-decl.c
===================================================================
--- c-decl.c    (revision 183121)
+++ c-decl.c    (working copy)
@@ -1855,21 +1855,10 @@ diagnose_mismatched_decls (tree newdecl,
        {
          if (DECL_INITIAL (olddecl))
            {
-             /* If both decls are in the same TU and the new declaration
-                isn't overriding an extern inline reject the new decl.
+             /* If both decls are in the same TU reject the new decl.
                 In c99, no overriding is allowed in the same translation
                 unit.  */
-             if ((!DECL_EXTERN_INLINE (olddecl)
-                  || DECL_EXTERN_INLINE (newdecl)
-                  || (!flag_gnu89_inline
-                      && (!DECL_DECLARED_INLINE_P (olddecl)
-                          || !lookup_attribute ("gnu_inline",
-                                                DECL_ATTRIBUTES (olddecl)))
-                      && (!DECL_DECLARED_INLINE_P (newdecl)
-                          || !lookup_attribute ("gnu_inline",
-                                                DECL_ATTRIBUTES (newdecl))))
-                 )
-                 && same_translation_unit_p (newdecl, olddecl))
+             if (same_translation_unit_p (newdecl, olddecl))
                {
                  error ("redefinition of %q+D", newdecl);
                  locate_old_decl (olddecl);