[Bug regression/49139] New: always_inline attribute inconsistencies

[Bug regression/49139] New: always_inline attribute inconsistencies

[chrbr at gcc dot gnu.org]

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

           Summary: always_inline attribute inconsistencies
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: minor
          Priority: P3
         Component: regression
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: chrbr@gcc.gnu.org


Created attachment 24340
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24340
fails even without -Winline

Hello,

I'm wondering whether or not the always_inline in PIC must prevails over the
general preemption rules. Apparently not accordingly to the default behavior
for the compilation in -O2 -fpic of

__attribute((always_inline)) void bar() { }

void test(void)
{
  bar();
}

foo.c:1:35: sorry, unimplemented: inlining failed in call to 'bar': function
body can be overwriten at linktime
foo.c:5:6: sorry, unimplemented: called from here

First (1): inlining cannot be done, So without -Werror, this should be a
warning, not a "sorry error". 

Or (2): The function could be inlined, despite the preemption rule. But it is
just not yet implemented. In this case.

In case of (1), I'm attaching here a C++ reduced piece of code that triggers
this error even without using -Winline, making the error quite unpredictable,
since this depends of the presence of another inlined call (here, "Alloc"),
forcing a call to "expand_call_inline" producing the same effect than -Winline
(as shown with a breakpoint in ipa-inline-transform:306 on trunk)

So which one of the strategies is good: Fixing the warning, and make sure that
it only (and consistently) occurs when -Winline is used, or always honoring the
always_inline attribute despite visibility potential issues, considering that a
user directive bypass the defaults ?

Thanks, 
Christian

[Bug regression/49139] always_inline attribute inconsistencies

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-05-24 12:54:20 UTC ---
You forgot to add inline keyword, always_inline attribute makes no sense on
functions without inline keyword (or C++ methods defined in class).

[Bug regression/49139] always_inline attribute inconsistencies

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |INVALID

--- Comment #2 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-05-24 13:05:51 UTC ---
And for 1), with always_inline you indicate that it is a bug to not inline
this function and inlining is required for correctness.  So we can't just
warn.

[Bug regression/49139] always_inline attribute inconsistencies

[chrbr at gcc dot gnu.org]

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

--- Comment #3 from chrbr at gcc dot gnu.org 2011-05-24 14:47:54 UTC ---
Thanks a lot, that's clear. This makes non-sense not to use inline with this
attribute. Accordingly not a bug.

So I am wondering if we could be more helpful to the users by changing the
"sorry, unimplemented" error message by something more explicit, something like
"invalid attribute without inline", and more consistent among the cases ?

Looking on a few opensources packages, I see a few usages of this attribute
without the inline keyword (e.g, the gcc testsuite, some target dl-machine.h in
the glibc). This is not frequent, I admit, but enough to be confusing.

[Bug middle-end/49139] always_inline attribute inconsistencies

[manu at gcc dot gnu.org]

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

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
           Keywords|                            |diagnostic
   Last reconfirmed|                            |2011.05.24 15:12:15
          Component|regression                  |middle-end
                 CC|                            |manu at gcc dot gnu.org
         Resolution|INVALID                     |
     Ever Confirmed|0                           |1
           Severity|minor                       |enhancement

--- Comment #4 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2011-05-24 15:12:15 UTC ---
Using "sorry" for this doesn't make any sense. I'd would like to have time to
fix this, but I don't.

Christian, I would suggest that you put forward a patch for comments including
self-contained testcases.

[Bug middle-end/49139] always_inline attribute inconsistencies

[chrbr at gcc dot gnu.org]

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

--- Comment #5 from chrbr at gcc dot gnu.org 2011-05-24 15:39:18 UTC ---
(In reply to comment #4)
> Using "sorry" for this doesn't make any sense. I'd would like to have time to
> fix this, but I don't.
> 
> Christian, I would suggest that you put forward a patch for comments including
> self-contained testcases.

OK, I have the testcases, I will try a patch in the light of the semantic
clarification from Jacub and Richard.

[Bug middle-end/49139] always_inline attribute inconsistencies on failure

[chrbr at gcc dot gnu.org]

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

--- Comment #7 from chrbr at gcc dot gnu.org 2011-05-27 14:37:42 UTC ---
Created attachment 24373
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24373
testcase 1

[Bug middle-end/49139] always_inline attribute inconsistencies

[chrbr at gcc dot gnu.org]

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

chrbr at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot       |chrbr at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #6 from chrbr at gcc dot gnu.org 2011-05-27 14:34:05 UTC ---
Created attachment 24372
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24372
always generates an error on always_inline failure

The attached testcases illustrate the current diagnostics on always_inline
failures"

- success with and without -Winline, although always_inline is not honored
   gcc fail_always_inline1.c -S -Winline -O0 -fpic 
   gcc fail_always_inline1.c -S -O2 -fpic 

- error: "sorry, unimplemented:" with -Winline only
   gcc fail_always_inline1.c -S  -Winline -O2 -fpic

-  error: "sorry, unimplemented" without -Winline
   gcc fail_always_inline2.c -S -fno-early-inlining -O2
   or the original c++ attachment in this defect

The attached patch always emits an error (and change sorry into error). In
particular it fixes the case where the inlining was not honored and not
detected. 
note that the error is emitted even if -Winline is not specified. Since this
reflects a misuse of the attribute and it is close to the actual behavior

Note that I stepped back on my initial proposal to emit a hint message
suggesting to use the "inline" keyword (if not defined in a C++ class), because
there are cases where even with "inline" the function would not be inlined.

Tested with standard bootstrap and regression testing on x86 (with tests
modification the diagnostic checks). I also double checked for legacy issues
with a full rebuild of a linux distribution.

Thanks,

[Bug middle-end/49139] always_inline attribute inconsistencies on failure

[chrbr at gcc dot gnu.org]

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

--- Comment #8 from chrbr at gcc dot gnu.org 2011-05-27 14:38:13 UTC ---
Created attachment 24374
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24374
testcase 2

[Bug middle-end/49139] always_inline attribute inconsistencies on failure

[manu at gcc dot gnu.org]

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

--- Comment #9 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2011-05-27 15:08:17 UTC ---
You patch looks fine to me but I cannot approve it. You'll need to submit it to
gcc-patches for review and approval. You'll need also a Changelog. If you don't
have svn access, you should mention in your emails that you need someone to
commit the patch for you.

See also http://gcc.gnu.org/contribute.html#patches

[Bug middle-end/49139] always_inline attribute inconsistencies on failure

[chrbr at gcc dot gnu.org]

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

--- Comment #10 from chrbr at gcc dot gnu.org 2011-06-21 06:42:08 UTC ---
Author: chrbr
Date: Tue Jun 21 06:42:05 2011
New Revision: 175239

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=175239
Log:
PR middle-end/49139 fix always_inline diagnostics

Added:
    trunk/gcc/testsuite/gcc.dg/fail_always_inline.c
Modified:
    trunk/gcc/cgraphunit.c
    trunk/gcc/ipa-inline-transform.c
    trunk/gcc/testsuite/g++.dg/ipa/devirt-7.C
    trunk/gcc/testsuite/gcc.dg/20051201-1.c
    trunk/gcc/testsuite/gcc.dg/always_inline.c
    trunk/gcc/testsuite/gcc.dg/always_inline2.c
    trunk/gcc/testsuite/gcc.dg/always_inline3.c
    trunk/gcc/testsuite/gcc.dg/debug/pr41264-1.c
    trunk/gcc/testsuite/gcc.dg/inline-22.c
    trunk/gcc/testsuite/gcc.dg/lto/20090218-1_0.c
    trunk/gcc/testsuite/gcc.dg/lto/20090218-1_1.c
    trunk/gcc/testsuite/gcc.dg/torture/pta-structcopy-1.c
    trunk/gcc/testsuite/gcc.dg/uninit-pred-5_a.c
    trunk/gcc/testsuite/gcc.dg/uninit-pred-5_b.c
    trunk/gcc/tree-inline.c

[Bug middle-end/49139] always_inline attribute inconsistencies on failure

[chrbr at gcc dot gnu.org]

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

--- Comment #11 from chrbr at gcc dot gnu.org 2011-06-21 06:43:28 UTC ---
Author: chrbr
Date: Tue Jun 21 06:43:26 2011
New Revision: 175240

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=175240
Log:
PR middle-end/49139 fix always_inline diagnostics

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog

[Bug middle-end/49139] always_inline attribute inconsistencies on failure

[chrbr at gcc dot gnu.org]

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

--- Comment #12 from chrbr at gcc dot gnu.org 2011-06-21 06:45:09 UTC ---
Author: chrbr
Date: Tue Jun 21 06:45:05 2011
New Revision: 175241

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=175241
Log:
PR middle-end/49139 PR other/43564 make sure the inline function is inlined

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ipa-inline.c

[Bug middle-end/49139] always_inline attribute inconsistencies on failure

[chrbr at gcc dot gnu.org]

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

--- Comment #13 from chrbr at gcc dot gnu.org 2011-06-21 06:48:50 UTC ---
Author: chrbr
Date: Tue Jun 21 06:48:45 2011
New Revision: 175242

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=175242
Log:
PR middle-end/49139 fix always_inline diagnostics

Added:
    trunk/gcc/testsuite/gcc.dg/fail_always_inline2.c
Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.dg/inline_1.c
    trunk/gcc/testsuite/gcc.dg/inline_2.c
    trunk/gcc/testsuite/gcc.dg/inline_3.c
    trunk/gcc/testsuite/gcc.dg/inline_4.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/pr40087.c

[Bug middle-end/49139] always_inline attribute inconsistencies on failure

[chrbr at gcc dot gnu.org]

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

chrbr at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |FIXED

--- Comment #14 from chrbr at gcc dot gnu.org 2011-07-11 13:02:22 UTC ---
as discussed
-emits a warning when __attribute__((always_inline)) is used without inline
keyword
-emits an error in an always_inline function couldn't be inlined.
-consistently behaves, with or without -Winline