On May 8, 2017 6:41:01 PM GMT+02:00, Peter Bergner <[email protected]> wrote:
>On 05/03/2017 08:32 AM, Richard Biener wrote:
> > As for Bernhards concern I share this -- please intead make the
> > interface take either a gimple_seq or a gimple_stmt_iterator
> > instead of a basic-block. That makes it more obvious you
> > can't use things like gsi_after_labels. Also I think it's more
> > natural to work backwards as the last stmt in the sequence
> > _has_ to be __builtin_unreachable () and thus checking that first
> > is the cheapest thing to do given that in most cases it will
> > not be __builtin_unreachable () (but a noreturn call or an
> > inifinite loop).
> >
> > Thus, name it gimple_seq_unreachable_p.
>
>So you mean something like the following?
Yes.
>
>/* Returns true if the sequence of statements STMTS only contains
> a call to __builtin_unreachable (). */
>
>bool
>gimple_seq_unreachable_p (gimple_seq stmts)
>{
> gimple_stmt_iterator gsi = gsi_last (stmts);
>
> if (!gimple_call_builtin_p (gsi_stmt (gsi), BUILT_IN_UNREACHABLE))
> return false;
>
> for (gsi_prev (&gsi); !gsi_end_p (gsi); gsi_prev (&gsi))
> {
> gimple *stmt = gsi_stmt (gsi);
> if (gimple_code (stmt) != GIMPLE_LABEL
> && !is_gimple_debug (stmt)
> && !gimple_clobber_p (stmt))
> return false;
> }
> return true;
>}
>
>
>
> > On Wed, 26 Apr 2017, Peter Bergner wrote:
> >> One difference from the last patch is that I am no longer setting
> >> default_label to NULL when we emit a decision tree. I noticed that
> >> the decision tree code seemed to generate slightly better code for
> >> some of my unit tests if I left it alone. This simplified the
> >> patch somewhat by removing the changes to emit_case_nodes().
>[snip]
> >
> > Can you do the gimple_unreachable_bb_p check earlier in
> > expand_case so it covers the emit_case_decision_tree path as well
> > (and verify that works, of course)? So basically right at
> >
> > /* 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);
> > int default_prob = default_edge->probability;
> >
> > handle this case.
>
>That is what the previous patch did, but as I mention above,
>we generate slightly better code for some test cases (other
>tests seemed to generate the same code) if we don't attempt
>to handle the decision tree case. I'll note that the current
>unpatched compiler already knows how to remove unreachable
>case statement blocks when we expand to a decision tree.
>
>I can add that code back if you think that it will have a
>positive benefit for some test case I haven't tried yet.
>
>Peter
