[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