On Tue, Jul 19, 2016 at 11:52 AM, Bernd Schmidt <bschm...@redhat.com> wrote:
> On 07/19/2016 10:07 AM, Richard Biener wrote:
>>
>>
>> This is not appropriate for CFG cleanup due to its complexity not
>> being O(# bbs + # edges).
>> I tried hard in the past to make it so (at least when no transform is
>> done).
>
>
> Why wouldn't it be, if no transform is done? Assuming we visit each bb once,
> we have at worst one walk over the successor edges of the predecessor (if we
> even find two switches on the same variable), and then we can decide whether
> or not to do the transformation.
I saw walks over stmts of a BB. IMHO that's a no-go.
That said, CFG cleanup is not the place for this optimization.
The only trivial CFG cleanup transform for switches I can see is transforming
them to a simple if / else in case there is a single non-default
label. And it's
not even doing that currently.
> When performing the transformation I could imagine one could construct a
> testcase where lots of these switches are nested inside each other, but I'm
> not convinved that's really a realistic worry.
>
>> Please move this transform elsewhere. I suggest the switch-conversion
>> pass or if that
>> is not a good fit then maybe if-combine (whose transforms are remotely
>> related).
>
>
> One problem is that this triggers rarely, but when it does, it occurs at
> various stages in the compilation after other optimizations have been done.
> Moving it to any given point is likely to limit the effectiveness.
Well, that's true for all optimization passes we have. The opportunity once it
arises is not likely to be removed by any pass.
>> Not looking closer at the patch but missing some comments on how it deals
>> with
>> common cases (you see to handle fallthrus to the default label by
>> ignoring them?)
>
>
> If you are thinking of
>
> switch (a)
> {
> case n:
> case m:
> default:
> switch (a) { .... }
> }
>
> then the cases for n and m can simply be dropped when merging from the
> second switch into the first one. That's what happens, and there's a comment
> for it. So please elaborate what you mean.
I'm thinking of
switch (a)
{
...
case n:
do-stuff;
default:
switch (a)
{
case n:
do-stuff;
...
}
}
yes, you can simply drop cases when there is no code in the outer switch.
Richard.
>
>
> Bernd
>
