| Message ID | 52AB6322.4050108@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 12/13/13 12:42, Aldy Hernandez wrote: > I'm fixing something completely unrelated, and noticed this... > > Perhaps I'm missing something, but why do we need a gate when it always > returns true? Now that we have this `pass_data' business, we can set > has_gate to false. > > I also fixed a minor typo in a comment. Note that the way we handle pass properties differs when we have a gate vs no gate. See register_dump_files_1. I have no clue why we have that gem and it looks damn strange to me. jeff
On 12/16/13 08:13, Jeff Law wrote: > On 12/13/13 12:42, Aldy Hernandez wrote: >> I'm fixing something completely unrelated, and noticed this... >> >> Perhaps I'm missing something, but why do we need a gate when it always >> returns true? Now that we have this `pass_data' business, we can set >> has_gate to false. >> >> I also fixed a minor typo in a comment. > Note that the way we handle pass properties differs when we have a gate > vs no gate. See register_dump_files_1. > > I have no clue why we have that gem and it looks damn strange to me. > > jeff Ughhh, that's just many shades of wrong. Alright, nice catch. I'll move onto something else. Thanks.
> On 12/13/13 12:42, Aldy Hernandez wrote: > >I'm fixing something completely unrelated, and noticed this... > > > >Perhaps I'm missing something, but why do we need a gate when it always > >returns true? Now that we have this `pass_data' business, we can set > >has_gate to false. > > > >I also fixed a minor typo in a comment. > Note that the way we handle pass properties differs when we have a > gate vs no gate. See register_dump_files_1. Removing the gate is OK. It used to be enabled only with -finline-functions until we started to rely on inliner to handle other things - always_inline and CFG fixups. Honza
On 12/16/13 10:07, Jan Hubicka wrote: >> On 12/13/13 12:42, Aldy Hernandez wrote: >>> I'm fixing something completely unrelated, and noticed this... >>> >>> Perhaps I'm missing something, but why do we need a gate when it always >>> returns true? Now that we have this `pass_data' business, we can set >>> has_gate to false. >>> >>> I also fixed a minor typo in a comment. >> Note that the way we handle pass properties differs when we have a >> gate vs no gate. See register_dump_files_1. > > Removing the gate is OK. It used to be enabled only > with -finline-functions until we started to rely on inliner to handle > other things - always_inline and CFG fixups. > > Honza > Alright. Are you OK Jeff with me removing it then?
On 12/16/13 11:13, Aldy Hernandez wrote: > On 12/16/13 10:07, Jan Hubicka wrote: >>> On 12/13/13 12:42, Aldy Hernandez wrote: >>>> I'm fixing something completely unrelated, and noticed this... >>>> >>>> Perhaps I'm missing something, but why do we need a gate when it always >>>> returns true? Now that we have this `pass_data' business, we can set >>>> has_gate to false. >>>> >>>> I also fixed a minor typo in a comment. >>> Note that the way we handle pass properties differs when we have a >>> gate vs no gate. See register_dump_files_1. >> >> Removing the gate is OK. It used to be enabled only >> with -finline-functions until we started to rely on inliner to handle >> other things - always_inline and CFG fixups. >> >> Honza >> > > Alright. Are you OK Jeff with me removing it then? Yes -- Jan knows the IPA code better than I and thus is in a better position to know if the differences in how properties are handled for gate/no-gate are an issue or not for the IPA passes. Jeff
On Mon, Dec 16, 2013 at 09:13:41AM -0700, Jeff Law wrote: > On 12/13/13 12:42, Aldy Hernandez wrote: > >I'm fixing something completely unrelated, and noticed this... > > > >Perhaps I'm missing something, but why do we need a gate when it always > >returns true? Now that we have this `pass_data' business, we can set > >has_gate to false. > > > >I also fixed a minor typo in a comment. > Note that the way we handle pass properties differs when we have a > gate vs no gate. See register_dump_files_1. > > I have no clue why we have that gem and it looks damn strange to me. afaik that code is useless, I was going to remove it in http://gcc.gnu.org/ml/gcc-patches/2013-11/msg00854.html but I didn't finish fixing the issues with passes with an execute hook and child passes before the end of stage 1 so the unrelated bits didn't get committed yet. Trev > > jeff
diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 715b3a2..dd5d86e 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,9 @@ +2013-12-12 Aldy Hernandez <aldyh@redhat.com> + + * ipa-inline.c (gate_ipa_inline): Remove. + (const pass_data pass_data_ipa_inline): Unset has_gate. + (class pass_ipa_inline): Remove gate() method. + 2013-12-09 Richard Biener <rguenther@suse.de> PR middle-end/38474 diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c index 38157ca..9fd5d41 100644 --- a/gcc/ipa-inline.c +++ b/gcc/ipa-inline.c @@ -2339,19 +2339,6 @@ make_pass_early_inline (gcc::context *ctxt) return new pass_early_inline (ctxt); } - -/* When to run IPA inlining. Inlining of always-inline functions - happens during early inlining. - - Enable inlining unconditoinally, because callgraph redirection - happens here. */ - -static bool -gate_ipa_inline (void) -{ - return true; -} - namespace { const pass_data pass_data_ipa_inline = @@ -2359,7 +2346,7 @@ const pass_data pass_data_ipa_inline = IPA_PASS, /* type */ "inline", /* name */ OPTGROUP_INLINE, /* optinfo_flags */ - true, /* has_gate */ + false, /* has_gate */ true, /* has_execute */ TV_IPA_INLINING, /* tv_id */ 0, /* properties_required */ @@ -2386,7 +2373,6 @@ public: {} /* opt_pass methods: */ - bool gate () { return gate_ipa_inline (); } unsigned int execute () { return ipa_inline (); } }; // class pass_ipa_inline diff --git a/gcc/tree-inline.h b/gcc/tree-inline.h index d871fc4..741d6eb 100644 --- a/gcc/tree-inline.h +++ b/gcc/tree-inline.h @@ -162,8 +162,8 @@ typedef struct eni_weights_d /* Cost of return. */ unsigned return_cost; - /* True when time of statemnt should be estimated. Thus i.e - cost of switch statement is logarithmic rather than linear in number + /* True when time of statement should be estimated. Thus, the + cost of a switch statement is logarithmic rather than linear in number of cases. */ bool time_based; } eni_weights;
I'm fixing something completely unrelated, and noticed this... Perhaps I'm missing something, but why do we need a gate when it always returns true? Now that we have this `pass_data' business, we can set has_gate to false. I also fixed a minor typo in a comment. Tested on x86-64 Linux. OK? commit a89f826a3c832da2d572bee9608d407b338efe9f Author: Aldy Hernandez <aldyh@redhat.com> Date: Thu Dec 12 15:14:28 2013 -0800 * ipa-inline.c (gate_ipa_inline): Remove. (const pass_data pass_data_ipa_inline): Unset has_gate. (class pass_ipa_inline): Remove gate() method.