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)