[PATCH] D22778: Add Loop Sink pass to reverse the LICM based of basic block frequency.

[Duncan P. N. Exon Smith via llvm-commits]

> On 2016-Aug-19, at 15:09, Daniel Berlin <dberlin at dberlin.org> wrote:
> 
> Adding duncan since he (probably to his dismay), knows a lot about ilist:
>  
> Adding
> Because the reverse iterator does not support in-place replacement. If I move the current instruction out of the BB, even if I pre-increment the iterator before, the pre-incremented iterator will also be moved to the new BB. I checked around llvm code base, seems there is no in-place update for reverse iterators.
> 
> That seems ... broken.

Yes.

> 
> I assume the preincrement issue is due to how std::reverse_iterator works

Yes.  ilist::reverse_iterator is currently std::reverse_iterator<ilist::iterator>, which is an insane model for a list.

> (duncan, i assume nothing can be done here)

Actually, I have a patch that fixes this.  Assuming r279314 sticks (isn't reverted over the weekend), I should be able to commit it sometime next week.

> 
> However, that just probably means we should make movebefore do the same thing we made eraseFromParent do, so you can construct iterators from the result.

In the meantime I actually recommend not using reverse iterators at all if you're going to invalidate anything.  They're too confusing.