This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: RFC: Patch for switch elimination (PR 54742)
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Jeff Law <law at redhat dot com>
- Cc: Steve Ellcey <sellcey at mips dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, james dot greenhalgh at arm dot com
- Date: Thu, 4 Sep 2014 14:57:47 +0200
- Subject: Re: RFC: Patch for switch elimination (PR 54742)
- Authentication-results: sourceware.org; auth=none
- References: <1407865606 dot 2601 dot 74 dot camel at ubuntu-sellcey> <53EA5D74 dot 9020809 at redhat dot com> <47b32a49-298e-44f0-b84b-b8f664847a67 at email dot android dot com> <53EA7BD0 dot 1030901 at redhat dot com> <CAFiYyc2U6WSFw_Cy95q27mNkFpJJVeBL8ZvAvXkO-=UpiJB4ag at mail dot gmail dot com> <5407869B dot 7030102 at redhat dot com>
On Wed, Sep 3, 2014 at 11:22 PM, Jeff Law <law@redhat.com> wrote:
> On 08/13/14 03:44, Richard Biener wrote:
>>
>>
>> I don't see that this pass should "scrog" a loop beyond repair. Btw,
>> the "proper" way of just fixing loops up (assuming that all loop
>> headers are still at their appropriate place) is to _just_ do
>> loops_set_state (LOOPS_NEED_FIXUP).
>
> This pass can quite easily create new irreducible loops and non-nested
> loops, the pass may take a previously well defined natural loop and make it
> irreducible. When I worked on it, I didn't see any reasonable way to update
> the loop structures.
The auto-fixup should end up removing the loop in this case (well,
I think so, and if not I'm happy to improve it).
Note that I think we arrived at the point where the loop structure
has annotations that are required for correctness :/ (simduid
for example - if that goes away we do ...? ICE? generate
wrong code? I don't know - Jakub shoud).
So there might be loops that we want to avoid applying such
disruptive transforms to.
>> But still this is in theory very bad as you cause important annotations
>> to be lost. If the loop is truly gone, ok, but if it just re-materializes
>> then you've done a bad job here. Consider the case where a
>> loop becomes a loop nest - you'd want to preserve the loop header
>> as the header of the outer loop (which you'd have to identify its
>> header in some way - dominator checks to the rescue!) and let
>> fixup discover the new inner loop.
>
> While I'd love to be able to be able to update and DTRT here, I just
> couldn't see a way to do it. And while I hate losing the loop structure and
> missed-optimizations that it may lead to later, I judged the benefit of
> removing the multi-way branch to be so beneficial that it outweighed the
> losses elsewhere.
>
>
>
>>
>> Yes, we may have little utility for dealing with the more complex
>> cases and I've been hesitant to enforce not dropping loops on the
>> floor an ICE (well, mainly because we can't even bootstrap with
>> such check ...), but in the end we should arrive there.
>
> It'd certainly be nice. I really don't like the idea of dropping loops on
> the floor. If we can get there, great, but I suspect you'll find its harder
> than expected.
Sure. But as said above explicitely dropping (by setting ->header
to NULL) is discouraged - it's better to get it done automatically
by only setting LOOPS_NEED_FIXUP. In some cases that's not
possible - like when we _remove_ the basic-block that ->header
or ->latch points to we obviously can't leave them point to that
block. So we either have to know better (or do a good guess
that we are sure doesn't point to a wrong "real" header/latch)
or punt and set to NULL.
Richard.
> jeff