[PATCH] D51327: [MemorySSA] Add expesive check for validating clobber accesses.

[George Burgess IV via Phabricator via llvm-commits]

george.burgess.iv accepted this revision.
george.burgess.iv added a comment.
This revision is now accepted and ready to land.

LGTM after the requested nit. Thanks again!



================
Comment at: lib/Analysis/MemorySSA.cpp:1705
+    UpwardsMemoryQuery Q(I, MUD);
+    checkClobberSanity(const_cast<MemoryUseOrDef *>(MUD), Clobber, *Loc, *this, Q, *AA);
+  }
----------------
asbirlea wrote:
> Nit: keep the `const` up until `upward_defs_begin` and const_cast there?
> (Worklist can be ConstMemoryAccessPair).
> This seemed cleaner, but I'm ok with it either way.
> Nit: keep the const up until upward_defs_begin and const_cast there?

Yes, please.

Also please add a FIXME or note that, given infinite free time, we should probably refactor to make upward_defs_iterator respect constness...

It's a slightly larger change, but it pushes this const_cast as close to the point of "here's where this isn't annotated with const but logically is" as we can easily get it, which makes the safety of the `const_cast` a bit easier to reason about IMO (and keeps the API a bit cleaner).


Repository:
  rL LLVM

https://reviews.llvm.org/D51327