| Message ID | 4DFF4BD2.7060806@st.com |
|---|---|
| State | New |
| Headers | show |
Christian Bruel <christian.bruel@st.com> writes: > 2011-06-16 Christian Bruel <christian.bruel@st.com> > > PR 49139/43654 Please use the correct PR number format here: PR middle-end/49139 PR middle-end/43654 Otherwise the check-in notices don't get into the PRs. Thanks. Rainer
On Mon, Jun 20, 2011 at 3:32 PM, Christian Bruel <christian.bruel@st.com> wrote: > In addition to the approved part of the patch, I finally got the testsuite > and a full Linux distribution to build without false warnings, with the > additional following changes: > > * two false warnings detected during a Linux distrib rebuild > > - __typeof (__error) doesn't propagate the inline keyword. even if the > function had attribute always_inline. > - extern inline function redefinition would inherit the attribute, even if > they are not inline > > DECL_UNINLINABLE is set for redefining extern inline functions. (commented > in c-decl.c:2340). So indeed testing this flags in the cgraphunit.c part > fixed both. > I checked that this doesn't conflict with other DECL_UNINLINABLE warning > emitted from ree_inlinable_function_p. (which is called after) > > * One silent bug revealed, undetected in the GCC testuite: > > - PR43564.c compiled in -O0, the "__clz" function was silently not inlined. > Fixed in ipa-inline.c:inline_forbidden_p_stmt - if ((caller_opt->x_optimize > callee_opt->x_optimize) - || (caller_opt->x_optimize_size != callee_opt->x_optimize_size)) + if (((caller_opt->x_optimize > callee_opt->x_optimize) + || (caller_opt->x_optimize_size != callee_opt->x_optimize_size)) + /* gcc.dg/pr43564.c. look at forced inline even in -O0. */ + && !lookup_attribute ("always_inline", DECL_ATTRIBUTES (e->callee->decl))) please check DECL_DISREGARD_INLINE_LIMITS instead. > * LTO pitfall: It's OK to not have a body to inline when generating the LTO > Gimple. Enforced by PR20090218-1_0.c I'm not 100% sure here ;) At least as we still output a regular object file without the function being inlined. But as we're going towars slim-LTO (hopefully) I guess it's ok. > * In addition to the parts of the patch that was approved, I had to fix the > tests that didn't use the inline keyword but enforced the always_inline > attribute to pass without the warning > I choose to add -Wno-attributes to the option list. Could as well fix the > test by adding the inline keyword, but I preferred to not modify the > original test when possible. > > * A minor code optimization in ipa-inline-transform.c: Don't call > optimize_inline_calls if there is not callee. > > Comments, OK to apply ? Ok with the above change and the PRs mentioned in the changelog properly (see Rainers comment). Thanks, Richard. > Many thanks > > Christian > > 2011-06-16 Christian Bruel <christian.bruel@st.com> > > PR 49139/43654 > * cgraphunit.c (process_function_and_variable_attributes): warn > for always_inline functions that are not inline. > * ipa-inline-transform.c (inline_transform): Always call > optimize_inline. > * ipa-inline.c (can_inline_edge_p): Check always_inline. > * tree-inline.c (tree_inlinable_function_p): Call error instead > of sorry. > (expand_call_inline): Likewise. > > 2011-06-16 Christian Bruel <christian.bruel@st.com> > > * gcc.dg/always_inline.c: Removed -Winline. Update checks > * gcc.dg/always_inline2.c: Likewise. > * gcc.dg/always_inline3.c: Likewise. > * gcc.dg/debug/pr41264-1.c: Add -Wno-attributes. > * gcc.dg/inline_1.c: Likewise. > * gcc.dg/inline_2.c: Likewise. > * gcc.dg/inline_3.c: Likewise. > * gcc.dg/inline_4.c: Likewise. > * gcc.dg/20051201-1.c: Likewise. > * gcc.dg/torture/pta-structcopy-1.c: Likewise. > * gcc.dg/inline-22.c: Likewise. > * gcc.dg/lto/20090218-1_0.c: Set inline keyword. > * gcc.dg/lto/20090218-1_1.c: Likewise. > * g++.dg/ipa/devirt-7.C: Likewise. > * gcc.dg/uninit-pred-5_a.c: Likewise. > * gcc.dg/uninit-pred-5_b.c: Likewise. > * gcc.dg/fail_always_inline.c: New. >
On Jun 20, 2011, at 6:41 AM, Rainer Orth wrote: > Christian Bruel <christian.bruel@st.com> writes: > >> 2011-06-16 Christian Bruel <christian.bruel@st.com> >> >> PR 49139/43654 > > Please use the correct PR number format here: > > PR middle-end/49139 > PR middle-end/43654 > > Otherwise the check-in notices don't get into the PRs. Also, I think there is a preference to pick the best bug report to make this a fix for, and list just one number, and then close the second as a dup of the first, after the first is fixed.
On 06/20/2011 03:41 PM, Rainer Orth wrote: > Christian Bruel<christian.bruel@st.com> writes: > >> 2011-06-16 Christian Bruel<christian.bruel@st.com> >> >> PR 49139/43654 > > Please use the correct PR number format here: > > PR middle-end/49139 > PR middle-end/43654 > > Otherwise the check-in notices don't get into the PRs. OK thanks, in fact I was referring to the file gcc.dg/pr43564.c (there was a typo in the number btw). But good indeed to send the notice into the original PR as well. Cheers Christian > > Thanks. > Rainer >
On Mon, Jun 20, 2011 at 3:56 PM, Christian Bruel <christian.bruel@st.com> wrote: > On 06/20/2011 03:41 PM, Rainer Orth wrote: >> >> Christian Bruel<christian.bruel@st.com> writes: >> >>> 2011-06-16 Christian Bruel<christian.bruel@st.com> >>> >>> PR 49139/43654 >> >> Please use the correct PR number format here: >> >> PR middle-end/49139 >> PR middle-end/43654 >> >> Otherwise the check-in notices don't get into the PRs. > > OK thanks, in fact I was referring to the file gcc.dg/pr43564.c (there was a > typo in the number btw). But good indeed to send the notice into the > original PR as well. The code now looks like if (node->callees) { /* Redirecting edges might lead to a need for vops to be recomputed. */ todo |= TODO_update_ssa_only_virtuals; todo = optimize_inline_calls (current_function_decl); } I have committed an obvious patch. Richard. > Cheers > > Christian > >> >> Thanks. >> Rainer >> >
Index: ipa-inline-transform.c =================================================================== --- ipa-inline-transform.c (revision 175201) +++ ipa-inline-transform.c (working copy) @@ -348,8 +348,7 @@ { unsigned int todo = 0; struct cgraph_edge *e; - bool inline_p = false; - + /* FIXME: Currently the pass manager is adding inline transform more than once to some clones. This needs revisiting after WPA cleanups. */ if (cfun->after_inlining) @@ -361,20 +360,17 @@ save_inline_function_body (node); for (e = node->callees; e; e = e->next_callee) + cgraph_redirect_edge_call_stmt_to_callee (e); + + timevar_push (TV_INTEGRATION); + if (node->callees) { - cgraph_redirect_edge_call_stmt_to_callee (e); - if (!e->inline_failed || warn_inline) - inline_p = true; /* Redirecting edges might lead to a need for vops to be recomputed. */ todo |= TODO_update_ssa_only_virtuals; - } - - if (inline_p) - { - timevar_push (TV_INTEGRATION); todo = optimize_inline_calls (current_function_decl); - timevar_pop (TV_INTEGRATION); } + timevar_pop (TV_INTEGRATION); + cfun->always_inline_functions_inlined = true; cfun->after_inlining = true; return todo | execute_fixup_cfg (); Index: cgraphunit.c =================================================================== --- cgraphunit.c (revision 175201) +++ cgraphunit.c (working copy) @@ -986,6 +986,14 @@ DECL_ATTRIBUTES (decl) = remove_attribute ("weakref", DECL_ATTRIBUTES (decl)); } + + if (lookup_attribute ("always_inline", DECL_ATTRIBUTES (decl)) + && !DECL_DECLARED_INLINE_P (decl) + /* redefining always_inline function makes it DECL_UNINLINABLE. */ + && !DECL_UNINLINABLE (decl)) + warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes, + "always_inline function might not be inlinable"); + process_common_attributes (decl); } for (vnode = varpool_nodes; vnode != first_var; vnode = vnode->next) Index: ipa-inline.c =================================================================== --- ipa-inline.c (revision 175201) +++ ipa-inline.c (working copy) @@ -318,8 +318,10 @@ ? callee_tree : optimization_default_node); - if ((caller_opt->x_optimize > callee_opt->x_optimize) - || (caller_opt->x_optimize_size != callee_opt->x_optimize_size)) + if (((caller_opt->x_optimize > callee_opt->x_optimize) + || (caller_opt->x_optimize_size != callee_opt->x_optimize_size)) + /* gcc.dg/pr43564.c. look at forced inline even in -O0. */ + && !lookup_attribute ("always_inline", DECL_ATTRIBUTES (e->callee->decl))) { e->inline_failed = CIF_TARGET_OPTIMIZATION_MISMATCH; inlinable = false; Index: tree-inline.c =================================================================== --- tree-inline.c (revision 175201) +++ tree-inline.c (working copy) @@ -3192,7 +3192,7 @@ As a bonus we can now give more details about the reason why a function is not inlinable. */ if (always_inline) - sorry (inline_forbidden_reason, fn); + error (inline_forbidden_reason, fn); else if (do_warning) warning (OPT_Winline, inline_forbidden_reason, fn); @@ -3742,11 +3742,13 @@ if (lookup_attribute ("always_inline", DECL_ATTRIBUTES (fn)) /* Avoid warnings during early inline pass. */ - && cgraph_global_info_ready) + && cgraph_global_info_ready + /* PR 20090218-1_0.c. Body can be provided by another module. */ + && (reason != CIF_BODY_NOT_AVAILABLE || !flag_generate_lto)) { - sorry ("inlining failed in call to %q+F: %s", fn, - _(cgraph_inline_failed_string (reason))); - sorry ("called from here"); + error ("inlining failed in call to always_inline %q+F: %s", fn, + cgraph_inline_failed_string (reason)); + error ("called from here"); } else if (warn_inline && DECL_DECLARED_INLINE_P (fn)
In addition to the approved part of the patch, I finally got the testsuite and a full Linux distribution to build without false warnings, with the additional following changes: * two false warnings detected during a Linux distrib rebuild - __typeof (__error) doesn't propagate the inline keyword. even if the function had attribute always_inline. - extern inline function redefinition would inherit the attribute, even if they are not inline DECL_UNINLINABLE is set for redefining extern inline functions. (commented in c-decl.c:2340). So indeed testing this flags in the cgraphunit.c part fixed both. I checked that this doesn't conflict with other DECL_UNINLINABLE warning emitted from ree_inlinable_function_p. (which is called after) * One silent bug revealed, undetected in the GCC testuite: - PR43564.c compiled in -O0, the "__clz" function was silently not inlined. Fixed in ipa-inline.c:inline_forbidden_p_stmt * LTO pitfall: It's OK to not have a body to inline when generating the LTO Gimple. Enforced by PR20090218-1_0.c * In addition to the parts of the patch that was approved, I had to fix the tests that didn't use the inline keyword but enforced the always_inline attribute to pass without the warning I choose to add -Wno-attributes to the option list. Could as well fix the test by adding the inline keyword, but I preferred to not modify the original test when possible. * A minor code optimization in ipa-inline-transform.c: Don't call optimize_inline_calls if there is not callee. Comments, OK to apply ? Many thanks Christian 2011-06-16 Christian Bruel <christian.bruel@st.com> PR 49139/43654 * cgraphunit.c (process_function_and_variable_attributes): warn for always_inline functions that are not inline. * ipa-inline-transform.c (inline_transform): Always call optimize_inline. * ipa-inline.c (can_inline_edge_p): Check always_inline. * tree-inline.c (tree_inlinable_function_p): Call error instead of sorry. (expand_call_inline): Likewise. 2011-06-16 Christian Bruel <christian.bruel@st.com> * gcc.dg/always_inline.c: Removed -Winline. Update checks * gcc.dg/always_inline2.c: Likewise. * gcc.dg/always_inline3.c: Likewise. * gcc.dg/debug/pr41264-1.c: Add -Wno-attributes. * gcc.dg/inline_1.c: Likewise. * gcc.dg/inline_2.c: Likewise. * gcc.dg/inline_3.c: Likewise. * gcc.dg/inline_4.c: Likewise. * gcc.dg/20051201-1.c: Likewise. * gcc.dg/torture/pta-structcopy-1.c: Likewise. * gcc.dg/inline-22.c: Likewise. * gcc.dg/lto/20090218-1_0.c: Set inline keyword. * gcc.dg/lto/20090218-1_1.c: Likewise. * g++.dg/ipa/devirt-7.C: Likewise. * gcc.dg/uninit-pred-5_a.c: Likewise. * gcc.dg/uninit-pred-5_b.c: Likewise. * gcc.dg/fail_always_inline.c: New. 2011-06-16 Christian Bruel <christian.bruel@st.com> PR 49139/43654 * cgraphunit.c (process_function_and_variable_attributes): warn when always_inline functions that are not inline. * ipa-inline-transform.c (inline_transform): Always call optimize_inline. * ipa-inline.c (can_inline_edge_p): Check always_inline. * tree-inline.c (tree_inlinable_function_p): Use error instead of sorry. (expand_call_inline): Likewise. Index: testsuite/ChangeLog 2011-06-16 Christian Bruel <christian.bruel@st.com> * gcc.dg/always_inline.c: Removed -Winline. Update checks * gcc.dg/always_inline2.c: Likewise. * gcc.dg/always_inline3.c: Likewise. * gcc.dg/debug/pr41264-1.c: Add -Wno-attributes. * gcc.dg/inline_1.c: Likewise. * gcc.dg/inline_2.c: Likewise. * gcc.dg/inline_3.c: Likewise. * gcc.dg/inline_4.c: Likewise. * gcc.dg/20051201-1.c: Likewise. * gcc.dg/torture/pta-structcopy-1.c: Likewise. * gcc.dg/inline-22.c: Likewise. * gcc.dg/lto/20090218-1_0.c: Set inline keyword. * gcc.dg/lto/20090218-1_1.c: Likewise. * g++.dg/ipa/devirt-7.C: Likewise. * gcc.dg/uninit-pred-5_a.c: Likewise. * gcc.dg/uninit-pred-5_b.c: Likewise. * gcc.dg/fail_always_inline.c: New. Index: testsuite/gcc.dg/always_inline.c =================================================================== --- testsuite/gcc.dg/always_inline.c (revision 175201) +++ testsuite/gcc.dg/always_inline.c (working copy) @@ -1,8 +1,8 @@ /* { dg-do compile } */ -/* { dg-options "-Winline -O2" } */ +/* { dg-options "-O2" } */ #include <stdarg.h> inline __attribute__ ((always_inline)) void -e(int t, ...) /* { dg-message "sorry\[^\n\]*variable argument" "" } */ +e(int t, ...) /* { dg-error "variable argument lists" } */ { va_list q; va_start (q, t); Index: testsuite/gcc.dg/inline_2.c =================================================================== --- testsuite/gcc.dg/inline_2.c (revision 175201) +++ testsuite/gcc.dg/inline_2.c (working copy) @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -fdump-tree-optimized -fdisable-tree-einline=0:100 -fdisable-ipa-inline" } */ +/* { dg-options "-O2 -fdump-tree-optimized -fdisable-tree-einline=0:100 -fdisable-ipa-inline -Wno-attributes" } */ int g; __attribute__((always_inline)) void bar (void) { Index: testsuite/gcc.dg/20051201-1.c =================================================================== --- testsuite/gcc.dg/20051201-1.c (revision 175201) +++ testsuite/gcc.dg/20051201-1.c (working copy) @@ -2,7 +2,7 @@ tree_flow_call_edges_add. */ /* { dg-do compile } */ -/* { dg-options "-O1 -fprofile-generate" } */ +/* { dg-options "-O1 -fprofile-generate -Wno-attributes" } */ static __attribute__ ((always_inline)) void baz () Index: testsuite/gcc.dg/debug/pr41264-1.c =================================================================== --- testsuite/gcc.dg/debug/pr41264-1.c (revision 175201) +++ testsuite/gcc.dg/debug/pr41264-1.c (working copy) @@ -1,4 +1,5 @@ /* { dg-do compile } */ +/* { dg-options "-Wno-attributes" } */ #if (__SIZEOF_INT__ <= 2) typedef unsigned long hashval_t; Index: testsuite/gcc.dg/uninit-pred-5_a.c =================================================================== --- testsuite/gcc.dg/uninit-pred-5_a.c (revision 175201) +++ testsuite/gcc.dg/uninit-pred-5_a.c (working copy) @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wuninitialized -Wno-attributes -O2" } */ int g; int bar(); Index: testsuite/gcc.dg/inline_3.c =================================================================== --- testsuite/gcc.dg/inline_3.c (revision 175201) +++ testsuite/gcc.dg/inline_3.c (working copy) @@ -1,5 +1,5 @@ /* { dg-do compile { target i?86-*-linux* x86_64-*-linux* } } */ -/* { dg-options "-O2 -fdump-tree-optimized -fdisable-tree-einline=foo,foo2 -fdisable-ipa-inline" } */ +/* { dg-options "-O2 -fdump-tree-optimized -fdisable-tree-einline=foo,foo2 -fdisable-ipa-inline -Wno-attributes" } */ int g; __attribute__((always_inline)) void bar (void) { Index: testsuite/gcc.dg/inline-22.c =================================================================== --- testsuite/gcc.dg/inline-22.c (revision 175201) +++ testsuite/gcc.dg/inline-22.c (working copy) @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-funit-at-a-time" } */ +/* { dg-options "-funit-at-a-time -Wno-attributes" } */ /* Verify we can inline without a complete prototype and with promoted arguments. See also PR32492. */ __attribute__((always_inline)) void f1() {} Index: testsuite/gcc.dg/always_inline2.c =================================================================== --- testsuite/gcc.dg/always_inline2.c (revision 175201) +++ testsuite/gcc.dg/always_inline2.c (working copy) @@ -1,8 +1,8 @@ /* { dg-do compile } */ -/* { dg-options "-Winline -O2" } */ -inline __attribute__ ((always_inline)) void t(void); /* { dg-message "sorry\[^\n\]*body not available" "" } */ +/* { dg-options "-O2" } */ +inline __attribute__ ((always_inline)) void t(void); /* { dg-error "body not available" } */ void q(void) { - t(); /* { dg-message "sorry\[^\n\]*called from here" "" } */ + t(); /* { dg-error "called from here" } */ } Index: testsuite/gcc.dg/uninit-pred-5_b.c =================================================================== --- testsuite/gcc.dg/uninit-pred-5_b.c (revision 175201) +++ testsuite/gcc.dg/uninit-pred-5_b.c (working copy) @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-Wuninitialized -O2" } */ +/* { dg-options "-Wuninitialized -Wno-attributes -O2" } */ int g; int bar(); Index: testsuite/gcc.dg/inline_4.c =================================================================== --- testsuite/gcc.dg/inline_4.c (revision 175201) +++ testsuite/gcc.dg/inline_4.c (working copy) @@ -1,5 +1,5 @@ /* { dg-do compile { target i?86-*-linux* x86_64-*-linux* } } */ -/* { dg-options "-O2 -fdump-tree-optimized -fdisable-tree-einline=foo2 -fdisable-ipa-inline" } */ +/* { dg-options "-O2 -fdump-tree-optimized -fdisable-tree-einline=foo2 -fdisable-ipa-inline -Wno-attributes" } */ int g; __attribute__((always_inline)) void bar (void) { Index: testsuite/gcc.dg/lto/20090218-1_0.c =================================================================== --- testsuite/gcc.dg/lto/20090218-1_0.c (revision 175201) +++ testsuite/gcc.dg/lto/20090218-1_0.c (working copy) @@ -1,4 +1,4 @@ -void set_mem_alias_set () __attribute__ ((always_inline)); +void inline set_mem_alias_set () __attribute__ ((always_inline)); void emit_push_insn () { set_mem_alias_set (); } Index: testsuite/gcc.dg/lto/20090218-1_1.c =================================================================== --- testsuite/gcc.dg/lto/20090218-1_1.c (revision 175201) +++ testsuite/gcc.dg/lto/20090218-1_1.c (working copy) @@ -4,6 +4,6 @@ } static void __attribute__ ((noinline)) get_mem_attrs () { } -void __attribute__ ((always_inline)) set_mem_alias_set () { +void inline __attribute__ ((always_inline)) set_mem_alias_set () { get_mem_attrs (); } Index: testsuite/gcc.dg/torture/pta-structcopy-1.c =================================================================== --- testsuite/gcc.dg/torture/pta-structcopy-1.c (revision 175201) +++ testsuite/gcc.dg/torture/pta-structcopy-1.c (working copy) @@ -1,5 +1,5 @@ /* { dg-do run } */ -/* { dg-options "-fdump-tree-ealias" } */ +/* { dg-options "-fdump-tree-ealias -Wno-attributes" } */ /* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */ struct X Index: testsuite/gcc.dg/tree-ssa/pr40087.c =================================================================== --- testsuite/gcc.dg/tree-ssa/pr40087.c (revision 175201) +++ testsuite/gcc.dg/tree-ssa/pr40087.c (working copy) @@ -3,7 +3,7 @@ extern void abort (void); -static void __attribute__((always_inline)) +static inline void __attribute__((always_inline)) reverse(int *first, int *last) { if (first == last--) Index: testsuite/gcc.dg/always_inline3.c =================================================================== --- testsuite/gcc.dg/always_inline3.c (revision 175201) +++ testsuite/gcc.dg/always_inline3.c (working copy) @@ -1,11 +1,11 @@ /* { dg-do compile } */ -/* { dg-options "-Winline -O2" } */ +/* { dg-options "-O2" } */ int do_something_evil (void); inline __attribute__ ((always_inline)) void -q2(void) /* { dg-message "sorry\[^\n\]*recursive" "" } */ +q2(void) /* { dg-error "recursive inlining" } */ { if (do_something_evil ()) return; - q2(); /* { dg-message "sorry\[^\n\]*called from here" "" } */ + q2(); /* { dg-error "called from here" } */ q2(); /* With -O2 we don't warn here, it is eliminated by tail recursion. */ } Index: testsuite/gcc.dg/inline_1.c =================================================================== --- testsuite/gcc.dg/inline_1.c (revision 175201) +++ testsuite/gcc.dg/inline_1.c (working copy) @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -fdump-tree-optimized -fdisable-tree-einline -fdisable-ipa-inline" } */ +/* { dg-options "-O2 -fdump-tree-optimized -fdisable-tree-einline -fdisable-ipa-inline -Wno-attributes" } */ int g; __attribute__((always_inline)) void bar (void) { Index: testsuite/g++.dg/ipa/devirt-7.C =================================================================== --- testsuite/g++.dg/ipa/devirt-7.C (revision 175201) +++ testsuite/g++.dg/ipa/devirt-7.C (working copy) @@ -56,7 +56,7 @@ return 1; } -int __attribute__ ((always_inline)) +int inline __attribute__ ((always_inline)) A::middleman_1 (int i) { return this->foo (i);