[PATCH] D29119: [ImplicitNullCheck] NFC isSuitableMemoryOp cleanup

[Serguei Katkov via Phabricator via llvm-commits]

skatkov updated this revision to Diff 86022.
skatkov edited the summary of this revision.
skatkov added a comment.

Please take a look.


https://reviews.llvm.org/D29119

Files:
  lib/CodeGen/ImplicitNullChecks.cpp


Index: lib/CodeGen/ImplicitNullChecks.cpp
===================================================================
--- lib/CodeGen/ImplicitNullChecks.cpp
+++ lib/CodeGen/ImplicitNullChecks.cpp
@@ -158,11 +158,17 @@
                                    MachineBasicBlock *HandlerMBB);
   void rewriteNullChecks(ArrayRef<NullCheck> NullCheckList);
 
+  enum SuitableResult {
+    SR_OK,
+    SR_Continue,
+    SR_Impossible
+  };
+
   /// Is \p MI a memory operation that can be used to implicitly null check the
   /// value in \p PointerReg?  \p PrevInsts is the set of instruction seen since
   /// the explicit null check on \p PointerReg.
-  bool isSuitableMemoryOp(MachineInstr &MI, unsigned PointerReg,
-                          ArrayRef<MachineInstr *> PrevInsts);
+  SuitableResult isSuitableMemoryOp(MachineInstr &MI, unsigned PointerReg,
+                                    ArrayRef<MachineInstr *> PrevInsts);
 
   /// Return true if \p FaultingMI can be hoisted from after the the
   /// instructions in \p InstsSeenSoFar to before them.  Set \p Dependence to a
@@ -283,30 +289,30 @@
   return false;
 }
 
-bool ImplicitNullChecks::isSuitableMemoryOp(
+ImplicitNullChecks::SuitableResult ImplicitNullChecks::isSuitableMemoryOp(
     MachineInstr &MI, unsigned PointerReg, ArrayRef<MachineInstr *> PrevInsts) {
   int64_t Offset;
   unsigned BaseReg;
 
   if (!TII->getMemOpBaseRegImmOfs(MI, BaseReg, Offset, TRI) ||
       BaseReg != PointerReg)
-    return false;
+    return SR_Continue;
 
   // We want the load to be issued at a sane offset from PointerReg, so that
   // if PointerReg is null then the load reliably page faults.
   if (!(MI.mayLoad() && !MI.isPredicable() && Offset < PageSize))
-    return false;
+    return SR_Continue;
 
   // Finally, we need to make sure that the load instruction actually is
   // loading from PointerReg, and there isn't some re-definition of PointerReg
   // between the compare and the load.
   for (auto *PrevMI : PrevInsts)
     for (auto &PrevMO : PrevMI->operands())
       if (PrevMO.isReg() && PrevMO.getReg() &&
           TRI->regsOverlap(PrevMO.getReg(), PointerReg))
-        return false;
+        return SR_Impossible;
 
-  return true;
+  return SR_OK;
 }
 
 bool ImplicitNullChecks::canHoistLoadInst(
@@ -481,7 +487,10 @@
       return false;
 
     MachineInstr *Dependence;
-    if (isSuitableMemoryOp(MI, PointerReg, InstsSeenSoFar) &&
+    SuitableResult SRResult = isSuitableMemoryOp(MI, PointerReg, InstsSeenSoFar);
+    if (SRResult == SR_Impossible)
+      return false;
+    if ((SRResult == SR_OK) &&
         canHoistLoadInst(&MI, PointerReg, InstsSeenSoFar, NullSucc,
                          Dependence)) {
       NullCheckList.emplace_back(&MI, MBP.ConditionDef, &MBB, NotNullSucc,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D29119.86022.patch
Type: text/x-patch
Size: 2767 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170127/e0beb759/attachment.bin>