This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix switch expansion (PR middle-end/81698)
- From: Richard Biener <rguenther at suse dot de>
- To: Jakub Jelinek <jakub at redhat dot com>,Martin Liška <mliska at suse dot cz>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Fri, 04 Aug 2017 20:22:21 +0200
- Subject: Re: [PATCH] Fix switch expansion (PR middle-end/81698)
- Authentication-results: sourceware.org; auth=none
- References: <20170804154221.GG2123@tucnak>
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