[PATCH] D23184: Rewrite domination verifier to handle local domination as well.
Daniel Berlin via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 5 12:24:10 PDT 2016
On Fri, Aug 5, 2016 at 12:04 PM, George Burgess IV <
george.burgess.iv at gmail.com> wrote:
> george.burgess.iv accepted this revision.
> george.burgess.iv added a comment.
> This revision is now accepted and ready to land.
>
> LGTM with comments, thanks.
>
>
> ================
> Comment at: include/llvm/Transforms/Utils/MemorySSA.h:594
> @@ +593,3 @@
> + /// dominates Use \p B.
> + bool dominates(const MemoryAccess *A, const Use &B) const;
> +
> ----------------
> Do we see this being used outside of verification?
Yes, actually.
It also matches what Dominators.h provides, and that gets used.
> If not, make this private, please.
>
> ================
> Comment at: lib/Transforms/Utils/MemorySSA.cpp:1872
> @@ -1891,1 +1871,3 @@
> + for (const Use &U : MP->uses())
> + assert(dominates(MP, U) && "Memory PHI does not dominate it's
> uses");
> for (Instruction &I : B) {
> ----------------
> Looks like this definition is provided if `NDEBUG` is defined. Please
> braces around the assert. :)
>
> ================
> Comment at: lib/Transforms/Utils/MemorySSA.cpp:1879
> @@ -1913,1 +1878,3 @@
> + for (const Use &U : MD->uses())
> + assert(dominates(MD, U) && "Memory Def does not dominate it's
> uses");
> }
> ----------------
> Same as above
>
>
> https://reviews.llvm.org/D23184
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160805/e9f85d5a/attachment.html>
More information about the llvm-commits
mailing list