[PATCH] D78586: [MachineVerifier] Add more checks for registers in live-in lists.

[Eli Friedman via Phabricator via llvm-commits]

efriedma updated this revision to Diff 260393.
efriedma added a comment.

Address review comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78586/new/

https://reviews.llvm.org/D78586

Files:
  llvm/lib/CodeGen/MachineVerifier.cpp


Index: llvm/lib/CodeGen/MachineVerifier.cpp
===================================================================
--- llvm/lib/CodeGen/MachineVerifier.cpp
+++ llvm/lib/CodeGen/MachineVerifier.cpp
@@ -81,6 +81,13 @@
 
 using namespace llvm;
 
+// FIXME: This is a temporary flag while backends are fixed; we shouldn't
+// need to turn off this check.
+static cl::opt<bool> VerifierStrictLivein(
+    "machine-verifier-strict-livein", cl::Hidden,
+    cl::desc("Enable strict verification of basic block live-ins"),
+    cl::init(false));
+
 namespace {
 
   struct MachineVerifier {
@@ -2434,19 +2441,42 @@
   // that the register is in regsLiveOut of each predecessor block. Since
   // this must come from a definition in the predecesssor or its live-in
   // list, this will catch a live-through case where the predecessor does not
-  // have the register in its live-in list.  This currently only checks
-  // registers that have no aliases, are not allocatable and are not
-  // reserved, which could mean a condition code register for instance.
-  if (MRI->tracksLiveness())
-    for (const auto &MBB : *MF)
+  // have the register in its live-in list.
+  //
+  // We do a relatively relaxed check for registers with aliases: some
+  // register that aliases the register in question must be live-out of each
+  // predecessor.  In some cases, it might make sense to try to do something
+  // more strict.  But it's not clear what the rule would look like, and this
+  // is enough to catch a lot of interesting cases
+  if (MRI->tracksLiveness()) {
+    for (const auto &MBB : *MF) {
+      if (MBB.isEHPad())
+        continue;
       for (MachineBasicBlock::RegisterMaskPair P : MBB.liveins()) {
         MCPhysReg LiveInReg = P.PhysReg;
+
+        // We don't track liveness for reserved registers.
+        if (isReserved(LiveInReg))
+          continue;
+
+        // Checking of allocatable/aliased registers is controlled by a flag
+        // to deal with regression test failures.
         bool hasAliases = MCRegAliasIterator(LiveInReg, TRI, false).isValid();
-        if (hasAliases || isAllocatable(LiveInReg) || isReserved(LiveInReg))
+        if (!VerifierStrictLivein && (hasAliases || isAllocatable(LiveInReg)))
           continue;
+
         for (const MachineBasicBlock *Pred : MBB.predecessors()) {
           BBInfo &PInfo = MBBInfoMap[Pred];
-          if (!PInfo.regsLiveOut.count(LiveInReg)) {
+          MCRegAliasIterator AliasingRegs(LiveInReg, TRI, true);
+          bool FoundPred = false;
+          for (MCRegAliasIterator AliasingRegs(LiveInReg, TRI, true);
+               AliasingRegs.isValid(); ++AliasingRegs) {
+            if (PInfo.regsLiveOut.count(*AliasingRegs)) {
+              FoundPred = true;
+              break;
+            }
+          }
+          if (!FoundPred) {
             report("Live in register not found to be live out from predecessor.",
                    &MBB);
             errs() << TRI->getName(LiveInReg)
@@ -2455,6 +2485,8 @@
           }
         }
       }
+    }
+  }
 
   for (auto CSInfo : MF->getCallSitesInfo())
     if (!CSInfo.first->isCall())


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D78586.260393.patch
Type: text/x-patch
Size: 3129 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200427/5a531507/attachment.bin>