[PATCH] D67562: [MemorySSA] Update MSSA for non-conventional AA.

[George Burgess IV via Phabricator via llvm-commits]

george.burgess.iv added a comment.

thanks for this!

i like the new approach much more :)



================
Comment at: lib/Analysis/MemorySSA.cpp:1742
 
+  if (!I->mayReadFromMemory() && !I->mayWriteToMemory())
+    return nullptr;
----------------
does obviate the need for the recently-added `isa<DbgInfoIntrinsic>()`?

if so, please add a comment noting that nonstandard AA pipelines might leave us with silly modref results for `I`, so this check is necessary for correctness.


================
Comment at: test/Analysis/MemorySSA/loop-rotate-disablebasicaa.ll:5
+; CHECK-LABEL: @main
+define  void @main() {
+entry:
----------------
world's smallest nit: `s/define  void/define void/`


================
Comment at: test/Analysis/MemorySSA/loop-rotate-disablebasicaa.ll:10
+for.cond120:                                      ; preds = %for.body127, %entry
+  call void @foo()
+  br i1 undef, label %for.body127, label %for.cond.cleanup126
----------------
should we be `CHECK-NOT`'ing a MemoryDef for this? seems more direct than expecting a verifier error.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67562/new/

https://reviews.llvm.org/D67562