| Message ID | a97f955d-a33b-9907-1e32-3b117cadac79@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | deprecate -finline-limit and make it an optimization option (PR 84603) | expand |
On Thu, 1 Mar 2018, Martin Sebor wrote: > While testing my recent changes to the handling of attributes > on C++ templates I noticed that the -finline-limit=N option > is not recognized in attribute or pragma optimize. In response > to the bug I raised, Richard you said the option is deprecated, > so I went ahead and documented the deprecation in the manual > and also added a deprecation warning. I also added it to > Optimization options. The attached patch reflects these > changes. > > I also tried to put together a test to verify that the option > works as expected, both in attributes and in pragmas. I wasn't > able to get it to work reliably or sensibly. > > In C, #pragma optimize ("inline-limit=1") has a global effect > on all functions in the file, even those lexically before the > pragma. In C++, the pragma has no effect. Both of these > effects are contrary to my reading of the manual. > > Attribute optimize ("inline-limit") behaves similarly, which > seems even more unexpected and even more contrary to the manual > which by referring to Function Specific Option Pragmas suggests > that both the pragma and especially the attribute apply > optimizations to just the functions they're specified for. It can't really work so please do _not_ add the option to the set of Optimize options. Inlining is an IPA task so "per function" parameters do not make sense at all. IMHO the bug is simply INVALID. Richard.
On 03/02/2018 01:05 AM, Richard Biener wrote: > On Thu, 1 Mar 2018, Martin Sebor wrote: > >> While testing my recent changes to the handling of attributes >> on C++ templates I noticed that the -finline-limit=N option >> is not recognized in attribute or pragma optimize. In response >> to the bug I raised, Richard you said the option is deprecated, >> so I went ahead and documented the deprecation in the manual >> and also added a deprecation warning. I also added it to >> Optimization options. The attached patch reflects these >> changes. >> >> I also tried to put together a test to verify that the option >> works as expected, both in attributes and in pragmas. I wasn't >> able to get it to work reliably or sensibly. >> >> In C, #pragma optimize ("inline-limit=1") has a global effect >> on all functions in the file, even those lexically before the >> pragma. In C++, the pragma has no effect. Both of these >> effects are contrary to my reading of the manual. >> >> Attribute optimize ("inline-limit") behaves similarly, which >> seems even more unexpected and even more contrary to the manual >> which by referring to Function Specific Option Pragmas suggests >> that both the pragma and especially the attribute apply >> optimizations to just the functions they're specified for. > > It can't really work so please do _not_ add the option to > the set of Optimize options. Inlining is an IPA task so > "per function" parameters do not make sense at all. It does work as I expect for other inlining options (e.g., for -finline-functions or -finline-functions-called-once) so either there's something special/different about -finline-limit or that it works for the others is an accident. Can you please clarify which it is? Either way, wherever they come into play, the limitations you refer to should be mentioned in the GCC warning when an option that can't or may not work is used in attribute or #pragma optimize, and it should also be explained in the manual. How else are users supposed to know? As it is, since -finline-limit is listed among optimization options, and since attribute and #pragma optimize say they accept optimization options, it's a bug that GCC rejects it as if it were invalid. Martin
On 03/02/2018 01:05 AM, Richard Biener wrote: > On Thu, 1 Mar 2018, Martin Sebor wrote: > >> While testing my recent changes to the handling of attributes >> on C++ templates I noticed that the -finline-limit=N option >> is not recognized in attribute or pragma optimize. In response >> to the bug I raised, Richard you said the option is deprecated, >> so I went ahead and documented the deprecation in the manual >> and also added a deprecation warning. I also added it to >> Optimization options. The attached patch reflects these >> changes. >> >> I also tried to put together a test to verify that the option >> works as expected, both in attributes and in pragmas. I wasn't >> able to get it to work reliably or sensibly. >> >> In C, #pragma optimize ("inline-limit=1") has a global effect >> on all functions in the file, even those lexically before the >> pragma. In C++, the pragma has no effect. Both of these >> effects are contrary to my reading of the manual. >> >> Attribute optimize ("inline-limit") behaves similarly, which >> seems even more unexpected and even more contrary to the manual >> which by referring to Function Specific Option Pragmas suggests >> that both the pragma and especially the attribute apply >> optimizations to just the functions they're specified for. > > It can't really work so please do _not_ add the option to > the set of Optimize options. Inlining is an IPA task so > "per function" parameters do not make sense at all. It does work as I expect for other inlining options (e.g., for -finline-functions or -finline-functions-called-once) so either there's something special/different about -finline-limit or that it works for the others is an accident. Can you please clarify which it is? Either way, wherever they come into play, the limitations you refer to should be mentioned in the GCC warning when an option that can't or may not work is used in attribute or #pragma optimize, and it should also be explained in the manual. How else are users supposed to know? As it is, since -finline-limit is listed among optimization options, and since attribute and #pragma optimize say they accept optimization options, it's a bug that GCC rejects it as if it were invalid. Martin
On Sat, 3 Mar 2018, Martin Sebor wrote: > On 03/02/2018 01:05 AM, Richard Biener wrote: > > On Thu, 1 Mar 2018, Martin Sebor wrote: > > > > > While testing my recent changes to the handling of attributes > > > on C++ templates I noticed that the -finline-limit=N option > > > is not recognized in attribute or pragma optimize. In response > > > to the bug I raised, Richard you said the option is deprecated, > > > so I went ahead and documented the deprecation in the manual > > > and also added a deprecation warning. I also added it to > > > Optimization options. The attached patch reflects these > > > changes. > > > > > > I also tried to put together a test to verify that the option > > > works as expected, both in attributes and in pragmas. I wasn't > > > able to get it to work reliably or sensibly. > > > > > > In C, #pragma optimize ("inline-limit=1") has a global effect > > > on all functions in the file, even those lexically before the > > > pragma. In C++, the pragma has no effect. Both of these > > > effects are contrary to my reading of the manual. > > > > > > Attribute optimize ("inline-limit") behaves similarly, which > > > seems even more unexpected and even more contrary to the manual > > > which by referring to Function Specific Option Pragmas suggests > > > that both the pragma and especially the attribute apply > > > optimizations to just the functions they're specified for. > > > > It can't really work so please do _not_ add the option to > > the set of Optimize options. Inlining is an IPA task so > > "per function" parameters do not make sense at all. > > It does work as I expect for other inlining options (e.g., for > -finline-functions or -finline-functions-called-once) so either > there's something special/different about -finline-limit or that > it works for the others is an accident. Can you please clarify > which it is? It works for the others because those flags can be properly tested per function. Since -finline-limit just maps to some --params and those are not per function it cannot work similarly for that. Adding 'Optimization' to -finline-limit will just cause that flag to be kept per function without any effect given nothing tests that flag but only the params. Consider #pragma GCC optimize("-finline-limit=10") void foo () { } #pragma GCC optimize("-finline-limit=100") where we'll likely just have the 100 limit at the end. I'd rather _not_ expose this kind of brokeness to the user. > Either way, wherever they come into play, the limitations you > refer to should be mentioned in the GCC warning when an option > that can't or may not work is used in attribute or #pragma > optimize, and it should also be explained in the manual. How > else are users supposed to know? They know because the option doesn't work there but only on the command line? > As it is, since -finline-limit is listed among optimization > options, and since attribute and #pragma optimize say they > accept optimization options, it's a bug that GCC rejects it > as if it were invalid. That's a documentation bug then. Specifially it says "This pragma allows you to set global optimization options for functions defined later in the source file." that's not very specific and I don't read into it that you can add all and only those options listed in the "Optimization Options" list. After all there's no cross-reference. Specifically it says "global" optimization options -- which is a subset of "optimization options". I'd have expected "local" optimization options here but that's besides the point. I'm fine with documenting for -finline-limit that it cannot be used in the pragma or attribute but I expect others to be in the same category. Richard.
PR middle-end/84603 - -finline-limit not accepted in attribute and #pragma optimize gcc/testsuite/ChangeLog: PR middle-end/84603 * gcc.dg/inline-40.c: New test. gcc/ChangeLog: PR middle-end/84603 * common.opt (-finline-limit): Add to Optimization options. Add deprecation warning. * doc/invoke.texi: Document deprecation. Index: gcc/common.opt =================================================================== --- gcc/common.opt (revision 258114) +++ gcc/common.opt (working copy) @@ -1621,7 +1621,7 @@ finline-limit- Common RejectNegative Joined Alias(finline-limit=) finline-limit= -Common RejectNegative Joined UInteger +Common RejectNegative Joined UInteger Optimization Warn(%<-finline-limit%> is deprecated; use %<--param%> instead) -finline-limit=<number> Limit the size of inlined functions to <number>. finline-atomics Index: gcc/doc/invoke.texi =================================================================== --- gcc/doc/invoke.texi (revision 258114) +++ gcc/doc/invoke.texi (working copy) @@ -7854,6 +7854,8 @@ in default behavior. abstract measurement of function's size. In no way does it represent a count of assembly instructions and as such its exact meaning might change from one release to an another. +@code{-finline-limit} is deprecated in favor of +the corresponding @option{--param @var{name}=@var{value}} options. @item -fno-keep-inline-dllexport @opindex fno-keep-inline-dllexport Index: gcc/testsuite/gcc.dg/inline-40.c =================================================================== --- gcc/testsuite/gcc.dg/inline-40.c (nonexistent) +++ gcc/testsuite/gcc.dg/inline-40.c (working copy) @@ -0,0 +1,19 @@ +/* PR middle-end/84603 - -finline-limit not accepted in attribute + and #pragma optimize + { dg-do compile } + { dg-options "-Wall" } */ + +/* Verify that the options are accepted in both pragmas and attributes. */ +#pragma GCC optimize ("inline-functions") +void f0 (void); + +#pragma GCC optimize ("inline-limit=32") /* { dg-bogus "bad option .-finline-limit=32." } */ +void f1 (void); + +void __attribute__ ((optimize ("inline-functions"))) +f2 (void); + +void __attribute__ ((optimize ("inline-limit=100"))) /* { dg-bogus "bad option .-finline-limit=100." } */ +f3 (void); + +
While testing my recent changes to the handling of attributes on C++ templates I noticed that the -finline-limit=N option is not recognized in attribute or pragma optimize. In response to the bug I raised, Richard you said the option is deprecated, so I went ahead and documented the deprecation in the manual and also added a deprecation warning. I also added it to Optimization options. The attached patch reflects these changes. I also tried to put together a test to verify that the option works as expected, both in attributes and in pragmas. I wasn't able to get it to work reliably or sensibly. In C, #pragma optimize ("inline-limit=1") has a global effect on all functions in the file, even those lexically before the pragma. In C++, the pragma has no effect. Both of these effects are contrary to my reading of the manual. Attribute optimize ("inline-limit") behaves similarly, which seems even more unexpected and even more contrary to the manual which by referring to Function Specific Option Pragmas suggests that both the pragma and especially the attribute apply optimizations to just the functions they're specified for. I don't expect this to be news to anyone here or easily fixable, but I think the effects or limitations of the mechanism should be made clear in the manual. Ditto for #pragma GDD diagnostic and its effect on middle-end warnings (or lack thereof). With the increasing number of such warnings that's increasingly becoming a source of frustration for users. If you agree I'll put together a documentation patch to try to clarify this. Martin