diff mbox

Fix switch expansion (PR middle-end/81698)

Message ID 20170804154221.GG2123@tucnak
State New
Headers show

Commit Message

Jakub Jelinek Aug. 4, 2017, 3:42 p.m. UTC
Hi!

As mentioned inthe PR, the switch expansion relies on EDGE_SUCC (bb, 0)
from a GIMPLE_SWITCH to be the default edge, but GIMPLE only ensures that
default label is the first one.

So, this patch (in expand_case) instead finds the default_edge from the
default label and corresponding block.
Another issue is emit_case_dispatch_table - this function is called from
expand_case as well as sjlj landing pad handling.  For the former, it
suffers from a similar problem, and additionally we could have removed
the default edge already, so even for switches that formerly had a default
label we suddenly consider a random different edge as the default.
For sjlj, I've just looked at one testcase in cross to hppa2.0w-hp-hpux10.0,
and I believe stmt_bb should be just NULL, in any case if it would be
non-NULL and there is some edge, it would hardly be the default edge,
since the caller is emitting the default label after the switch expansion.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-08-04  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/81698
	* stmt.c (emit_case_dispatch_table): Add DEFAULT_EDGE argument,
	instead of computing it in the function.  Formatting fix.
	(expand_case): Don't rely on default_edge being the first edge,
	clear it if removing it, pass default_edge to
	emit_case_dispatch_table.
	(expand_sjlj_dispatch_table): Pass NULL as DEFAULT_EDGE, formatting
	fix.


	Jakub

Comments

Richard Biener Aug. 4, 2017, 6:22 p.m. UTC | #1
On August 4, 2017 5:42:21 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>As mentioned inthe PR, the switch expansion relies on EDGE_SUCC (bb, 0)
>from a GIMPLE_SWITCH to be the default edge, but GIMPLE only ensures
>that
>default label is the first one.
>
>So, this patch (in expand_case) instead finds the default_edge from the
>default label and corresponding block.
>Another issue is emit_case_dispatch_table - this function is called
>from
>expand_case as well as sjlj landing pad handling.  For the former, it
>suffers from a similar problem, and additionally we could have removed
>the default edge already, so even for switches that formerly had a
>default
>label we suddenly consider a random different edge as the default.
>For sjlj, I've just looked at one testcase in cross to
>hppa2.0w-hp-hpux10.0,
>and I believe stmt_bb should be just NULL, in any case if it would be
>non-NULL and there is some edge, it would hardly be the default edge,
>since the caller is emitting the default label after the switch
>expansion.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

>2017-08-04  Jakub Jelinek  <jakub@redhat.com>
>
>	PR middle-end/81698
>	* stmt.c (emit_case_dispatch_table): Add DEFAULT_EDGE argument,
>	instead of computing it in the function.  Formatting fix.
>	(expand_case): Don't rely on default_edge being the first edge,
>	clear it if removing it, pass default_edge to
>	emit_case_dispatch_table.
>	(expand_sjlj_dispatch_table): Pass NULL as DEFAULT_EDGE, formatting
>	fix.
>
>--- gcc/stmt.c.jj	2017-07-14 13:04:47.000000000 +0200
>+++ gcc/stmt.c	2017-08-04 14:36:28.936447265 +0200
>@@ -945,8 +945,8 @@ conditional_probability (profile_probabi
> static void
> emit_case_dispatch_table (tree index_expr, tree index_type,
> 			  struct case_node *case_list, rtx default_label,
>-			  tree minval, tree maxval, tree range,
>-                          basic_block stmt_bb)
>+			  edge default_edge,  tree minval, tree maxval,
>+			  tree range, basic_block stmt_bb)
> {
>   int i, ncases;
>   struct case_node *n;
>@@ -954,7 +954,6 @@ emit_case_dispatch_table (tree index_exp
>   rtx_insn *fallback_label = label_rtx (case_list->code_label);
>   rtx_code_label *table_label = gen_label_rtx ();
>   bool has_gaps = false;
>-  edge default_edge = stmt_bb ? EDGE_SUCC (stmt_bb, 0) : NULL;
>profile_probability default_prob = default_edge ?
>default_edge->probability
> 						  : profile_probability::never ();
>   profile_probability base = get_outgoing_edge_probs (stmt_bb);
>@@ -1026,9 +1025,8 @@ emit_case_dispatch_table (tree index_exp
>          through the indirect jump or the direct conditional jump
>          before that. Split the probability of reaching the
>          default label among these two jumps.  */
>-      new_default_prob = conditional_probability
>(default_prob.apply_scale
>-							 (1, 2),
>-                                                  base);
>+      new_default_prob
>+	= conditional_probability (default_prob.apply_scale (1, 2), base);
>       default_prob = default_prob.apply_scale (1, 2);
>       base -= default_prob;
>     }
>@@ -1147,9 +1145,10 @@ expand_case (gswitch *stmt)
>   do_pending_stack_adjust ();
> 
>   /* Find the default case target label.  */
>-  default_label = jump_target_rtx
>-      (CASE_LABEL (gimple_switch_default_label (stmt)));
>-  edge default_edge = EDGE_SUCC (bb, 0);
>+  tree default_lab = CASE_LABEL (gimple_switch_default_label (stmt));
>+  default_label = jump_target_rtx (default_lab);
>+  basic_block default_bb = label_to_block_fn (cfun, default_lab);
>+  edge default_edge = find_edge (bb, default_bb);
>   profile_probability default_prob = default_edge->probability;
> 
>   /* Get upper and lower bounds of case values.  */
>@@ -1245,9 +1244,10 @@ expand_case (gswitch *stmt)
> 	{
> 	  default_label = NULL;
> 	  remove_edge (default_edge);
>+	  default_edge = NULL;
> 	}
>       emit_case_dispatch_table (index_expr, index_type,
>-				case_list, default_label,
>+				case_list, default_label, default_edge,
> 				minval, maxval, range, bb);
>     }
> 
>@@ -1340,9 +1340,9 @@ expand_sjlj_dispatch_table (rtx dispatch
> 	}
> 
>       emit_case_dispatch_table (index_expr, index_type,
>-				case_list, default_label,
>+				case_list, default_label, NULL,
> 				minval, maxval, range,
>-                                BLOCK_FOR_INSN (before_case));
>+				BLOCK_FOR_INSN (before_case));
>       emit_label (default_label);
>     }
> 
>
>	Jakub
diff mbox

Patch

--- gcc/stmt.c.jj	2017-07-14 13:04:47.000000000 +0200
+++ gcc/stmt.c	2017-08-04 14:36:28.936447265 +0200
@@ -945,8 +945,8 @@  conditional_probability (profile_probabi
 static void
 emit_case_dispatch_table (tree index_expr, tree index_type,
 			  struct case_node *case_list, rtx default_label,
-			  tree minval, tree maxval, tree range,
-                          basic_block stmt_bb)
+			  edge default_edge,  tree minval, tree maxval,
+			  tree range, basic_block stmt_bb)
 {
   int i, ncases;
   struct case_node *n;
@@ -954,7 +954,6 @@  emit_case_dispatch_table (tree index_exp
   rtx_insn *fallback_label = label_rtx (case_list->code_label);
   rtx_code_label *table_label = gen_label_rtx ();
   bool has_gaps = false;
-  edge default_edge = stmt_bb ? EDGE_SUCC (stmt_bb, 0) : NULL;
   profile_probability default_prob = default_edge ? default_edge->probability
 						  : profile_probability::never ();
   profile_probability base = get_outgoing_edge_probs (stmt_bb);
@@ -1026,9 +1025,8 @@  emit_case_dispatch_table (tree index_exp
          through the indirect jump or the direct conditional jump
          before that. Split the probability of reaching the
          default label among these two jumps.  */
-      new_default_prob = conditional_probability (default_prob.apply_scale
-							 (1, 2),
-                                                  base);
+      new_default_prob
+	= conditional_probability (default_prob.apply_scale (1, 2), base);
       default_prob = default_prob.apply_scale (1, 2);
       base -= default_prob;
     }
@@ -1147,9 +1145,10 @@  expand_case (gswitch *stmt)
   do_pending_stack_adjust ();
 
   /* Find the default case target label.  */
-  default_label = jump_target_rtx
-      (CASE_LABEL (gimple_switch_default_label (stmt)));
-  edge default_edge = EDGE_SUCC (bb, 0);
+  tree default_lab = CASE_LABEL (gimple_switch_default_label (stmt));
+  default_label = jump_target_rtx (default_lab);
+  basic_block default_bb = label_to_block_fn (cfun, default_lab);
+  edge default_edge = find_edge (bb, default_bb);
   profile_probability default_prob = default_edge->probability;
 
   /* Get upper and lower bounds of case values.  */
@@ -1245,9 +1244,10 @@  expand_case (gswitch *stmt)
 	{
 	  default_label = NULL;
 	  remove_edge (default_edge);
+	  default_edge = NULL;
 	}
       emit_case_dispatch_table (index_expr, index_type,
-				case_list, default_label,
+				case_list, default_label, default_edge,
 				minval, maxval, range, bb);
     }
 
@@ -1340,9 +1340,9 @@  expand_sjlj_dispatch_table (rtx dispatch
 	}
 
       emit_case_dispatch_table (index_expr, index_type,
-				case_list, default_label,
+				case_list, default_label, NULL,
 				minval, maxval, range,
-                                BLOCK_FOR_INSN (before_case));
+				BLOCK_FOR_INSN (before_case));
       emit_label (default_label);
     }