[PATCH] D8688: Update MergedLoadStoreMotion to use MemorySSA

Daniel Berlin via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 22 13:22:12 PDT 2016


On Wed, Jun 22, 2016 at 1:16 PM, George Burgess IV <
george.burgess.iv at gmail.com> wrote:

> george.burgess.iv added a comment.
>
> Drive-by comments about comments (insert "we need to go deeper" meme here)
>
>
> ================
> Comment at: lib/Transforms/Scalar/MergedLoadStoreMotion.cpp:791
> @@ +790,3 @@
> +  // Walk all accesses in first block, put them into a hash table.
> +  // We can't just use a norm
> +  for (auto &Access : *Succ0Accesses) {
> ----------------
> Potentially unfinished thought?
>
>
Don't remember what it was, removed.


> ================
> Comment at: lib/Transforms/Scalar/MergedLoadStoreMotion.cpp:797
> @@ +796,3 @@
> +    Instruction *I = MU->getMemoryInst();
> +    // Only move non-simple (atomic, volatile) loads.
> +    LoadInst *Load0 = dyn_cast<LoadInst>(I);
> ----------------
> Did you mean "only move simple (non-atomic, non-volatile)"?
>
> yes

>
> http://reviews.llvm.org/D8688
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160622/a80ba7d5/attachment.html>


More information about the llvm-commits mailing list