[ImplicitNullCheck] Fix an edge case where we were hoisting incorrectly

ImplicitNullCheck keeps track of one instruction that the memory
operation depends on that it also hoists with the memory operation.
When hoisting this dependency, it would sometimes clobber a live-in
value to the basic block we were hoisting the two things out of.  Fix
this by explicitly looking for such dependencies.

I also noticed two redundant checks on `MO.isDef()` in IsMIOperandSafe.
They're redundant since register MachineOperands are either Defs or Uses
-- there is no third kind.  I'll change the checks to asserts in a later
commit.

llvm-svn: 287213

1 files changed, 19 insertions, 1 deletions

diff --git a/llvm/lib/CodeGen/ImplicitNullChecks.cpp b/llvm/lib/CodeGen/ImplicitNullChecks.cpp
index 6c35aaac7d2..31d65e69844 100644
--- a/llvm/lib/CodeGen/ImplicitNullChecks.cpp
+++ b/llvm/lib/CodeGen/ImplicitNullChecks.cpp
@@ -268,7 +268,25 @@ bool HazardDetector::isSafeToHoist(MachineInstr *MI,
             return false;
           assert((!MO.isDef() || RegDefs.count(MO.getReg())) &&
                  "All defs must be tracked in RegDefs by now!");
-          return !MO.isDef() || RegDefs.find(MO.getReg())->second == MI;
+
+          if (!MO.isDef()) {
+            // FIXME: This is unnecessary, we should be able to
+            // assert(MO.isDef()) here.
+            return true;
+          }
+
+          for (unsigned Reg : RegUses)
+            if (TRI.regsOverlap(Reg, MO.getReg()))
+              return false; // We found a write-after-read
+
+          for (auto &OtherDef : RegDefs) {
+            unsigned OtherReg = OtherDef.first;
+            MachineInstr *OtherMI = OtherDef.second;
+            if (OtherMI != MI && TRI.regsOverlap(OtherReg, MO.getReg()))
+              return false;
+          }
+
+          return true;
         };
 
         if (!all_of(MI->operands(), IsMIOperandSafe))