[PATCH] D16950: Compute live-in for MemorySSA

[Daniel Berlin via llvm-commits]

On Sat, Feb 6, 2016 at 4:34 PM, David Majnemer <david.majnemer at gmail.com>
wrote:

> majnemer added a subscriber: majnemer.
> majnemer accepted this revision.
> majnemer added a reviewer: majnemer.
> majnemer added a comment.
> This revision is now accepted and ready to land.
>
> This LGTM.  An argument could be made, either for or against, that the
> change to minimize the number of attempts to update `DefiningBlocks` could
> be split out.
>
> i can stage it into two SVN commits :)

>
> ================
> Comment at: lib/Transforms/Utils/MemorySSA.cpp:308-313
> @@ -267,1 +307,8 @@
> +    // live into it to.
> +    for (pred_iterator PI = pred_begin(BB), E = pred_end(BB); PI != E;
> ++PI) {
> +      BasicBlock *P = *PI;
> +
> +      // Otherwise it is, add to the worklist.
> +      LiveInBlockWorklist.push_back(P);
> +    }
>    }
> ----------------
> Any reason not to use the range-based `predecessors`?
>

No.

> It should make the code a little shorter:
>   for (BasicBlock *P : predecessors(BB))
>     LiveInBlockWorklist.push_back(P);
>
> I guess you could also use `append` instead:
>   LiveInBlockWorklist.append(pred_begin(BB), pred_end(BB));
>

I think i'm going to do this one :)


>
> ================
> Comment at: test/Transforms/Util/MemorySSA/livein.ll:26
> @@ +25,3 @@
> +merge:
> +;CHECK: 2 = MemoryPhi({left,1},{right,liveOnEntry})
> +%c = load i8, i8* %0
> ----------------
> Would you mind adding a space between that semicolon and the CHECK to
> match the others?
>
>
> http://reviews.llvm.org/D16950
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160206/5ab63432/attachment.html>