On Fri, Sep 1, 2017 at 12:44 PM, Martin Liška <mli...@suse.cz> wrote: > On 09/01/2017 10:26 AM, Richard Biener wrote: >> On Fri, Sep 1, 2017 at 10:07 AM, Martin Liška <mli...@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 <mli...@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 <mli...@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). > > Good, so I will leave it to you. Should I in meantime remove the assert in > tree-switch-conversion.c ?
Yes, as said your patch was generally OK, I just wondered why we left the switches "unoptimized". Richard. >> >>> 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 <mli...@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 <mli...@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 >>>>>>> >>>>>>> >>>>> >>> >
