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

Duncan P. N. Exon Smith via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 23 12:21:14 PDT 2016


Note the following for an answer to Daniel's query:
  http://lists.llvm.org/pipermail/llvm-dev/2016-August/103997.html
  http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20160822/384491.html

> On 2016-Aug-19, at 15:17, Duncan P. N. Exon Smith via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> 
>> 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.

^ this is what I've linked to at the top.

>> 
>> 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.
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list