On Mon, Dec 17, 2012 at 11:26:23AM +0100, Richard Biener 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)?
Yeah, this is my understanding.
We had:
flags:CCZ=cmp(r66:DI,r67:DI)
, but CSE noticed that r66:DI is in fact r67:DI, so it produces:
flags:CCZ=cmp(r66:DI,r67:DI)
thus we always take one edge and the another one is not needed.
> 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).
I'll try hack something up in remove_edge, as you suggested on IRC.
Thanks,
Marek
