| Message ID | 201107241912.51760.ebotcazou@adacore.com |
|---|---|
| State | New |
| Headers | show |
On Sun, Jul 24, 2011 at 7:12 PM, Eric Botcazou <ebotcazou@adacore.com> wrote: > Hi, > > we sometimes get messages like this in Ada: > > prime-mc2-other.adb: In function 'PRIME.MC2.OTHER.DO_SOMETHING': > prime-mc2.adb:2:4: warning: inlining failed in call > to 'PRIME.MC2.GET_INPUT_VALUE.PART': non-call exception handling mismatch > [-Winline] > prime-mc2-other.adb:3:4: warning: called from here [-Winline] > > Since this is for a pure Ada program, it's unexpected. This stems from virtual > cloning: cgraph_create_virtual_clone creates the virtual clone and does: > > DECL_STRUCT_FUNCTION (new_decl) = NULL; > > so the can_throw_non_call_exceptions flag isn't preserved and can_inline_edge_p > is fooled into thinking that it cannot inline. > > It's probably better not to fiddle with virtual cloning so the attached patch > teaches can_inline_edge_p to look into DECL_STRUCT_FUNCTION of the original > nodes if it is dealing with virtual clones. > > Tested on i586-suse-linux, OK for the mainline? Doesn't cgraph_function_or_thunk_node already deal with this? Honza? Richard. > > 2011-07-24 Eric Botcazou <ebotcazou@adacore.com> > > * ipa-inline.c (can_inline_edge_p): Look into DECL_STRUCT_FUNCTION of > original nodes if we are dealing with virtual clones. > > > -- > Eric Botcazou >
> On Sun, Jul 24, 2011 at 7:12 PM, Eric Botcazou <ebotcazou@adacore.com> wrote: > > Hi, > > > > we sometimes get messages like this in Ada: > > > > prime-mc2-other.adb: In function 'PRIME.MC2.OTHER.DO_SOMETHING': > > prime-mc2.adb:2:4: warning: inlining failed in call > > to 'PRIME.MC2.GET_INPUT_VALUE.PART': non-call exception handling mismatch > > [-Winline] > > prime-mc2-other.adb:3:4: warning: called from here [-Winline] > > > > Since this is for a pure Ada program, it's unexpected. This stems from virtual > > cloning: cgraph_create_virtual_clone creates the virtual clone and does: > > > > DECL_STRUCT_FUNCTION (new_decl) = NULL; > > > > so the can_throw_non_call_exceptions flag isn't preserved and can_inline_edge_p > > is fooled into thinking that it cannot inline. > > > > It's probably better not to fiddle with virtual cloning so the attached patch > > teaches can_inline_edge_p to look into DECL_STRUCT_FUNCTION of the original > > nodes if it is dealing with virtual clones. > > > > Tested on i586-suse-linux, OK for the mainline? > > Doesn't cgraph_function_or_thunk_node already deal with this? Honza? No, the problem here is deciding whether we can inline a clone. We look into DECL_STRUCT_FUNCTION that we can't. The real fix is one commented in: /* Don't inline if the callee can throw non-call exceptions but the caller cannot. FIXME: this is obviously wrong for LTO where STRUCT_FUNCTION is missing. Move the flag into cgraph node or mirror it in the inline summary. */ I plan to look into this before next release. I would, for sure, welcome Eric beating me. If he don't have time to do so, I think the patch is OK as it is, since it improves the situation despite the fact that it won't fix the same problem with WPA. Honza > > Richard. > > > > > 2011-07-24 Eric Botcazou <ebotcazou@adacore.com> > > > > * ipa-inline.c (can_inline_edge_p): Look into DECL_STRUCT_FUNCTION of > > original nodes if we are dealing with virtual clones. > > > > > > -- > > Eric Botcazou > >
> No, the problem here is deciding whether we can inline a clone. > We look into DECL_STRUCT_FUNCTION that we can't. The real fix is one > commented in: > > /* Don't inline if the callee can throw non-call exceptions but the > caller cannot. > FIXME: this is obviously wrong for LTO where STRUCT_FUNCTION is > missing. Move the flag into cgraph node or mirror it in the inline summary. > */ The irony being that I implemented the flag for the sake of LTO, based on suggestions made on this list... So why is STRUCT_FUNCTION missing now? > I plan to look into this before next release. I would, for sure, welcome > Eric beating me. If he don't have time to do so, I think the patch is OK as > it is, since it improves the situation despite the fact that it won't fix > the same problem with WPA. OK, I'll install the patch for now.
> > No, the problem here is deciding whether we can inline a clone. > > We look into DECL_STRUCT_FUNCTION that we can't. The real fix is one > > commented in: > > > > /* Don't inline if the callee can throw non-call exceptions but the > > caller cannot. > > FIXME: this is obviously wrong for LTO where STRUCT_FUNCTION is > > missing. Move the flag into cgraph node or mirror it in the inline summary. > > */ > > The irony being that I implemented the flag for the sake of LTO, based on > suggestions made on this list... So why is STRUCT_FUNCTION missing now? I noticed that, sorry it was also my ommision. DECL_STRUCT_FUNCTION and function bodies are not load into WPA stage, only into ltrans. This is how WHOPR is designed. WPA stage is expected to work across cgraph/varpool and not look into the bodies/initializers. So this flag, as well as the optimization settings, needs to be copied there. Since we do a lot of querries from codegen to this particular flag, I would suggest to simply copy it into inline_summary and make inline_analyze_function to copy it rather than moving it to place less convenient for the codegen. Honza > > > I plan to look into this before next release. I would, for sure, welcome > > Eric beating me. If he don't have time to do so, I think the patch is OK as > > it is, since it improves the situation despite the fact that it won't fix > > the same problem with WPA. > > OK, I'll install the patch for now. > > -- > Eric Botcazou
Index: ipa-inline.c =================================================================== --- ipa-inline.c (revision 176622) +++ ipa-inline.c (working copy) @@ -238,9 +238,20 @@ can_inline_edge_p (struct cgraph_edge *e { bool inlinable = true; enum availability avail; - struct cgraph_node *callee = cgraph_function_or_thunk_node (e->callee, &avail); + struct cgraph_node *callee + = cgraph_function_or_thunk_node (e->callee, &avail); tree caller_tree = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (e->caller->decl); - tree callee_tree = callee ? DECL_FUNCTION_SPECIFIC_OPTIMIZATION (callee->decl) : NULL; + tree callee_tree + = callee ? DECL_FUNCTION_SPECIFIC_OPTIMIZATION (callee->decl) : NULL; + struct function *caller_cfun = DECL_STRUCT_FUNCTION (e->caller->decl); + struct function *callee_cfun + = callee ? DECL_STRUCT_FUNCTION (callee->decl) : NULL; + + if (!caller_cfun && e->caller->clone_of) + caller_cfun = DECL_STRUCT_FUNCTION (e->caller->clone_of->decl); + + if (!callee_cfun && callee && callee->clone_of) + callee_cfun = DECL_STRUCT_FUNCTION (callee->clone_of->decl); gcc_assert (e->inline_failed); @@ -277,12 +288,8 @@ can_inline_edge_p (struct cgraph_edge *e caller cannot. FIXME: this is obviously wrong for LTO where STRUCT_FUNCTION is missing. Move the flag into cgraph node or mirror it in the inline summary. */ - else if (DECL_STRUCT_FUNCTION (callee->decl) - && DECL_STRUCT_FUNCTION - (callee->decl)->can_throw_non_call_exceptions - && !(DECL_STRUCT_FUNCTION (e->caller->decl) - && DECL_STRUCT_FUNCTION - (e->caller->decl)->can_throw_non_call_exceptions)) + else if (callee_cfun && callee_cfun->can_throw_non_call_exceptions + && !(caller_cfun && caller_cfun->can_throw_non_call_exceptions)) { e->inline_failed = CIF_NON_CALL_EXCEPTIONS; inlinable = false;