Improve flow graph DOT dump: conditional display, block ID (#52489)

1. For conditional blocks, display a small summary of the branch taken path
of the block (namely, the JTRUE condition). This is currently very limited,
but could be expanded to additional cases as it proves useful (but it needs
to be very compact; you're expected to consult the JitDump for more details).
2. Optionally display both the bbNum and bbID as the block label. This makes
it easier to see which blocks remained the same through renames if comparing
flow graph displays from different phases. This is enabled by setting
`COMPlus_JitDumpFgBlockID=1`.

diff --git a/src/coreclr/jit/block.cpp b/src/coreclr/jit/block.cpp
index c5eba8f..1c132ab 100644 (file)
--- a/src/coreclr/jit/block.cpp
+++ b/src/coreclr/jit/block.cpp
@@ -702,7 +702,7 @@ void BasicBlock::dspBlockHeader(Compiler* compiler,
     printf("\n");
 }
 
-const char* BasicBlock::dspToString(int blockNumPadding /* = 2*/)
+const char* BasicBlock::dspToString(int blockNumPadding /* = 0 */)
 {
     static char buffers[3][64]; // static array of 3 to allow 3 concurrent calls in one printf()
     static int  nextBufferIndex = 0;

diff --git a/src/coreclr/jit/block.h b/src/coreclr/jit/block.h
index 5c41dcd..6f267e0 100644 (file)
--- a/src/coreclr/jit/block.h
+++ b/src/coreclr/jit/block.h
@@ -1032,6 +1032,8 @@ struct BasicBlock : private LIR::Range
     // in the BB list with that stamp (in this field); then we can tell if (e.g.) predecessors are
     // still in the BB list by whether they have the same stamp (with high probability).
     unsigned bbTraversalStamp;
+
+    // bbID is a unique block identifier number that does not change: it does not get renumbered, like bbNum.
     unsigned bbID;
 #endif // DEBUG
 

diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp
index 864d363..3aefdcb 100644 (file)
--- a/src/coreclr/jit/fgdiagnostic.cpp
+++ b/src/coreclr/jit/fgdiagnostic.cpp
@@ -609,6 +609,7 @@ FILE* Compiler::fgOpenFlowGraphFile(bool* wbDontClose, Phases phase, LPCWSTR typ
 //      COMPlus_JitDumpFgLoops         (dot only) 0 for no loop information; non-zero to include loop regions.
 //      COMPlus_JitDumpFgConstrained   (dot only) 0 == don't constrain to mostly linear layout; non-zero == force
 //                                     mostly lexical block linear layout.
+//      COMPlus_JitDumpFgBlockId       Display blocks with block ID, not just bbNum.
 //
 bool Compiler::fgDumpFlowGraph(Phases phase)
 {
@@ -623,11 +624,13 @@ bool Compiler::fgDumpFlowGraph(Phases phase)
     // avoid asserts.
     const bool includeLoops = (JitConfig.JitDumpFgLoops() != 0) && !compIsForInlining() && (phase < PHASE_RATIONALIZE);
     const bool constrained  = JitConfig.JitDumpFgConstrained() != 0;
+    const bool useBlockId   = JitConfig.JitDumpFgBlockID() != 0;
 #else  // !DEBUG
     const bool createDotFile = true;
     const bool includeEH     = false;
     const bool includeLoops  = false;
     const bool constrained   = true;
+    const bool useBlockId    = false;
 #endif // !DEBUG
 
     FILE* fgxFile = fgOpenFlowGraphFile(&dontClose, phase, createDotFile ? W("dot") : W("fgx"));
@@ -741,7 +744,94 @@ bool Compiler::fgDumpFlowGraph(Phases phase)
     {
         if (createDotFile)
         {
-            fprintf(fgxFile, "    " FMT_BB " [label = \"" FMT_BB, block->bbNum, block->bbNum);
+            fprintf(fgxFile, "    " FMT_BB " [label = \"", block->bbNum);
+
+            if (useBlockId)
+            {
+                fprintf(fgxFile, "%s", block->dspToString());
+            }
+            else
+            {
+                fprintf(fgxFile, FMT_BB, block->bbNum);
+            }
+
+            if (block->bbJumpKind == BBJ_COND)
+            {
+                fprintf(fgxFile, "\\n");
+
+                // Include a line with the basics of the branch condition, if possible.
+                // Find the loop termination test at the bottom of the loop.
+                Statement* condStmt = block->lastStmt();
+                if (condStmt != nullptr)
+                {
+                    GenTree* const condTree = condStmt->GetRootNode();
+                    noway_assert(condTree->gtOper == GT_JTRUE);
+                    GenTree* const compareTree = condTree->AsOp()->gtOp1;
+                    if (compareTree->OperIsCompare())
+                    {
+                        // Want to generate something like:
+                        //   V01 <= 7
+                        //   V01 > V02
+
+                        const char* opName = GenTree::OpName(compareTree->OperGet());
+                        // Make it look nicer if we can
+                        switch (compareTree->OperGet())
+                        {
+                            case GT_EQ:
+                                opName = "==";
+                                break;
+                            case GT_NE:
+                                opName = "!=";
+                                break;
+                            case GT_LT:
+                                opName = "<";
+                                break;
+                            case GT_LE:
+                                opName = "<=";
+                                break;
+                            case GT_GE:
+                                opName = ">=";
+                                break;
+                            case GT_GT:
+                                opName = ">";
+                                break;
+                            default:
+                                break;
+                        }
+
+                        auto displayOperand = [&](GenTree* const tree) {
+                            if (tree->IsCnsIntOrI())
+                            {
+                                fprintf(fgxFile, "%d", tree->AsIntCon()->gtIconVal);
+                            }
+                            else if (tree->IsCnsFltOrDbl())
+                            {
+                                fprintf(fgxFile, "%g", tree->AsDblCon()->gtDconVal);
+                            }
+                            else if (tree->IsLocal())
+                            {
+                                fprintf(fgxFile, "V%02u", tree->AsLclVarCommon()->GetLclNum());
+                            }
+                            else
+                            {
+                                fprintf(fgxFile, "[%s]", GenTree::OpName(tree->OperGet()));
+                            }
+                        };
+
+                        GenTree* const lhs = compareTree->AsOp()->gtOp1;
+                        GenTree* const rhs = compareTree->AsOp()->gtOp2;
+
+                        displayOperand(lhs);
+                        fprintf(fgxFile, " %s ", opName);
+                        displayOperand(rhs);
+                    }
+                    else
+                    {
+                        // !OperIsCompare
+                        fprintf(fgxFile, "[%s]", GenTree::OpName(compareTree->OperGet()));
+                    }
+                }
+            }
 
             // "Raw" Profile weight
             if (block->hasProfileWeight())

diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h
index 783a7e0..9634442 100644 (file)
--- a/src/coreclr/jit/jitconfigvalues.h
+++ b/src/coreclr/jit/jitconfigvalues.h
@@ -211,6 +211,8 @@ CONFIG_INTEGER(JitDumpFgLoops, W("JitDumpFgLoops"), 0) // 0 == no loop regions;
 CONFIG_INTEGER(JitDumpFgConstrained, W("JitDumpFgConstrained"), 1) // 0 == don't constrain to mostly linear layout;
                                                                    // non-zero == force mostly lexical block
                                                                    // linear layout
+CONFIG_INTEGER(JitDumpFgBlockID, W("JitDumpFgBlockID"), 0) // 0 == display block with bbNum; 1 == display with both
+                                                           // bbNum and bbID
 
 CONFIG_STRING(JitLateDisasmTo, W("JITLateDisasmTo"))
 CONFIG_STRING(JitRange, W("JitRange"))