Subzero: Improve usability of liveness-related tools.

src/IceCfgNode.cpp

Index: src/IceCfgNode.cpp
diff --git a/src/IceCfgNode.cpp b/src/IceCfgNode.cpp
index dbbfc186a9c9ced499a864a8c20fced057de3faa..76629eddc72e3667c5cb0c24e89537ed01a8f787 100644
--- a/src/IceCfgNode.cpp
+++ b/src/IceCfgNode.cpp
@@ -698,16 +698,49 @@ void CfgNode::livenessAddIntervals(Liveness *Liveness, InstNumberT FirstInstNum,
   LiveBeginEndMap &MapEnd = *Liveness->getLiveEnd(this);
   std::sort(MapBegin.begin(), MapBegin.end());
   std::sort(MapEnd.begin(), MapEnd.end());
-  // Verify there are no duplicates.
-  auto ComparePair =
-      [](const LiveBeginEndMapEntry &A, const LiveBeginEndMapEntry &B) {
-        return A.first == B.first;
-      };
-  (void)ComparePair;
-  assert(std::adjacent_find(MapBegin.begin(), MapBegin.end(), ComparePair) ==
-         MapBegin.end());
-  assert(std::adjacent_find(MapEnd.begin(), MapEnd.end(), ComparePair) ==
-         MapEnd.end());
+  if (BuildDefs::asserts()) {

[John, 2015/09/25 23:02:16]

this would be easier to read (I think) if written as

if (!asserts())
  return;

failure = std::adjacent_find() != ..

if (!dump()) {
  assert(!failure);
  return;
}

if (!failure)
  return;

for (...)

It is easier to read if you don't have several nesting levels.

[Jim Stichnoth, 2015/09/25 23:56:01]

Done.

+    // Verify there are no duplicates.
+    auto ComparePair =
+        [](const LiveBeginEndMapEntry &A, const LiveBeginEndMapEntry &B) {
+          return A.first == B.first;
+        };
+    if (std::adjacent_find(MapBegin.begin(), MapBegin.end(), ComparePair) !=
+            MapBegin.end() ||
+        std::adjacent_find(MapEnd.begin(), MapEnd.end(), ComparePair) !=
+            MapEnd.end()) {
+      // There is definitely a liveness error.  Print all the errors.
+      if (BuildDefs::dump()) {
+        GlobalContext *Ctx = Func->getContext();
+        OstreamLocker L(Ctx);
+        Ostream &Str = Ctx->getStrDump();
+        if (Func->isVerbose()) {
+          Str << "Live range errors in the following block:\n";
+          dump(Func);
+        }
+        for (auto Start = MapBegin.begin();
+             (Start = std::adjacent_find(Start, MapBegin.end(), ComparePair)) !=
+             MapBegin.end();
+             ++Start) {
+          auto Next = Start + 1;
+          Str << "Duplicate LR begin, block " << getName() << ", instructions "
+              << Start->second << " & " << Next->second << ", variable "
+              << Liveness->getVariable(Start->first, this)->getName(Func)
+              << "\n";
+        }
+        for (auto Start = MapEnd.begin();
+             (Start = std::adjacent_find(Start, MapEnd.end(), ComparePair)) !=
+             MapEnd.end();
+             ++Start) {
+          auto Next = Start + 1;
+          Str << "Duplicate LR end,   block " << getName() << ", instructions "
+              << Start->second << " & " << Next->second << ", variable "
+              << Liveness->getVariable(Start->first, this)->getName(Func)
+              << "\n";
+        }
+      }
+      llvm::report_fatal_error("livenessAddIntervals: Liveness error");
+    }
+  }
 
   LivenessBV LiveInAndOut = LiveIn;
   LiveInAndOut &= LiveOut;
@@ -875,11 +908,11 @@ void emitLiveRangesEnded(Ostream &Str, const Cfg *Func, const Inst *Instr,
   bool First = true;
   Variable *Dest = Instr->getDest();
   // Normally we increment the live count for the dest register. But we
-  // shouldn't if the instruction's IsDestNonKillable flag is set, because this
+  // shouldn't if the instruction's IsDestRedefined flag is set, because this
   // means that the target lowering created this instruction as a non-SSA
   // assignment; i.e., a different, previous instruction started the dest
   // variable's live range.
-  if (!Instr->isDestNonKillable() && Dest && Dest->hasReg())
+  if (!Instr->isDestRedefined() && Dest && Dest->hasReg())
     ++LiveRegCount[Dest->getRegNum()];
   FOREACH_VAR_IN_INST(Var, *Instr) {
     bool ShouldReport = Instr->isLastUse(Var);