From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24813 invoked by alias); 1 Sep 2017 10:44:13 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 24800 invoked by uid 89); 1 Sep 2017 10:44:13 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.2 spammy=H*M:bc94 X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 01 Sep 2017 10:44:11 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 5446CAB43; Fri, 1 Sep 2017 10:44:09 +0000 (UTC) Subject: Re: [PATCH] Expand switch statements with a single (or none) non-default case. To: Richard Biener Cc: GCC Patches References: <60ea6209-3b23-6398-da99-19326e24d8c2@suse.cz> From: =?UTF-8?Q?Martin_Li=c5=a1ka?= Message-ID: Date: Fri, 01 Sep 2017 10:44:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2017-09/txt/msg00021.txt.bz2 On 09/01/2017 10:26 AM, Richard Biener wrote: > On Fri, Sep 1, 2017 at 10:07 AM, Martin Liška wrote: >> On 08/30/2017 02:56 PM, Richard Biener wrote: >>> On Wed, Aug 30, 2017 at 2:32 PM, Martin Liška wrote: >>>> On 08/30/2017 02:28 PM, Richard Biener wrote: >>>>> On Wed, Aug 30, 2017 at 1:13 PM, Martin Liška 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). Good, so I will leave it to you. Should I in meantime remove the assert in tree-switch-conversion.c ? > >> 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. Working on that, good place to do the transformation. Martin > >> Thoughts? >> >> Martin >> >>> >>> Richard. >>> >>>> Martin >>>> >>>>> >>>>> Richard. >>>>> >>>>>> Martin >>>>>> >>>>>> gcc/ChangeLog: >>>>>> >>>>>> 2017-08-25 Martin Liska >>>>>> >>>>>> 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 >>>>>> >>>>>> 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 >>>>>> >>>>>> >>>> >>