diff mbox

if-to-switch pass

Message ID 51D16FD0.3010800@mentor.com
State New
Headers show

Commit Message

Tom de Vries July 1, 2013, 12:02 p.m. UTC 
On 24/06/13 14:28, Richard Biener wrote:
> On Fri, Jun 21, 2013 at 10:37 PM, Cesar Philippidis
> <cesar_philippidis@mentor.com> wrote:
>> Here is an updated version of Tom's if-to-switch conversion pass that
>> was originally posted here:
>>
>> <http://gcc.gnu.org/ml/gcc-patches/2013-01/msg01210.html>.
>>
>> I corrected a build problem with ARM by including "tm_p.h" and an ICE
>> caused by NULL_TREE argument being passed to fold_binary () inside
>> refine_range_plus (). Also, TODO_ggc_collect has been removed in the
>> gimple_opt_pass struct.
>>
>> I bootstrapped and regression tested on x86_64-unknown-linux-gnu and
>> arm-none-linux-gnueabi. OK for trunk?
> 
> As we are doing switch-to-if conversion in the pass after this it looks like
> as if we can save us intermediate code generation if the two passes were
> unified?  That is, have a analysis phase that populates an internal
> representation
> of the switch / cascaded if and peforms transforms based on a single analysis
> of that representation.
> 

Richard,

I think that's doable, but it would be nice if we can get it committed in the
current form already.

> I also dislike the two passes being in early optimizations - that way they do
> not see the effects of IPA inlining / LTO IPA-CP transforms.
> I'd rather move
> it way down towards RTL expansion (though eventually some may say that
> switch-conversion may enable vectorization opportunities for example).
> 

Right, you discussed that with Steven here earlier:
http://gcc.gnu.org/ml/gcc-patches/2012-07/msg01156.html.

Steven proposed to move pass_switch_conversion to before split_functions.
Attached patch attempts to do that, I'm testing that now.

That doesn't address your concern about seeing effects of IPA inlining / LTO
IPA-CP transforms though.

Is this patch ok (once tested), or do you really want pass_switch_conversion
later, say the first in pass_all_optimizations, or f.i. after pass_pre to
pick-up on tail-merge creating opportunities for if-to-switch conversion once we
check that in before or merge with pass_switch_conversion?

Thanks,
- Tom

2013-07-01  Tom de Vries  <tom@codesourcery.com>

	passes.c (init_optimization_passes): Move pass_convert_switch to before
	pass_{feedback_,}split_functions.
	tree-pass.h (pass_feedback_convert_switch): Declare new pass.
	tree-switch-conversion.c (switchconv_gate): Don't run when profiling.
	(feedback_switchconv_gate): New function.
	(pass_feedback_convert_switch): New pass.

Comments

Tom de Vries July 1, 2013, 10:14 p.m. UTC | #1 
On 01/07/13 14:02, Tom de Vries wrote:
>> > I also dislike the two passes being in early optimizations - that way they do
>> > not see the effects of IPA inlining / LTO IPA-CP transforms.
>> > I'd rather move
>> > it way down towards RTL expansion (though eventually some may say that
>> > switch-conversion may enable vectorization opportunities for example).
>> > 
> Right, you discussed that with Steven here earlier:
> http://gcc.gnu.org/ml/gcc-patches/2012-07/msg01156.html.
> 
> Steven proposed to move pass_switch_conversion to before split_functions.
> Attached patch attempts to do that, I'm testing that now.
> 
> That doesn't address your concern about seeing effects of IPA inlining / LTO
> IPA-CP transforms though.
> 
> Is this patch ok (once tested),

Bootstrapped and reg-tested on x86_64 (ada inclusive), no issues found.

OK for trunk?

Thanks,
- Tom

> or do you really want pass_switch_conversion
> later, say the first in pass_all_optimizations, or f.i. after pass_pre to
> pick-up on tail-merge creating opportunities for if-to-switch conversion once we
> check that in before or merge with pass_switch_conversion?
> 
> Thanks,
> - Tom
> 
> 2013-07-01  Tom de Vries  <tom@codesourcery.com>
> 
> 	passes.c (init_optimization_passes): Move pass_convert_switch to before
> 	pass_{feedback_,}split_functions.
> 	tree-pass.h (pass_feedback_convert_switch): Declare new pass.
> 	tree-switch-conversion.c (switchconv_gate): Don't run when profiling.
> 	(feedback_switchconv_gate): New function.
> 	(pass_feedback_convert_switch): New pass.
diff mbox

Patch

diff --git a/gcc/passes.c b/gcc/passes.c
index 761f030..2d47239 100644
--- a/gcc/passes.c
+++ b/gcc/passes.c
@@ -1338,13 +1338,13 @@  init_optimization_passes (void)
 	  NEXT_PASS (pass_cd_dce);
 	  NEXT_PASS (pass_early_ipa_sra);
 	  NEXT_PASS (pass_tail_recursion);
-	  NEXT_PASS (pass_convert_switch);
           NEXT_PASS (pass_cleanup_eh);
           NEXT_PASS (pass_profile);
           NEXT_PASS (pass_local_pure_const);
 	  /* Split functions creates parts that are not run through
 	     early optimizations again.  It is thus good idea to do this
 	     late.  */
+          NEXT_PASS (pass_convert_switch);
           NEXT_PASS (pass_split_functions);
 	}
       NEXT_PASS (pass_release_ssa_names);
@@ -1355,6 +1355,7 @@  init_optimization_passes (void)
   NEXT_PASS (pass_ipa_tree_profile);
     {
       struct opt_pass **p = &pass_ipa_tree_profile.pass.sub;
+      NEXT_PASS (pass_feedback_convert_switch);
       NEXT_PASS (pass_feedback_split_functions);
     }
   NEXT_PASS (pass_ipa_increase_alignment);
diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h
index b8c59a7..8c2e2df 100644
--- a/gcc/tree-pass.h
+++ b/gcc/tree-pass.h
@@ -489,6 +489,7 @@  extern struct gimple_opt_pass pass_early_inline;
 extern struct gimple_opt_pass pass_inline_parameters;
 extern struct gimple_opt_pass pass_update_address_taken;
 extern struct gimple_opt_pass pass_convert_switch;
+extern struct gimple_opt_pass pass_feedback_convert_switch;
 
 /* The root of the compilation pass tree, once constructed.  */
 extern struct opt_pass *all_passes, *all_small_ipa_passes, *all_lowering_passes,
diff --git a/gcc/tree-switch-conversion.c b/gcc/tree-switch-conversion.c
index 9ad7daf..1793720 100644
--- a/gcc/tree-switch-conversion.c
+++ b/gcc/tree-switch-conversion.c
@@ -1460,7 +1460,9 @@  do_switchconv (void)
 static bool
 switchconv_gate (void)
 {
-  return flag_tree_switch_conversion != 0;
+  return (flag_tree_switch_conversion != 0
+	  && !profile_arc_flag
+	  && !flag_branch_probabilities);
 }
 
 struct gimple_opt_pass pass_convert_switch =
@@ -1485,3 +1487,35 @@  struct gimple_opt_pass pass_convert_switch =
   | TODO_verify_flow			/* todo_flags_finish */
  }
 };
+
+/* The pass gate. */
+
+static bool
+feedback_switchconv_gate (void)
+{
+  return (flag_tree_switch_conversion != 0
+	  && flag_branch_probabilities);
+}
+
+struct gimple_opt_pass pass_feedback_convert_switch =
+{
+ {
+  GIMPLE_PASS,
+  "switchconv",				/* name */
+  OPTGROUP_NONE,                        /* optinfo_flags */
+  feedback_switchconv_gate,		/* gate */
+  do_switchconv,			/* execute */
+  NULL,					/* sub */
+  NULL,					/* next */
+  0,					/* static_pass_number */
+  TV_TREE_SWITCH_CONVERSION,		/* tv_id */
+  PROP_cfg | PROP_ssa,	                /* properties_required */
+  0,					/* properties_provided */
+  0,					/* properties_destroyed */
+  0,					/* todo_flags_start */
+  TODO_update_ssa 
+  | TODO_verify_ssa
+  | TODO_verify_stmts
+  | TODO_verify_flow			/* todo_flags_finish */
+ }
+};