[PATCH] [CodeGen] Add a pass to fold null checks into nearby memory operations.

[Ahmed Bougacha]

================
Comment at: lib/CodeGen/ImplicitNullChecks.cpp:100-102
@@ +99,5 @@
+
+  void getAnalysisUsage(AnalysisUsage &AU) const override {
+    MachineFunctionPass::getAnalysisUsage(AU);
+  }
+
----------------
Is this necessary?

================
Comment at: lib/CodeGen/ImplicitNullChecks.cpp:134
@@ +133,3 @@
+
+  typedef TargetInstrInfo::MachineBranchPredicate MachineBranchPredicate;
+
----------------
How about 'using TargetInstrInfo::MachineBranchPredicate;'?

================
Comment at: lib/CodeGen/ImplicitNullChecks.cpp:192-195
@@ +191,6 @@
+    if (MemOp->mayLoad() && BaseReg == PointerReg && Offset < PageSize) {
+      // FIXME: we use MachineInstr::mayLoad which isn't quite correct for
+      // predicated load instructions that don't fault when the predicate is
+      // false.  We should add a instructionAlwaysLoads() hook to
+      // TargetInstrInfo and use that instead of mayLoad.
+      WorkList.emplace_back(MemOp, MBP.ConditionDef, &MBB, NotNullSucc,
----------------
Would (MayLoad && !Predicable) be (conservatively) correct?

================
Comment at: lib/CodeGen/ImplicitNullChecks.cpp:213
@@ +212,3 @@
+  unsigned NumDefs = LoadMI->getDesc().getNumDefs();
+  assert(NumDefs == 1 && "other cases unhandled!");
+
----------------
I'm not sure an assert is appropriate: I don't have an example but this sounds easy to trigger.  Should this be checked before even adding the NullCheck to the worklist?

================
Comment at: lib/CodeGen/ImplicitNullChecks.cpp:255
@@ +254,3 @@
+    TII->InsertBranch(*NC.CheckBlock, NC.NotNullSucc, nullptr,
+                      SmallVector<MachineOperand, 0>(), DL);
+
----------------
With r239553, you should be able to replace this with the IMO much more readable:

    /*Cond=*/None, DL);

http://reviews.llvm.org/D10201

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/