[llvm] r290683 - [NewGVN] Simplyfy loop NFC

[Sean Silva via llvm-commits]

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/
> Transforms/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/20161228/5ab5bae9/attachment.html>