[PATCH] D23214: [AliasAnalysis] Treat invariant.start as read-memory

[Anna Thomas via llvm-commits]

anna marked 2 inline comments as done.

================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:785
@@ +784,3 @@
+  // Like assumes, invariant.start intrinsics are also marked as arbitrarily
+  // writing so that proper control dependencies are maintained but they never
+  // mod any particular memory location visible to the IR.
----------------
anna wrote:
> dberlin wrote:
> > Errr, this is now wrong?
> > 
> > Maybe you mean "were both also marked"?
> I'm not sure if the comment is wrong. The comment intended to convey: "In other code such as MDA, instruction->MayWriteToMemory, etc, assumes and invariant.start are marked as read/write, even though they do not modify memory" However, while assume is marked as NoModRef above, we need to mark invariant.start as reading memory.
> 
> It actually follows from the above 2 intrinsic comments "assume and experimental_guard" :)
> Should I update the comment? 
> 
I've updated the comment to:
Like assumes, invariant.start intrinsics were also marked as arbitrarily
writing so that proper control dependencies are maintained but they never
mod any particular memory location visible to the IR.
*Unlike* assumes (which are now modeled as NoModRef), invariant.start intrinsic is now modeled as reading memory. This prevents hoisting the invariant.start intrinsic over stores. 


https://reviews.llvm.org/D23214