[llvm] r293361 - MemorySSA: Fix block numbering invalidation and replacement bugs discovered by updater

[Manuel Klimek via llvm-commits]

Fails with asan, Sam is going to revert.

On Sat, Jan 28, 2017 at 3:34 AM Daniel Berlin via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: dannyb
> Date: Fri Jan 27 20:22:52 2017
> New Revision: 293361
>
> URL: http://llvm.org/viewvc/llvm-project?rev=293361&view=rev
> Log:
> MemorySSA: Fix block numbering invalidation and replacement bugs
> discovered by updater
>
> Modified:
>     llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp
>     llvm/trunk/lib/Transforms/Utils/MemorySSAUpdater.cpp
>
> Modified: llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp?rev=293361&r1=293360&r2=293361&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp Fri Jan 27 20:22:52 2017
> @@ -1597,6 +1597,7 @@ void MemorySSA::insertIntoListsForBlock(
>        Defs->push_back(*NewAccess);
>      }
>    }
> +  BlockNumberingValid.erase(BB);
>  }
>
>  void MemorySSA::insertIntoListsBefore(MemoryAccess *What, const
> BasicBlock *BB,
> @@ -1624,6 +1625,7 @@ void MemorySSA::insertIntoListsBefore(Me
>          Defs->insert(InsertPt->getDefsIterator(), *What);
>      }
>    }
> +  BlockNumberingValid.erase(BB);
>  }
>
>  // Move What before Where in the IR.  The end result is taht What will
> belong to
> @@ -1638,13 +1640,19 @@ void MemorySSA::moveTo(MemoryUseOrDef *W
>    insertIntoListsBefore(What, BB, Where);
>  }
>
> +void MemorySSA::moveTo(MemoryUseOrDef *What, BasicBlock *BB,
> +                       InsertionPlace Point) {
> +  removeFromLists(What, false);
> +  What->setBlock(BB);
> +  insertIntoListsForBlock(What, BB, Point);
> +}
> +
>  MemoryPhi *MemorySSA::createMemoryPhi(BasicBlock *BB) {
>    assert(!getMemoryAccess(BB) && "MemoryPhi already exists for this BB");
>    MemoryPhi *Phi = new MemoryPhi(BB->getContext(), BB, NextID++);
> +  // Phi's always are placed at the front of the block.
>    insertIntoListsForBlock(Phi, BB, Beginning);
>    ValueToMemoryAccess[BB] = Phi;
> -  // Phi's always are placed at the front of the block.
> -  BlockNumberingValid.erase(BB);
>    return Phi;
>  }
>
> @@ -1665,7 +1673,6 @@ MemoryAccess *MemorySSA::createMemoryAcc
>                                                  InsertionPlace Point) {
>    MemoryUseOrDef *NewAccess = createDefinedAccess(I, Definition);
>    insertIntoListsForBlock(NewAccess, BB, Point);
> -  BlockNumberingValid.erase(BB);
>    return NewAccess;
>  }
>
> @@ -1675,7 +1682,6 @@ MemoryUseOrDef *MemorySSA::createMemoryA
>    assert(I->getParent() == InsertPt->getBlock() &&
>           "New and old access must be in the same block");
>    MemoryUseOrDef *NewAccess = createDefinedAccess(I, Definition);
> -  BlockNumberingValid.erase(InsertPt->getBlock());
>    insertIntoListsBefore(NewAccess, InsertPt->getBlock(),
>                          InsertPt->getIterator());
>    return NewAccess;
> @@ -1687,7 +1693,6 @@ MemoryUseOrDef *MemorySSA::createMemoryA
>    assert(I->getParent() == InsertPt->getBlock() &&
>           "New and old access must be in the same block");
>    MemoryUseOrDef *NewAccess = createDefinedAccess(I, Definition);
> -  BlockNumberingValid.erase(InsertPt->getBlock());
>    insertIntoListsBefore(NewAccess, InsertPt->getBlock(),
>                          ++(InsertPt->getIterator()));
>    return NewAccess;
>
> Modified: llvm/trunk/lib/Transforms/Utils/MemorySSAUpdater.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/MemorySSAUpdater.cpp?rev=293361&r1=293360&r2=293361&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/MemorySSAUpdater.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/MemorySSAUpdater.cpp Fri Jan 27
> 20:22:52 2017
> @@ -243,13 +243,17 @@ void MemorySSAUpdater::insertDef(MemoryD
>    // of that thing with us, since we are in the way of whatever was there
>    // before.
>    // We now define that def's memorydefs and memoryphis
> -  for (auto UI = DefBefore->use_begin(), UE = DefBefore->use_end(); UI !=
> UE;) {
> -    Use &U = *UI++;
> -    // Leave the uses alone
> -    if (isa<MemoryUse>(U.getUser()))
> -      continue;
> -    U.set(MD);
> +  if (DefBeforeSameBlock) {
> +    for (auto UI = DefBefore->use_begin(), UE = DefBefore->use_end();
> +         UI != UE;) {
> +      Use &U = *UI++;
> +      // Leave the uses alone
> +      if (isa<MemoryUse>(U.getUser()))
> +        continue;
> +      U.set(MD);
> +    }
>    }
> +
>    // and that def is now our defining access.
>    // We change them in this order otherwise we will appear in the use list
>    // above and reset ourselves.
>
>
> _______________________________________________
> 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/20170130/b14f6f82/attachment.html>