SelectionDAGDumper: Print simple operands inline.

[MatzeB]

Summary

Print simple operands inline instead of their pointer/value number.
Simple operands are things like Constant(FP)?, Register and undef,

Before:

  0x7fb6cb8ab800: v4f32 = Register %vreg0
0x7fb6cb8ab928: v4f32,ch = CopyFromReg 0x7fb6cb413770, 0x7fb6cb8ab800
0x7fb6cb8abb78: i64 = Constant<0>

0x7fb6cb8abca0: f32 = extract_vector_elt 0x7fb6cb8ab928, 0x7fb6cb8abb78

After:

0x7fb6cb8ab928 = CopyFromReg:v4f32,ch 0x7fb6cb413770, Register:v4f32  %vreg0
0x7fb6cb8abca0 = extract_vector_elt:f32 0x7fb6cb8ab928, Constant:i64 <0>

[arsenm]

lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp

591 ↗

(On Diff #33831)

VALUETYPE would another good one to handle

725 ↗

(On Diff #33831)

single quotes

[MatzeB]

Implement suggestions.

[MatzeB]

Actually added VALUETYPE this time.

[MatzeB]

New version: Use printOperand() in DumpNodesr() as well; simplify the definition of a simple operand as one that has no predecessors (with the exception of EntryToken which looks nicer separate).

[MatzeB]

ping

[resistor]

LGTM.

[MatzeB]

New version which does not depend on http://reviews.llvm.org/D12565 anymore.

For inlined operations this will still print the type behind the operation ("Constant:i32<42>") for toplevel operations the type is still printed on the lhs as before ("tXX: type = operation").

Note that this commit now moves the node details behind the node (like dumpr() already did). The previous behaviour of printing the details behind the operands would be confusing in combination with inlined operations (confusing example: "load t0, t2, undef:i64<LD1[%tmp81]>" as the <LD1...> is part of the load not the undef). We now print: "load<LD1[%tmp81]> t0, t2, undef:i64".

Longer example, before:

t0: ch = EntryToken
  t1: i64 = Register %vreg0
t2: i64,ch = CopyFromReg t0, t1
  t3: i64 = Constant<1>
t4: i64 = add t2, t3
  t5: i64 = Constant<2>
t6: i64 = add t2, t5
t10: i64 = undef
t11: i8,ch = load t0, t2, t10<LD1[%tmp81]>
t12: i8,ch = load t0, t4, t10<LD1[%tmp10]>
t13: i8,ch = load t0, t6, t10<LD1[%tmp12]>

Now:

t0: ch = EntryToken
t2: i64,ch = CopyFromReg t0, Register:i64 %vreg0
t4: i64 = add t2, Constant:i64<1>
t6: i64 = add t2, Constant:i64<2>
t11: i8,ch = load<LD1[%tmp81]> t0, t2, undef:i64
t12: i8,ch = load<LD1[%tmp10]> t0, t4, undef:i64
t13: i8,ch = load<LD1[%tmp12]> t0, t6, undef:i64

[MatzeB]

Is this still good to go even in the reworked form from monday?

[resistor]

Fine with me.

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

 
 void SDNode::dump() const { dump(nullptr); }
 void SDNode::dump(const SelectionDAG *G) const {
   print(dbgs(), G);
   dbgs() << '\n';
 }
 
 void SDNode::print_types(raw_ostream &OS, const SelectionDAG *G) const {
-  OS << PrintNodeId(*this) << ": ";
-
   for (unsigned i = 0, e = getNumValues(); i != e; ++i) {
     if (i) OS << ",";
     if (getValueType(i) == MVT::Other)
       OS << "ch";
     else
       OS << getValueType(i).getEVTString();
   }
-  OS << " = " << getOperationName(G);
 }
 
 void SDNode::print_details(raw_ostream &OS, const SelectionDAG *G) const {
   if (const MachineSDNode *MN = dyn_cast<MachineSDNode>(this)) {
     if (!MN->memoperands_empty()) {
       OS << "<";
       OS << "Mem:";
       for (MachineSDNode::mmo_iterator i = MN->memoperands_begin(),
if (VerboseDAGDumping) {
     else
       OS << "<unknown>";
     OS << ':' << L->getLine();
     if (unsigned C = L->getColumn())
       OS << ':' << C;
   }
 }
 
+/// 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) {
+  if (Node.getOpcode() == ISD::EntryToken)
+    return false;
+  return Node.getNumOperands() == 0;
+}
+
 static void DumpNodes(const SDNode *N, unsigned indent, const SelectionDAG *G) {
-  for (const SDValue &Op : N->op_values())
+  for (const SDValue &Op : N->op_values()) {
+    if (shouldPrintInline(*Op.getNode()))
+      continue;
     if (Op.getNode()->hasOneUse())
       DumpNodes(Op.getNode(), indent+2, G);
+  }
 
   dbgs().indent(indent);
   N->dump(G);
 }
 
 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())
+    if (!N->hasOneUse() && N != getRoot().getNode() &&
+        (!shouldPrintInline(*N) || N->use_empty()))
       DumpNodes(N, 2, this);
   }
 
   if (getRoot().getNode()) DumpNodes(getRoot().getNode(), 2, this);
   dbgs() << "\n\n";
 }
 
 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 (shouldPrintInline(*Value.getNode())) {
+    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;
+    return false;
+  }
+}
+
 typedef SmallPtrSet<const SDNode *, 128> VisitedSDNodeSet;
 static void DumpNodesr(raw_ostream &OS, const SDNode *N, unsigned indent,
                        const SelectionDAG *G, VisitedSDNodeSet &once) {
   if (!once.insert(N).second) // If we've been here before, return now.
     return;
 
   // Dump the current SDNode, but don't end the line yet.
   OS.indent(indent);
   N->printr(OS, G);
 
   // Having printed this SDNode, walk the children:
   for (unsigned i = 0, e = N->getNumOperands(); i != e; ++i) {
-    const SDNode *child = N->getOperand(i).getNode();
-
     if (i) OS << ",";
     OS << " ";
 
-    if (child->getNumOperands() == 0) {
-      // This child has no grandchildren; print it inline right here.
-      child->printr(OS, G);
-      once.insert(child);
-    } else {         // Just the address. FIXME: also print the child's opcode.
-      OS << (const void*)child;
-      if (unsigned RN = N->getOperand(i).getResNo())
-        OS << ":" << RN;
-    }
+    const SDValue Op = N->getOperand(i);
+    bool printedInline = printOperand(OS, G, Op);
+    if (printedInline)
+      once.insert(Op.getNode());
   }
 
   OS << "\n";
 
   // Dump children that have grandchildren on their own line(s).
   for (const SDValue &Op : N->op_values())
     DumpNodesr(OS, Op.getNode(), indent+2, G, once);
 }

 }
 
 void SDNode::dumprFull(const SelectionDAG *G) const {
   // Don't print impossibly deep things.
   dumprWithDepth(G, 10);
 }
 
 void SDNode::print(raw_ostream &OS, const SelectionDAG *G) const {
-  print_types(OS, G);
+  printr(OS, G);
   for (unsigned i = 0, e = getNumOperands(); i != e; ++i) {
     if (i) OS << ", "; else OS << " ";
-    const SDValue Operand = getOperand(i);
-    OS << PrintNodeId(*Operand.getNode());
-    if (unsigned RN = Operand.getResNo())
-      OS << ":" << RN;
+    printOperand(OS, G, getOperand(i));
   }
-  print_details(OS, G);
 }

llvm/trunk/test/CodeGen/X86/merge-store-partially-alias-loads.ll

 ; X86-DAG: movb 2([[BASEREG]]), [[HI1:%[a-z]+]]
 
 ; X86-NEXT: movw [[LO2]], 1([[BASEREG]])
 ; X86-NEXT: movb [[HI1]], 3([[BASEREG]])
 ; X86-NEXT: retq
 
 ; DBGDAG-LABEL: Optimized lowered selection DAG: BB#0 'merge_store_partial_overlap_load:'
 ; DBGDAG: [[ENTRYTOKEN:t[0-9]+]]: ch = EntryToken
-; DBGDAG-DAG: [[TWO:t[0-9]+]]: i64 = Constant<2>
 ; DBGDAG-DAG: [[BASEPTR:t[0-9]+]]: i64,ch = CopyFromReg [[ENTRYTOKEN]],
-; DBGDAG-DAG: [[ADDPTR:t[0-9]+]]: i64 = add [[BASEPTR]], [[TWO]]
+; DBGDAG-DAG: [[ADDPTR:t[0-9]+]]: i64 = add [[BASEPTR]], Constant:i64<2>
 
-; DBGDAG-DAG: [[LD2:t[0-9]+]]: i16,ch = load [[ENTRYTOKEN]], [[BASEPTR]], t{{[0-9]+}}<LD2[%tmp81](align=1)>
-; DBGDAG-DAG: [[LD1:t[0-9]+]]: i8,ch = load [[ENTRYTOKEN]], [[ADDPTR]], t{{[0-9]+}}<LD1[%tmp12]>
+; DBGDAG-DAG: [[LD2:t[0-9]+]]: i16,ch = load<LD2[%tmp81](align=1)> [[ENTRYTOKEN]], [[BASEPTR]], undef:i64
+; DBGDAG-DAG: [[LD1:t[0-9]+]]: i8,ch = load<LD1[%tmp12]> [[ENTRYTOKEN]], [[ADDPTR]], undef:i64
 
 ; DBGDAG: [[LOADTOKEN:t[0-9]+]]: ch = TokenFactor [[LD2]]:1, [[LD1]]:1
 
-; DBGDAG-DAG: [[ST2:t[0-9]+]]: ch = store [[LOADTOKEN]], [[LD2]], t{{[0-9]+}}, t{{[0-9]+}}<ST2[%tmp10](align=1)>
-; DBGDAG-DAG: [[ST1:t[0-9]+]]: ch = store [[ST2]], [[LD1]], t{{[0-9]+}}, t{{[0-9]+}}<ST1[%tmp14]>
+; DBGDAG-DAG: [[ST2:t[0-9]+]]: ch = store<ST2[%tmp10](align=1)> [[LOADTOKEN]], [[LD2]], t{{[0-9]+}}, undef:i64
+; DBGDAG-DAG: [[ST1:t[0-9]+]]: ch = store<ST1[%tmp14]> [[ST2]], [[LD1]], t{{[0-9]+}}, undef:i64
 ; DBGDAG: X86ISD::RET_FLAG [[ST1]],
 
 ; DBGDAG: Type-legalized selection DAG: BB#0 'merge_store_partial_overlap_load:'
 define void @merge_store_partial_overlap_load([4 x i8]* %tmp) {
   %tmp8 = getelementptr inbounds [4 x i8], [4 x i8]* %tmp, i32 0, i8 0
   %tmp10 = getelementptr inbounds [4 x i8], [4 x i8]* %tmp, i32 0, i8 1
   %tmp12 = getelementptr inbounds [4 x i8], [4 x i8]* %tmp, i32 0, i8 2
   %tmp14 = getelementptr [4 x i8], [4 x i8]* %tmp, i32 0, i8 3