[llvm] r290683 - [NewGVN] Simplyfy loop NFC

Piotr Padlewski via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 29 01:23:48 PST 2016


CurrIter have only one use.


auto CurrIter = MI;
++MI;
Value *Member = *CurrIter;

I can rewrite beginning of loop as
Value *Member = *MI++;

Which makes it trivial that it is range for loop.

Of course I am not 100% sure, operator* and operator++ might do some weird
things here, but I assumed they are sane.
Tests also passed


2016-12-29 2:20 GMT+01:00 Sean Silva <chisophugis at gmail.com>:

>
>
> On Wed, Dec 28, 2016 at 11:42 AM, Piotr Padlewski via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: prazek
>> Date: Wed Dec 28 13:42:49 2016
>> New Revision: 290683
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=290683&view=rev
>> Log:
>> [NewGVN] Simplyfy loop NFC
>>
>> Modified:
>>     llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp
>>
>> Modified: llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transform
>> s/Scalar/NewGVN.cpp?rev=290683&r1=290682&r2=290683&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp Wed Dec 28 13:42:49 2016
>> @@ -1915,10 +1915,7 @@ bool NewGVN::eliminateInstructions(Funct
>>
>>      // Cleanup the congruence class.
>>      SmallPtrSet<Value *, 4> MembersLeft;
>> -    for (auto MI = CC->Members.begin(), ME = CC->Members.end(); MI !=
>> ME;) {
>> -      auto CurrIter = MI;
>> -      ++MI;
>> -      Value *Member = *CurrIter;
>> +    for (Value * Member : CC->Members) {
>>
>
> Is this correct? Usually this kind of loop where the iterator is
> incremented at the start of the loop will invalidate the iterator somewhere
> later, so rewriting it to be a range for (or a simple loop with the
> increment inside the `for` header where it usually goes) will introduce a
> nasty bug.
>
> I haven't looked closely enough in this particular case, but I'd
> definitely like to see a bit more justification (ideally in the commit
> message in the future) for the safety of this refactoring when dealing with
> this kind of "increment at the beginning" loops.
>
> -- Sean Silva
>
>
>>        if (Member->getType()->isVoidTy()) {
>>          MembersLeft.insert(Member);
>>          continue;
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161229/209f245d/attachment.html>


More information about the llvm-commits mailing list