On Wed, Sep 3, 2014 at 11:22 PM, Jeff Law <l...@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
