On Mon, Dec 17, 2012 at 11:26 AM, Richard Biener
<richard.guent...@gmail.com> wrote:
> On Mon, Dec 17, 2012 at 11:16 AM, Marek Polacek <pola...@redhat.com> wrote:
>> On Mon, Dec 17, 2012 at 10:27:54AM +0100, Richard Biener wrote:
>>> This looks like the wrong place to fix (the delete-basic-block cfghook
>>> tryings to fixup loops are incredibly fragile, because usually
>>> delete_basic_block
>>> is called because of another cfg manipulation takes place).  That is,
>>> the right cfghook place would be where the latch edge is deleted (of course
>>> you cannot know whether it'll be just redirected - thus the fragility of 
>>> cfghook
>>> fixes for loops).
>>>
>>> Which pass does this deletion?  The correct fix is to fix that pass to
>>> correctly care about the high-level CFG transform it performs.
>>
>> It's cse1.  I didn't see any place in there where I could fix things up,
>> since it looks we aren't directly manipulating the CFG there (it
>> rather find paths, stores them in ebb data, then walks the insns in BBs,
>> and calls cse_insn on each of them, but it's so big
>> and complex that I'm very likely wrong here), only
>> via cleanup_cfg at the end of the pass, which is what calls
>> delete_unreachable_blocks->delete_basic_block, here we delete two
>> latch nodes.  It seems legal to delete them, because at the end of BBs
>> before these latches is an unconditional jump at (label_ref 67).
>> I don't know how could we teach the CSE beast to care about high-level
>> CFG transformations.  Thanks,
>
> Hmm, I think I remember this case ... (and I fixed it up in
> cfg_cleanup I think).
>
> So I suppose cse turns a conditional jump into an unconditional one (but
> maybe only cfg_cleanup realizes that)?
>
> I think that whoever figures out the latch edge is never taken ought to fixup
> loop structure.  Btw, what also could be done is trying to teach
> fix_loop_structure
> of this case (but only as a last resort I think).

That said, ISTR playing with removing the loop if remove_edge would remove
the last latch from it.  Note that the only reliable thing with preserved loops
but not loop_optimizer_init called is the loop->header field and the only
basic-block with reliable ->loop_father is the header block itself.

Richard.

> Richard.
>
>>         Marek

Reply via email to