On May 8, 2017 6:41:01 PM GMT+02:00, Peter Bergner <berg...@vnet.ibm.com> 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