Re: [PATCH] Expand switch statements with a single (or none) non-default case.

[Richard Biener <richard dot guenther at gmail dot com>]

On Fri, Sep 1, 2017 at 10:07 AM, Martin Liška <mliska@suse.cz> wrote:
> On 08/30/2017 02:56 PM, Richard Biener wrote:
>> On Wed, Aug 30, 2017 at 2:32 PM, Martin Liška <mliska@suse.cz> wrote:
>>> On 08/30/2017 02:28 PM, Richard Biener wrote:
>>>> On Wed, Aug 30, 2017 at 1:13 PM, Martin Liška <mliska@suse.cz> wrote:
>>>>> Hi.
>>>>>
>>>>> Simple transformation of switch statements where degenerated switch can be interpreted
>>>>> as gimple condition (or removed if having any non-default case). I originally though
>>>>> that we don't have switch statements without non-default cases, but PR82032 shows we
>>>>> can see it.
>>>>>
>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>>>>
>>>>> Ready to be installed?
>>>>
>>>> While I guess this case is ok to handle here it would be nice if CFG cleanup
>>>> would do the same.  I suppose find_taken_edge somehow doesn't work for
>>>> this case even after my last enhancement?  Or is CFG cleanup for some reason
>>>> not run?
>>>
>>> Do you mean both with # of non-default edges equal to 0 and 1?
>>> Let me take a look.
>>
>> First and foremost 0.  The case of 1 non-default and a default would
>> need extra code.
>
> For the test-case I reduced, one needs:
>
> diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
> index b7593068ea9..13af516c6ac 100644
> --- a/gcc/tree-cfg.c
> +++ b/gcc/tree-cfg.c
> @@ -8712,7 +8712,7 @@ const pass_data pass_data_split_crit_edges =
>    PROP_no_crit_edges, /* properties_provided */
>    0, /* properties_destroyed */
>    0, /* todo_flags_start */
> -  0, /* todo_flags_finish */
> +  TODO_cleanup_cfg, /* todo_flags_finish */
>  };
>
>  class pass_split_crit_edges : public gimple_opt_pass
>
> And the code eliminates the problematic switch statement. Do you believe it's the right approach
> to add the clean up and preserve the assert in tree-switch-conversion.c?

Eh, no.  If you run cleanup-cfg after critical edge splitting they
will be unsplit immediately, making
it (mostly) a no-op.

OTOH I wanted to eliminate that "pass" in favor of just calling
split_critical_edges () where needed
(that's already done in some places).

> For the case with # of edges == 1, should I place it to tree-cfg.c in order to trigger it as a clean-up?

I believe the code for edges == 1 can reside in
cleanup_control_expr_graph.  Probably easiest
from a flow perspective if we do the switch -> cond transform before
the existing code handling
cond and switch via find-taken-edge.

> Thoughts?
>
> Martin
>
>>
>> Richard.
>>
>>> Martin
>>>
>>>>
>>>> Richard.
>>>>
>>>>> Martin
>>>>>
>>>>> gcc/ChangeLog:
>>>>>
>>>>> 2017-08-25  Martin Liska  <mliska@suse.cz>
>>>>>
>>>>>         PR tree-optimization/82032
>>>>>         * tree-switch-conversion.c (generate_high_low_equality): New
>>>>>         function.
>>>>>         (expand_degenerated_switch): Likewise.
>>>>>         (process_switch): Call expand_degenerated_switch.
>>>>>         (try_switch_expansion): Likewise.
>>>>>         (emit_case_nodes): Use generate_high_low_equality.
>>>>>
>>>>> gcc/testsuite/ChangeLog:
>>>>>
>>>>> 2017-08-25  Martin Liska  <mliska@suse.cz>
>>>>>
>>>>>         PR tree-optimization/82032
>>>>>         * gcc.dg/tree-ssa/pr68198.c: Update jump threading expectations.
>>>>>         * gcc.dg/tree-ssa/switch-expansion.c: New test.
>>>>>         * gcc.dg/tree-ssa/vrp34.c: Update scanned pattern.
>>>>>         * g++.dg/other/pr82032.C: New test.
>>>>> ---
>>>>>  gcc/testsuite/g++.dg/other/pr82032.C             |  36 +++++++
>>>>>  gcc/testsuite/gcc.dg/tree-ssa/pr68198.c          |   6 +-
>>>>>  gcc/testsuite/gcc.dg/tree-ssa/switch-expansion.c |  14 +++
>>>>>  gcc/testsuite/gcc.dg/tree-ssa/vrp34.c            |   5 +-
>>>>>  gcc/tree-switch-conversion.c                     | 123 ++++++++++++++++++-----
>>>>>  5 files changed, 152 insertions(+), 32 deletions(-)
>>>>>  create mode 100644 gcc/testsuite/g++.dg/other/pr82032.C
>>>>>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/switch-expansion.c
>>>>>
>>>>>
>>>
>