This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Expand switch statements with a single (or none) non-default case.
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Martin Liška <mliska at suse dot cz>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 1 Sep 2017 10:26:30 +0200
- Subject: Re: [PATCH] Expand switch statements with a single (or none) non-default case.
- Authentication-results: sourceware.org; auth=none
- References: <bde90963-7695-89d5-6792-e8f803f71160@suse.cz> <CAFiYyc0bNt7Ysumz_ZVXW8uiMpCAfDhPKZatiGMET6rRNiXRtQ@mail.gmail.com> <b9003663-ce11-8f29-6ebb-624077b53157@suse.cz> <CAFiYyc3VE78iX+sgXoz_DKYP-LWdxBrE=D5WjBRN-e45xZdyGw@mail.gmail.com> <60ea6209-3b23-6398-da99-19326e24d8c2@suse.cz>
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
>>>>>
>>>>>
>>>
>