Created attachment 24340 [details] 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
You forgot to add inline keyword, always_inline attribute makes no sense on functions without inline keyword (or C++ methods defined in class).
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.
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.
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.
(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.
Created attachment 24372 [details] 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,
Created attachment 24373 [details] testcase 1
Created attachment 24374 [details] testcase 2
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
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
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
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
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
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