[llvm] 425573a - [ImplicitNullChecks] NFC: Refactor dependence safety check

[Anna Thomas via llvm-commits]

Author: Anna Thomas
Date: 2020-09-02T10:29:44-04:00
New Revision: 425573a2fa2dc5666273944a584acdb286447b66

URL: https://github.com/llvm/llvm-project/commit/425573a2fa2dc5666273944a584acdb286447b66
DIFF: https://github.com/llvm/llvm-project/commit/425573a2fa2dc5666273944a584acdb286447b66.diff

LOG: [ImplicitNullChecks] NFC: Refactor dependence safety check

After computing dependence, we check if it is safe to hoist by
identifying if it clobbers any liveIns in the sibling block (NullSucc).
This check is moved to its own function which will be used in the
soon-to-be modified dependence checking algorithm for implicit null
checks pass.

Tests-Run: lit tests on X86/implicit-*

Added: 
    

Modified: 
    llvm/lib/CodeGen/ImplicitNullChecks.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/ImplicitNullChecks.cpp b/llvm/lib/CodeGen/ImplicitNullChecks.cpp
index c6b1eeb3408c..dc1b0a867b0d 100644
--- a/llvm/lib/CodeGen/ImplicitNullChecks.cpp
+++ b/llvm/lib/CodeGen/ImplicitNullChecks.cpp
@@ -200,6 +200,13 @@ class ImplicitNullChecks : public MachineFunctionPass {
                                        unsigned PointerReg,
                                        ArrayRef<MachineInstr *> PrevInsts);
 
+  /// Returns true if \p DependenceMI can clobber the liveIns in NullSucc block
+  /// if it was hoisted to the NullCheck block. This is used by caller
+  /// canHoistInst to decide if DependenceMI can be hoisted safely.
+  bool canDependenceHoistingClobberLiveIns(MachineInstr *DependenceMI,
+                                           MachineBasicBlock *NullSucc,
+                                           unsigned PointerReg);
+
   /// Return true if \p FaultingMI can be hoisted from after the
   /// instructions in \p InstsSeenSoFar to before them.  Set \p Dependence to a
   /// non-null value if we also need to (and legally can) hoist a depedency.
@@ -401,32 +408,9 @@ ImplicitNullChecks::isSuitableMemoryOp(const MachineInstr &MI,
   return SR_Suitable;
 }
 
-bool ImplicitNullChecks::canHoistInst(MachineInstr *FaultingMI,
-                                      unsigned PointerReg,
-                                      ArrayRef<MachineInstr *> InstsSeenSoFar,
-                                      MachineBasicBlock *NullSucc,
-                                      MachineInstr *&Dependence) {
-  auto DepResult = computeDependence(FaultingMI, InstsSeenSoFar);
-  if (!DepResult.CanReorder)
-    return false;
-
-  if (!DepResult.PotentialDependence) {
-    Dependence = nullptr;
-    return true;
-  }
-
-  auto DependenceItr = *DepResult.PotentialDependence;
-  auto *DependenceMI = *DependenceItr;
-
-  // We don't want to reason about speculating loads.  Note -- at this point
-  // we should have already filtered out all of the other non-speculatable
-  // things, like calls and stores.
-  // We also do not want to hoist stores because it might change the memory
-  // while the FaultingMI may result in faulting.
-  assert(canHandle(DependenceMI) && "Should never have reached here!");
-  if (DependenceMI->mayLoadOrStore())
-    return false;
-
+bool ImplicitNullChecks::canDependenceHoistingClobberLiveIns(
+    MachineInstr *DependenceMI, MachineBasicBlock *NullSucc,
+    unsigned PointerReg) {
   for (auto &DependenceMO : DependenceMI->operands()) {
     if (!(DependenceMO.isReg() && DependenceMO.getReg()))
       continue;
@@ -449,7 +433,7 @@ bool ImplicitNullChecks::canHoistInst(MachineInstr *FaultingMI,
     // same as it would have been had the load not have executed and we'd have
     // branched to NullSucc directly.
     if (AnyAliasLiveIn(TRI, NullSucc, DependenceMO.getReg()))
-      return false;
+      return true;
 
     // The Dependency can't be re-defining the base register -- then we won't
     // get the memory operation on the address we want.  This is already
@@ -459,6 +443,39 @@ bool ImplicitNullChecks::canHoistInst(MachineInstr *FaultingMI,
            "Should have been checked before!");
   }
 
+  // The dependence does not clobber live-ins in NullSucc block.
+  return false;
+}
+
+bool ImplicitNullChecks::canHoistInst(MachineInstr *FaultingMI,
+                                      unsigned PointerReg,
+                                      ArrayRef<MachineInstr *> InstsSeenSoFar,
+                                      MachineBasicBlock *NullSucc,
+                                      MachineInstr *&Dependence) {
+  auto DepResult = computeDependence(FaultingMI, InstsSeenSoFar);
+  if (!DepResult.CanReorder)
+    return false;
+
+  if (!DepResult.PotentialDependence) {
+    Dependence = nullptr;
+    return true;
+  }
+
+  auto DependenceItr = *DepResult.PotentialDependence;
+  auto *DependenceMI = *DependenceItr;
+
+  // We don't want to reason about speculating loads.  Note -- at this point
+  // we should have already filtered out all of the other non-speculatable
+  // things, like calls and stores.
+  // We also do not want to hoist stores because it might change the memory
+  // while the FaultingMI may result in faulting.
+  assert(canHandle(DependenceMI) && "Should never have reached here!");
+  if (DependenceMI->mayLoadOrStore())
+    return false;
+
+  if (canDependenceHoistingClobberLiveIns(DependenceMI, NullSucc, PointerReg))
+    return false;
+
   auto DepDepResult =
       computeDependence(DependenceMI, {InstsSeenSoFar.begin(), DependenceItr});