remove gate for ipa_inline pass
diff mbox

Message ID 52AB6322.4050108@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez Dec. 13, 2013, 7:42 p.m. UTC
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.

Comments

Jeff Law Dec. 16, 2013, 4:13 p.m. UTC | #1
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
Aldy Hernandez Dec. 16, 2013, 5:47 p.m. UTC | #2
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.
Jan Hubicka Dec. 16, 2013, 6:07 p.m. UTC | #3
> 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
Aldy Hernandez Dec. 16, 2013, 6:13 p.m. UTC | #4
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?
Jeff Law Dec. 16, 2013, 6:19 p.m. UTC | #5
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
Trevor Saunders Dec. 16, 2013, 6:36 p.m. UTC | #6
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

Patch
diff mbox

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;