[PATCH] D29064: [MemorySSA] Add invariant.group handling

George Burgess IV via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 23 18:51:30 PST 2017


george.burgess.iv added a comment.

Thanks for this!

Haven't looked over the patch in great detail, but I do have a few high-level questions:

- Is it possible for an invariant group barrier to ever be removed from/added to a function? If so, then we're either going to have to treat it as Def, or we're going to have to add a `byTheWayThisInvariantBarrierIsNowDead` and/or a `weHaveANewInvariantBarrier` function to MemorySSA.

- Is it possible to add/remove/move an operation with invariant group metadata on it? If so, we'll probably need to change the update API to handle those cases sanely (otherwise, looks like PerBlockInvariantGroupAccesses can end up with dangling pointers?)



================
Comment at: lib/Transforms/Utils/MemorySSA.cpp:1338
+    // FIXME: This doesn't check if stack if stack empty.
+    // fix it, or write a comment if sentinel exists.
     while (VersionStack.back()->getBlock() == BackBlock)
----------------
`VersionStack.push_back(MSSA->getLiveOnEntryDef());` below takes care of that; `liveOnEntryDef` exists in the entry block, so it always dominates everything. :)

If you'd like to add commentary/asserts, feel free (though preferably as a separate commit).


https://reviews.llvm.org/D29064





More information about the llvm-commits mailing list