[SelectionDAG] Updates for -dag-dump-verbose

[bjope]

Summary

This patch makes some changes related to -dag-dump-verbose.
Main use case has been when debugging how SelectionDAG is
dealing with debug info (SDDbgValue nodes).

1. We now print the number of DbgValues that are mapped to each SDNode.
2. Removed duplicated printing of DebugLoc (nowadays DebugLoc is printed also when not using -dag-dump-verbose).
3. Renamed SDDbgValue::dump to SDDbgValue::print, and added a new SDDbgValue::dump that will start a new line after calling print.
4. SDDbgValue::print now prints "Order", and it also prints some additional information when kind is CONST/FRAMEIX/VREG.
5. SelectionDAG::dump() now dumps all SDDbgValue nodes after the list of SDNodes (both "regular" and "ByVal" SDDbgValue:s). Invalidated nodes are not printed.
6. Prohibit inline printing of SDNode operands that has SDDbgValue nodes associated to them.

[bjope]

@jmorse : Since you are working a little bit with the SelectionDAGBuilder and debug values etc, maybe you will find this patch helpful. It gives some more printouts for the "missing" DBG_VALUE nodes that normally isn't shown during ISel simply by using -debug or -dag-dump-verbose.

Before submitting this we need to decide upon a few things, such as:

1. should this be a separate option or could it simply be part of -dag-dump-verbose
2. should we convert -dag-dump-verbose into some multi-choice param with different verbosity levels

Maybe (1) is an acceptable solution.

Not sure about what kind of test cases that are needed for these. I haven't seen any test cases for -dag-dump-verbose so maybe it isn't that important to have tests for this kind of debugging aid?

[jmorse]

Hi Bjorn, this is highly useful and relevant to my interests,

In D56793#1360037, @bjope wrote:

Before submitting this we need to decide upon a few things, such as:

1. should this be a separate option or could it simply be part of -dag-dump-verbose
2. should we convert -dag-dump-verbose into some multi-choice param with different verbosity levels

IMHO this can be rolled into -dag-dump-verbose: for the normal use cases the user won't have any dbg.values, or should be able to trivially drop them if they don't want this information cluttering output.

Not sure about what kind of test cases that are needed for these. I haven't seen any test cases for -dag-dump-verbose so maybe it isn't that important to have tests for this kind of debugging aid?

I suspect no test is required (although I doubt it's written down anywhere). It'd be awkward to break someones CI testing because a non-functional textual representation changed. I've also seen a couple of reviews disallowing the use of -debug output to test the compiler behaviour.

include/llvm/CodeGen/SelectionDAG.h

1355–1357 ↗

(On Diff #182073)

Doesn't compile for me, the other GetDbgValues method is already const, so this only overloads the return type which C++ disallows.

1364–1367 ↗

(On Diff #182073)

Is it worth just removing the non-const DbgBegin/End here, as we return the same iterators anyway?

[bjope]

include/llvm/CodeGen/SelectionDAG.h

1355–1357 ↗

(On Diff #182073)

Ok. Sorry. I resurrected this from some old branch and forgot to verify that it works on latest trunk.
But since it seems useful I'll cleanup this a little bit (and of course making sure it compiles again).

[bjope]

Now refactored based on trunk (the first version I uploaded was a
quite old patch that didn't even compile, sorry for that).

I also removed the part about letting SelectionDAG::dump() dump
all SDNodes (even the single-use nodes normally printed inline)
on separate lines. I've always found it a little bit confusing
that it says "SelectionDAG has 22 nodes:" and then it prints
what seems to be less than 22 nodes, due to some being printed
inline (and the node id is not printed for the inlined ones).
I'm thinking about adding a new option for disabling the
inline printing, but that will go in a separate patch (it is
not really related to -dag-dump-verbose).

[bjope]

Some more updates (some tweaks after having looked at some logs).

llvm/trunk/include/llvm/CodeGen/SelectionDAG.h

ArrayRef<SDDbgValue*> GetDbgValues(const SDNode* SD) const {
     return DbgInfo->getSDDbgValues(SD);
   }
 
 public:
   /// Return true if there are any SDDbgValue nodes associated
   /// with this SelectionDAG.
   bool hasDebugValues() const { return !DbgInfo->empty(); }
 
-  SDDbgInfo::DbgIterator DbgBegin() { return DbgInfo->DbgBegin(); }
-  SDDbgInfo::DbgIterator DbgEnd()   { return DbgInfo->DbgEnd(); }
+  SDDbgInfo::DbgIterator DbgBegin() const { return DbgInfo->DbgBegin(); }
+  SDDbgInfo::DbgIterator DbgEnd() const  { return DbgInfo->DbgEnd(); }
 
-  SDDbgInfo::DbgIterator ByvalParmDbgBegin() {
+  SDDbgInfo::DbgIterator ByvalParmDbgBegin() const {
     return DbgInfo->ByvalParmDbgBegin();
   }
-
-  SDDbgInfo::DbgIterator ByvalParmDbgEnd()   {
+  SDDbgInfo::DbgIterator ByvalParmDbgEnd() const {
     return DbgInfo->ByvalParmDbgEnd();
   }
 
-  SDDbgInfo::DbgLabelIterator DbgLabelBegin() {
+  SDDbgInfo::DbgLabelIterator DbgLabelBegin() const {
     return DbgInfo->DbgLabelBegin();
   }
-  SDDbgInfo::DbgLabelIterator DbgLabelEnd() {
+  SDDbgInfo::DbgLabelIterator DbgLabelEnd() const {
     return DbgInfo->DbgLabelEnd();
   }
 
   /// To be invoked on an SDNode that is slated to be erased. This
   /// function mirrors \c llvm::salvageDebugInfo.
   void salvageDebugInfo(SDNode &N);
 
   void dump() const;

llvm/trunk/lib/CodeGen/SelectionDAG/SDNodeDbgValue.h

public:
   /// SDDbgValue has been emitted to an MBB.
   void setIsEmitted() { Emitted = true; }
   bool isEmitted() const { return Emitted; }
 
   /// clearIsEmitted - Reset Emitted flag, for certain special cases where
   /// dbg.addr is emitted twice.
   void clearIsEmitted() { Emitted = false; }
 
-  LLVM_DUMP_METHOD void dump(raw_ostream &OS) const;
+  LLVM_DUMP_METHOD void dump() const;
+  LLVM_DUMP_METHOD void print(raw_ostream &OS) const;
 };
 
 /// Holds the information from a dbg_label node through SDISel.
 /// We do not use SDValue here to avoid including its header.
 class SDDbgLabel {
   MDNode *Label;
   DebugLoc DL;
   unsigned Order;

llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp

#endif
   case ISD::BlockAddress:               return "BlockAddress";
   case ISD::INTRINSIC_WO_CHAIN:
   case ISD::INTRINSIC_VOID:
   case ISD::INTRINSIC_W_CHAIN: {
     unsigned OpNo = getOpcode() == ISD::INTRINSIC_WO_CHAIN ? 0 : 1;
     unsigned IID = cast<ConstantSDNode>(getOperand(OpNo))->getZExtValue();
     if (IID < Intrinsic::num_intrinsics)
       return Intrinsic::getName((Intrinsic::ID)IID, None);
+    else if (!G)
+      return "Unknown intrinsic";
     else if (const TargetIntrinsicInfo *TII = G->getTarget().getIntrinsicInfo())
       return TII->getName(IID);
     llvm_unreachable("Invalid intrinsic ID");
   }
 
   case ISD::BUILD_VECTOR:               return "BUILD_VECTOR";
   case ISD::TargetConstant:
     if (cast<ConstantSDNode>(this)->isOpaque())
void SDNode::print_details(raw_ostream &OS, const SelectionDAG *G) const {
 
   if (VerboseDAGDumping) {
     if (unsigned Order = getIROrder())
         OS << " [ORD=" << Order << ']';
 
     if (getNodeId() != -1)
       OS << " [ID=" << getNodeId() << ']';
     if (!(isa<ConstantSDNode>(this) || (isa<ConstantFPSDNode>(this))))
-      OS << "# D:" << isDivergent();
+      OS << " # D:" << isDivergent();
 
-    if (!G)
-      return;
-
-    DILocation *L = getDebugLoc();
-    if (!L)
-      return;
-
-    if (auto *Scope = L->getScope())
-      OS << Scope->getFilename();
-    else
-      OS << "<unknown>";
-    OS << ':' << L->getLine();
-    if (unsigned C = L->getColumn())
-      OS << ':' << C;
-
-    for (SDDbgValue *Dbg : G->GetDbgValues(this)) {
-      if (Dbg->getKind() != SDDbgValue::SDNODE || Dbg->isInvalidated())
-        continue;
-      Dbg->dump(OS);
-    }
+    if (G && !G->GetDbgValues(this).empty()) {
+      OS << " [NoOfDbgValues=" << G->GetDbgValues(this).size() << ']';
+      for (SDDbgValue *Dbg : G->GetDbgValues(this))
+        if (!Dbg->isInvalidated())
+          Dbg->print(OS);
+    } else if (getHasDebugValue())
+      OS << " [NoOfDbgValues>0]";
   }
 }
 
-LLVM_DUMP_METHOD void SDDbgValue::dump(raw_ostream &OS) const {
- OS << " DbgVal";
- if (kind==SDNODE)
-   OS << '(' << u.s.ResNo << ')';
+LLVM_DUMP_METHOD void SDDbgValue::print(raw_ostream &OS) const {
+  OS << " DbgVal(Order=" << getOrder() << ')';
+  if (isInvalidated()) OS << "(Invalidated)";
+  if (isEmitted()) OS << "(Emitted)";
+  switch (getKind()) {
+  case SDNODE:
+    if (getSDNode())
+      OS << "(SDNODE=" << PrintNodeId(*getSDNode()) << ':' <<  getResNo() << ')';
+    else
+      OS << "(SDNODE)";
+    break;
+  case CONST:
+    OS << "(CONST)";
+    break;
+  case FRAMEIX:
+    OS << "(FRAMEIX=" << getFrameIx() << ')';
+    break;
+  case VREG:
+    OS << "(VREG=" << getVReg() << ')';
+    break;
+  }
+  if (isIndirect()) OS << "(Indirect)";
   OS << ":\"" << Var->getName() << '"';
 #ifndef NDEBUG
   if (Expr->getNumElements())
     Expr->dump();
 #endif
 }
 
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+LLVM_DUMP_METHOD void SDDbgValue::dump() const {
+  if (isInvalidated())
+    return;
+  print(dbgs());
+  dbgs() << "\n";
+}
+#endif
+
 /// Return true if this node is so simple that we should just print it inline
 /// if it appears as an operand.
-static bool shouldPrintInline(const SDNode &Node) {
+static bool shouldPrintInline(const SDNode &Node, const SelectionDAG *G) {
+  // Avoid lots of cluttering when inline printing nodes with associated
+  // DbgValues in verbose mode.
+  if (VerboseDAGDumping && G && !G->GetDbgValues(&Node).empty())
+    return false;
   if (Node.getOpcode() == ISD::EntryToken)
     return false;
   return Node.getNumOperands() == 0;
 }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 static void DumpNodes(const SDNode *N, unsigned indent, const SelectionDAG *G) {
   for (const SDValue &Op : N->op_values()) {
-    if (shouldPrintInline(*Op.getNode()))
+    if (shouldPrintInline(*Op.getNode(), G))
       continue;
     if (Op.getNode()->hasOneUse())
       DumpNodes(Op.getNode(), indent+2, G);
   }
 
   dbgs().indent(indent);
   N->dump(G);
 }
 
 LLVM_DUMP_METHOD void SelectionDAG::dump() const {
   dbgs() << "SelectionDAG has " << AllNodes.size() << " nodes:\n";
 
   for (allnodes_const_iterator I = allnodes_begin(), E = allnodes_end();
        I != E; ++I) {
     const SDNode *N = &*I;
     if (!N->hasOneUse() && N != getRoot().getNode() &&
-        (!shouldPrintInline(*N) || N->use_empty()))
+        (!shouldPrintInline(*N, this) || N->use_empty()))
       DumpNodes(N, 2, this);
   }
 
   if (getRoot().getNode()) DumpNodes(getRoot().getNode(), 2, this);
-  dbgs() << "\n\n";
+  dbgs() << "\n";
+
+  if (VerboseDAGDumping) {
+    if (DbgBegin() != DbgEnd())
+      dbgs() << "SDDbgValues:\n";
+    for (auto *Dbg : make_range(DbgBegin(), DbgEnd()))
+      Dbg->dump();
+    if (ByvalParmDbgBegin() != ByvalParmDbgEnd())
+      dbgs() << "Byval SDDbgValues:\n";
+    for (auto *Dbg : make_range(ByvalParmDbgBegin(), ByvalParmDbgEnd()))
+      Dbg->dump();
+  }
+  dbgs() << "\n";
 }
 #endif
 
 void SDNode::printr(raw_ostream &OS, const SelectionDAG *G) const {
   OS << PrintNodeId(*this) << ": ";
   print_types(OS, G);
   OS << " = " << getOperationName(G);
   print_details(OS, G);
 }
 
 static bool printOperand(raw_ostream &OS, const SelectionDAG *G,
                          const SDValue Value) {
   if (!Value.getNode()) {
     OS << "<null>";
     return false;
-  } else if (shouldPrintInline(*Value.getNode())) {
+  } else if (shouldPrintInline(*Value.getNode(), G)) {
     OS << Value->getOperationName(G) << ':';
     Value->print_types(OS, G);
     Value->print_details(OS, G);
     return true;
   } else {
     OS << PrintNodeId(*Value.getNode());
     if (unsigned RN = Value.getResNo())
       OS << ':' << RN;