Re: [PATCH, v3] Fix PR51513, switch statement with default case containing __builtin_unreachable leads to wild branch

[Peter Bergner <bergner@vnet.ibm.com>]

On 05/08/2017 12:44 PM, Richard Biener wrote:

>>> 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.

Any comment on the above?

Peter