[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-*
diff --git a/llvm/lib/CodeGen/ImplicitNullChecks.cpp b/llvm/lib/CodeGen/ImplicitNullChecks.cpp
index c6b1eeb..dc1b0a8 100644
--- a/llvm/lib/CodeGen/ImplicitNullChecks.cpp
+++ b/llvm/lib/CodeGen/ImplicitNullChecks.cpp
@@ -200,6 +200,13 @@
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,6 +408,45 @@
return SR_Suitable;
}
+bool ImplicitNullChecks::canDependenceHoistingClobberLiveIns(
+ MachineInstr *DependenceMI, MachineBasicBlock *NullSucc,
+ unsigned PointerReg) {
+ for (auto &DependenceMO : DependenceMI->operands()) {
+ if (!(DependenceMO.isReg() && DependenceMO.getReg()))
+ continue;
+
+ // Make sure that we won't clobber any live ins to the sibling block by
+ // hoisting Dependency. For instance, we can't hoist INST to before the
+ // null check (even if it safe, and does not violate any dependencies in
+ // the non_null_block) if %rdx is live in to _null_block.
+ //
+ // test %rcx, %rcx
+ // je _null_block
+ // _non_null_block:
+ // %rdx = INST
+ // ...
+ //
+ // This restriction does not apply to the faulting load inst because in
+ // case the pointer loaded from is in the null page, the load will not
+ // semantically execute, and affect machine state. That is, if the load
+ // was loading into %rax and it faults, the value of %rax should stay the
+ // 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 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
+ // checked in \c IsSuitableMemoryOp.
+ assert(!(DependenceMO.isDef() &&
+ TRI->regsOverlap(DependenceMO.getReg(), PointerReg)) &&
+ "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,
@@ -427,37 +473,8 @@
if (DependenceMI->mayLoadOrStore())
return false;
- for (auto &DependenceMO : DependenceMI->operands()) {
- if (!(DependenceMO.isReg() && DependenceMO.getReg()))
- continue;
-
- // Make sure that we won't clobber any live ins to the sibling block by
- // hoisting Dependency. For instance, we can't hoist INST to before the
- // null check (even if it safe, and does not violate any dependencies in
- // the non_null_block) if %rdx is live in to _null_block.
- //
- // test %rcx, %rcx
- // je _null_block
- // _non_null_block:
- // %rdx = INST
- // ...
- //
- // This restriction does not apply to the faulting load inst because in
- // case the pointer loaded from is in the null page, the load will not
- // semantically execute, and affect machine state. That is, if the load
- // was loading into %rax and it faults, the value of %rax should stay the
- // 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;
-
- // 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
- // checked in \c IsSuitableMemoryOp.
- assert(!(DependenceMO.isDef() &&
- TRI->regsOverlap(DependenceMO.getReg(), PointerReg)) &&
- "Should have been checked before!");
- }
+ if (canDependenceHoistingClobberLiveIns(DependenceMI, NullSucc, PointerReg))
+ return false;
auto DepDepResult =
computeDependence(DependenceMI, {InstsSeenSoFar.begin(), DependenceItr});