[PATCH] D11044: [ImplicitNullChecks] Be smarter in picking the memory op.

[Philip Reames]

reames added inline comments.

================
Comment at: lib/CodeGen/ImplicitNullChecks.cpp:212
@@ +211,3 @@
+
+  // Returns true if it is safe to reorder MI to before NotNullSucc.
+  auto IsSafeToReorder = [&](MachineInstr *MI) {
----------------
Naming wise, this is a bit confusing.  It's specifically whether the instruction is safe to reorder past the previously saved information.  At first, I had expected this predicate to by applied to each instruction skipped.

================
Comment at: lib/CodeGen/ImplicitNullChecks.cpp:255
@@ +254,3 @@
+
+    for (auto *MMO : MI->memoperands())
+      // Right now we don't want to worry about LLVM's memory model.
----------------
If one of these loads potentially overlap with the value we eventually load from, don't we end up with a faulting instruction not recorded in the side table?

(To say this differently, I think you need to account for aliasing in your loads.)


http://reviews.llvm.org/D11044