[PATCH] D21463: Add MemoryAccess creation and PHI creation APIs to MemorySSA

[Daniel Berlin via llvm-commits]

On Mon, Jun 20, 2016 at 12:21 PM, George Burgess IV <
george.burgess.iv at gmail.com> wrote:

> george.burgess.iv added a comment.
>
> Looks good overall -- just a few nits.
>
>
> ================
> Comment at: include/llvm/Transforms/Utils/MemorySSA.h:553
> @@ +552,3 @@
> +  /// It will return the new MemoryAccess.
> +  MemoryAccess *createMemoryAccess(Instruction *I, MemoryAccess
> *Definition,
> +                                   const BasicBlock *BB, InsertionPlace
> Point);
> ----------------
> `createMemoryAccessInBB`, maybe? Naming is hard.
>

Fixed

>
> ================
> Comment at: include/llvm/Transforms/Utils/MemorySSA.h:557
> @@ +556,3 @@
> +  /// MemoryAccess.
> +  //
> +  /// Returns the new MemoryAccess.
> ----------------
> ///
>

Fixed

>
> ================
> Comment at: lib/Transforms/Utils/MemorySSA.cpp:589
> @@ +588,3 @@
> +/// order and existence of memory affecting instructions.
> +
> +void MemorySSA::verifyOrdering(Function &F) const {
> ----------------
> Please remove this newline
>
Fixed.


>
> ================
> Comment at: lib/Transforms/Utils/MemorySSA.cpp:595
> @@ +594,3 @@
> +  for (BasicBlock &B : F) {
> +    SmallVector<MemoryAccess *, 32> ActualAccesses;
> +    const AccessListType *AL = getBlockAccesses(&B);
> ----------------
> Bring this out of the loop + clear on each iteration, please.
>

Fixed.


>
> ================
> Comment at: lib/Transforms/Utils/MemorySSA.cpp:602
> @@ +601,3 @@
> +      MemoryAccess *MA = getMemoryAccess(&I);
> +      assert((!MA || AL != nullptr) && "We have memory affecting
> instructions "
> +                                       "in this block but they are not in
> the "
> ----------------
> Varying compare-against-null styles?


Fixed.


>
>

> http://reviews.llvm.org/D21463
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160620/5c6f3c6b/attachment.html>