-Winline does not respect -fno-default-inline (this is split out of bug17115, and a possible 3.3 regression). Consider this code: struct Foo { int a(int r) { return r & 1; } virtual int b(int r) { return r & 2; } static int c(int r) { return r & 4; } }; int bar(int r) { Foo f; int k = 0; k |= f.a(r); k |= f.b(r); k |= f.a(r); return k; } > g++-3.3 -c -O2 -fno-default-inline -Winline test.cpp > g++-3.4 -c -O2 -fno-default-inline -Winline test.cpp test.cpp: In function `int bar(int)': test.cpp:6: warning: inlining failed in call to 'int Foo::a(int)': --param max-inline-insns-single limit reached test.cpp:13: warning: called from here test.cpp:7: warning: inlining failed in call to 'virtual int Foo::b(int)': --param max-inline-insns-single limit reached test.cpp:13: warning: called from here test.cpp:6: warning: inlining failed in call to 'int Foo::a(int)': --param max-inline-insns-single limit reached test.cpp:13: warning: called from here test.cpp:6: warning: inlining failed in call to 'int Foo::a(int)': --param max-inline-insns-single limit reached test.cpp:13: warning: called from here test.cpp:7: warning: inlining failed in call to 'virtual int Foo::b(int)': --param max-inline-insns-single limit reached test.cpp:13: warning: called from here test.cpp:6: warning: inlining failed in call to 'int Foo::a(int)': --param max-inline-insns-single limit reached test.cpp:13: warning: called from here Futhermore, the warnings disappear if you use -O3 instead of -O2, so -O3 seems to silently override -fno-default-inline (at least a documentation bug). Full test program and log file will follow as attachment.
Created attachment 7383 [details] Test program, and log of compilation with various gcc versions.
Confirmed, a 3.4 and 4.0 regression. But note -O3 enables -finline-functions which enables inlining for all functions so that is not a documentation problem at all. : Search converges between 2004-01-01-trunk (#437) and 2004-01-17-trunk (#438).
Mine.
Created attachment 7390 [details] Patch for this bug This is a patch for this bug. I would appreciate some help with testing because I have limited resources so I do not know when I am going to be able to fully test this.
Postponed until GCC 3.4.4.
I eventually managed to test the patch, but there are testsuite failures: FAIL: gcc.dg/debug/dwarf2/dwarf-die7.c scan-assembler 1.*DW_AT_inline FAIL: gcc.dg/winline-2.c (test for warnings, line 4) FAIL: gcc.dg/winline-2.c (test for warnings, line 7) FAIL: gcc.dg/winline-4.c (test for warnings, line 4) FAIL: gcc.dg/winline-4.c (test for warnings, line 7)
FAIL: gcc.dg/debug/dwarf2/dwarf-die7.c scan-assembler 1.*DW_AT_inline Was there before your patch if you are testing on 4.0.0.
Giovanni, your patch actually looks correct to me. It may be just exposing a latent bug in the C front end. I'm not sure but I'd expect that if a function is declared inline as in testsuite/gcc.dg/winline-2.c: /* { dg-do compile } */ /* { dg-options "-Winline -O2" } */ inline int q(void); /* { dg-warning "body not available" "" } */ inline int t(void) { return q(); /* { dg-warning "called from here" "" } */ } then q should be DECL_DECLARED_INLINE - but it looks like it's not, or you shouldn't see those new FAILs.
JSM, do you agree with Steven's analysys? It looks like DECL_DECLARED_INLINE should just mean there, so the C frontend might be wrong in this regard.
Subject: Re: [3.4/4.0 Regression] -Winline does not respect -fno-default-inline On Wed, 24 Nov 2004, giovannibajo at libero dot it wrote: > JSM, do you agree with Steven's analysys? It looks like DECL_DECLARED_INLINE > should just mean there, so the C frontend might be wrong in this regard. I agree the C front end should set DECL_DECLARED_INLINE when a function is declared inline. Before looking for a problem in that regard, make sure that (a) DECL_DECLARED_INLINE is what is checked for the diagnostics in question, (b) it is indeed not set for the relevant decl.
The problem appears to be the other way round. The C frontend mark the declaration as DECL_DECLARED_INLINE but not as DECL_INLINE because the body is not available. The relevant portion of code is this: else if (declspecs->inline_p) { /* Record that the function is declared `inline'. */ DECL_DECLARED_INLINE_P (decl) = 1; /* Do not mark bare declarations as DECL_INLINE. Doing so in the presence of multiple declarations can result in the abstract origin pointing between the declarations, which will confuse dwarf2out. */ if (initialized) { DECL_INLINE (decl) = 1; if (storage_class == csc_extern) current_extern_inline = 1; } The C++ frontend instead does not have this issue: by default, all declarations marked by DECL_DECLARED_INLINE are also marked with DECL_INLINE. In fact, the testcase works even after my patch when compiled with cc1plus. The code has been there since this patch: http://gcc.gnu.org/ml/gcc-cvs/2001- 12/msg00833.html. It fixed PR 5163. I tried removing the if(initialized) and always set DECL_INLINE even for functions with no bodies, and the testcase for the bug still succeeds. Maybe the weirdness in dwarf2out has been fixed since then. RTH, do you think it makes sense to remove that check and set DECL_INLINE even for functions with no bodies in the C frontend? The C++ FE does that.
Patch posted here, waiting for review: http://gcc.gnu.org/ml/gcc-patches/2004-11/msg02664.html
Just a short note that the testcase has a typo and should read ... k |= f.a(r); k |= f.b(r); k |= f.c(r); ...
Subject: Re: [3.4/4.0 Regression] -Winline does not respect -fno-default-inline markus at oberhumer dot com <gcc-bugzilla@gcc.gnu.org> wrote: > Just a short note that the testcase has a > typo and should read > > ... > k |= f.a(r); k |= f.b(r); k |= f.c(r); > ... Right thanks for noticing. Luckily, this does not expose a bug in my patch, which is still valid. I'll make sure to commit the correct testcase when I get to it. Giovanni Bajo
What is the status of the latest patch from Giovanni ?
Removing rejects-valid marker; spurious warnings do not count as rejects-valid.
Moving to 4.0.2 pre Mark.
I'm somewhat confused by the proposed patch in Comment #12. The documentation says -Winline warns about functions *declared* inline, but not actually inlined. I think that makes sense. DECL_INLINE is an internal note to the compiler that it might make sense to inline that function. Warning a user because some small function, not declared inline, was not in fact inlined, seems wrong. So, I'm confused by "what we actually want is to emit a warning for the functions marked with DECL_INLINE". In any case, I think P2 is the right setting for this bug.
Giovanni's patch substitutes DECL_INLINE for DECL_DECLARED_INLINE_P. This breaks -finline-functions, since now we get warnings for functions that weren't originally declared inline. See for instance PR 10929 which is a report about this case. The testcase in 10929 won't fail anymore because of other changes, but it isn't hard to construct a testcase that will fail. Here is one: int foo (int); int main(int,char) { return foo(10); } int foo (int i) { static void *a[3] = {&&foo, &&bar, &&baz}; goto *a[i]; foo: return i; bar: return 0; baz: return -i; } This gives a warning with -O -Winline -finline-functions, but it shouldn't. I believe the correct solution here is to test both DECL_INLINE and DECL_DECLARED_INLINE_P. This seems to solve both problems. There is no warning with -finline-functions, because DECL_DECLARED_INLINE_P won't be set. There is no warning with -fno-default-inline, because DECL_INLINE won't be set. This construct is already used in another place in tree-inline.c. The C front end change doesn't re-introduce the PR 5163 failure because it was a different part of the patch that fixed it. The part that changed "flag_inline_functions" to "flag_inline_function && initialized" which isn't being reverted here. The part that is being reverted will cause the PR 5163 testcase to fail with gcc-3.1 only if we add the inline keyword to both vinit declarations. Investigating, I see that this no longer fails on mainline because of a side-effect of a C front end rewrite patch from Zack. http://gcc.gnu.org/ml/gcc-patches/2004-01/msg02114.html This replaces a DECL_ORIGIN call with a DECL_ABSTRACT_ORIGIN call which eliminates the problem that Richard's patch was fixing. The use of DECL_ORIGIN here was accidentally setting DECL_ABSTRACT_ORIGIN when we shouldn't have been setting it. So I think it is now safe to revert Richard's grokdeclarator change here, except that I think we should revert both parts, instead of just the one part that the proposed patch reverts. Just reverting one part leaves an inconsistency in the code.
This issue will not be resolved in GCC 4.1.0; retargeted at GCC 4.1.1.
Will not be fixed in 4.1.1; adjust target milestone to 4.1.2.
patch break -finline-functions, so it cannot be applied as is. removing patch keyword to avoid confusion with regressions awaiting review.
I think I found out what is going on, although I cannot decide myself what is the correct action. For functions declared within class scope we do: (gcc/cp/decl.c start_method() line 11285) DECL_DECLARED_INLINE_P (fndecl) = 1; if (flag_default_inline) DECL_INLINE (fndecl) = 1; Winline tests DECL_DECLARED_INLINE (which is correct). However, as shown above, fno-default-inline does not change the DECL_DECLARED_INLINE but DECL_INLINE. Solutions: 1) Nothing, things are correct as they are. 2) When we are going to emit the warning, we check for fno-default-inline and check that the function is declared within class scope. I have no idea how to do this or whether it is appropriate. 3) fno-default-inline sets both DECL_DECLARED_INLINE and DECL_INLINE to false. It may break other things and the text of fno-default-inline says that these functions will have linkage like inline functions. 4) Something else.
Created attachment 12372 [details] improved patch for inline warning Works for simple testcases. Needs full bootstrap regression test.
Subject: Re: [4.0/4.1/4.2 Regression] -Winline does not respect -fno-default-inline On Sat, 2006-09-30 at 12:36 +0000, lopezibanez at gmail dot com wrote: > I think I found out what is going on, although I cannot decide myself what is > the correct action. If you look at Comment #19, you will see that I have already provided a solution. However, I see that I failed to add the patch I wrote to the bug report. I will do that now. My work on this patch was interrupted by health problems, and I haven't gotten back to it yet. It is still part of my large backlog. Jim
(In reply to comment #25) > If you look at Comment #19, you will see that I have already provided a > solution. However, I see that I failed to add the patch I wrote to the > bug report. I will do that now. My work on this patch was interrupted > by health problems, and I haven't gotten back to it yet. It is still > part of my large backlog. And if you look at comment #18, you will see that Mark Mitchell has already stated the Winline is suppossed to warn "about functions *declared* inline, but not actually inlined. [...] DECL_INLINE is an internal note to the compiler that it might make sense to inline that function." Your patch will break Winline output for any function that is declared inline ( DECL_DECLARED_INLINE_P (fndecl) = 1;) but it is marked as not making sense to be inlined by the compiler (DECL_INLINE (fndecl) = 0;). From a quick grep, there seems to be a few places where this may occur. Of course, you know more about gcc than me. I was just trying to point out this potential issue.
Subject: Bug number PR middle-end/18071 A patch for this bug has been added to the patch tracker. The mailing list url for the patch is http://gcc.gnu.org/ml/gcc-patches/2007-01/msg01366.html
Open regression with no activity since February 14. Ping?
(In reply to comment #28) > Open regression with no activity since February 14. Ping? > The last thread about this was in: http://gcc.gnu.org/ml/gcc-patches/2007-07/msg00523.html Summarising: * C++ front-end sets DECL_DECLARED_INLINE_P for methods always. It sets DECL_INLINE unless -fno-default-inline. * -Winline warns when a function "declared inlined" is not inlined. Currently, "declared inline" means DECL_DECLARED_INLINE_P is set. * DECL_DECLARED_INLINE_P controls linkage. * -fno-default-inline should not affect linkage. Geoff Keating said: "DECL_DECLARED_INLINE_P in the C++ front-end means 'this function is an "inline function" as defined by the standard'. There are two ways to get a function to be an inline function: (a) the 'inline' keyword, and (b) define the function inside a class; the ways are equivalent from the language's perspective. See [dcl.fct.spec] in the C++ standard, especially paragraphs 2 and 3. The question here is whether -Winline should be limited to those inline functions in case (a) or whether it can include those in case (b) too." Therefore, if -Winline also include case (b) then the warning seems correct. A function that was declared inline was not inlined. Alternatively, we could not warn for -fno-default-inline but then we would have to detect that: 1) -fno-default-inline was given, 2) this is a method, 3) it wasn't declared using the 'inline' keyword. On the other hand, if -Winline is limited to case (a), then we need a way to distinguish between (a) and (b). Therefore, either the warning is correct and this is not a bug, or we need a way to distinguish between (a) and (b). At least, this is how I see. I wouldn't mind to implement other solution if one is clearly described and agreed.
I suggest to tag the DECL with TREE_NO_WARNING if -fno-default-inline is set and check this. On the trunk the functions are inlined anyway, because we inline small functions by default (and the functions are pure anyway). Also trunk tests if (!flag_inline_small_functions && !DECL_DECLARED_INLINE_P (decl)) { if (reason) *reason = N_("function not inline candidate"); return false; that is, it uses DECL_DECLARED_INLINE_P (!) which is not affected by -fno-default-inline. Better testcase: volatile int x; struct Foo { int a(int r) { return r & x; } virtual int b(int r) { return r & x; } static int c(int r) { return r & x; } }; int bar(int r) { Foo f; int k = 0; k |= f.a(r); k |= f.b(r); k |= f.a(r); return k; } which will on the trunk warn with -fno-inline only. With the proposed approach we'd improve this to ./cc1plus -quiet -Winline t.ii -O -fno-default-inline -fno-inline t.ii: In constructor 'Foo::Foo()': t.ii:2: warning: function 'Foo::Foo() throw ()' can never be inlined because it is suppressed using -fno-inline t.ii: In function 'int bar(int)': t.ii:2: warning: inlining failed in call to 'Foo::Foo() throw ()': function not inlinable t.ii:9: warning: called from here that is, the compiler-generated methods are still marked inline and are not affected by -fno-default-inline.
Created attachment 14943 [details] other approach to supress the warnings
I am bit confused by logic of code here. Middle end now ingore DECL_INLINE consistently, it is still arround since it affect some instantiation decisions in C++ FE. Does fno-default-inline work? I think only way to prevent inlining is to make C++ frontend to drop implicit noinline attribute or split the two meanings of DECL_DECLARED_INLINE (ie meaning to drive inliner and to drive linkage). As I understand it now, DECL_DECLARED_INLINE must be always set for functions that are implicitly inline, even with -fno-default-inline, because it affects linkage, right? Honza
Subject: Re: [4.0/4.1/4.2/4.3 Regression] -Winline does not respect -fno-default-inline On Tue, 15 Jan 2008, hubicka at gcc dot gnu dot org wrote: > ------- Comment #32 from hubicka at gcc dot gnu dot org 2008-01-15 15:47 ------- > I am bit confused by logic of code here. Middle end now ingore DECL_INLINE > consistently, it is still arround since it affect some instantiation decisions > in C++ FE. Does fno-default-inline work? I think only way to prevent inlining > is to make C++ frontend to drop implicit noinline attribute or split the two > meanings of DECL_DECLARED_INLINE (ie meaning to drive inliner and to drive > linkage). As I understand it now, DECL_DECLARED_INLINE must be always set for > functions that are implicitly inline, even with -fno-default-inline, because it > affects linkage, right? Yes, this is what I understand. I think we need a new flag specifying if in the source the 'inline' keyword was used and solely use that for inline warning purposes. (That is, I would not expect to get a warning for non-'inline' class methods either, regardless of -fdefault-inline setting). Richard.
Subject: Re: [4.0/4.1/4.2/4.3 Regression] -Winline does not respect -fno-default-inline > Yes, this is what I understand. I think we need a new flag specifying > if in the source the 'inline' keyword was used and solely use that for > inline warning purposes. (That is, I would not expect to get a warning > for non-'inline' class methods either, regardless of -fdefault-inline > setting). Here I tend to be more with Geoff: since C++ standard says that inline keyword is imlicit, I don't see why we should not warn on it and why extra inline keyword should make difference. We don't handle those functions specially in other inlining heuristics and thus it is bit instructive to users to force them realize that they really are inline. But if C++ people preffer to not warn, I definitly won't object. Honza
(In reply to comment #33) > Yes, this is what I understand. I think we need a new flag specifying > if in the source the 'inline' keyword was used and solely use that for > inline warning purposes. (That is, I would not expect to get a warning > for non-'inline' class methods either, regardless of -fdefault-inline > setting). > Then, you would need more than a new flag. Currently the problem is to distinguish 1) the explicit 'inline' keyword and the implicit inline keyword of -fdefault-inline from 2) no inline whatsoever and no inline because of -fno-default-inline Currently: explicit inline: DECL_DECLARED_INLINE fdefault-inline : DECL_DECLARED_INLINE fno-default-inline: DECL_DECLARED_INLINE no inline: (DECL_INLINE is an internal note that the function may be inlinable. Neither explicit inline nor -fdefault-inline actually ensure that DECL_INLINE will be always true). Ideally: explicit inline: DECL_DECLARED_INLINE && DECL_INLINE_LINKAGE fdefault-inline : DECL_DECLARED_INLINE && DECL_INLINE_LINKAGE fno-default-inline: DECL_INLINE_LINKAGE no inline: In your proposal we will need: explicit inline: DECL_EXPLICITLY_DECLARED_INLINE && DECL_DECLARED_INLINE && DECL_INLINE_LINKAGE fdefault-inline : DECL_DECLARED_INLINE && DECL_INLINE_LINKAGE fno-default-inline: DECL_INLINE_LINKAGE no inline:
Closing 4.1 branch.
Subject: Bug 18071 Author: hubicka Date: Wed Sep 17 15:00:59 2008 New Revision: 140418 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=140418 Log: PR c++/18071 * tree.h (DECL_INLINE): remove. (DECL_DECLARED_INLINE_P): Update docs. (DECL_NO_INLINE_WARNING_P): new. (tree_function_decl): Replace inline_flag by no_inline_warning_flag. * tree-inline.c (inlinable_function_p): Set DECL_NO_INLINE_WARNING_P. Java: * class.c (add_method_1): Do not initialize DECL_INLINE. (make_local_function_alias): Likewise. * expr.c (rewrite_arglist_getcaller): Set DECL_UNINLINABLE. * lang.c (java_decl_ok_for_sibcall): Use DECL_UNINLINABLE. Objc: * objc/objc-act.c (objc_finish_method_definition): Do not set DECL_INLINE. C++: * cp/decl.c (start_method): Set DECL_NO_INLINE_WARNING_P. Modified: trunk/gcc/ChangeLog trunk/gcc/cp/ChangeLog trunk/gcc/cp/decl.c trunk/gcc/java/ChangeLog trunk/gcc/java/class.c trunk/gcc/java/expr.c trunk/gcc/java/lang.c trunk/gcc/objc/ChangeLog trunk/gcc/objc/objc-act.c trunk/gcc/tree-inline.c trunk/gcc/tree.h
Fixed by Honza's patch.