[ThinLTO] Add option to dump per-module summary dot graph

[tejohnson]

Summary

I found that there currently isn't a way to invoke exportToDot from
the command line for a per-module summary index, and therefore no
testing of that case. Add an internal option and use it to test dumping
of per module summary indexes.

In particular, I am looking at fixing the limitation that causes the
aliasee GUID in the per-module summary to be 0, and want to be able to
test that change.

[mehdi_amini]

lib/Analysis/ModuleSummaryAnalysis.cpp

641

It this something that can be pushed to the client? It does not necessarily seems to me that it belongs to a library method that builds the summary (unless maybe if you're seeing this as pure debugging)

[tejohnson]

lib/Analysis/ModuleSummaryAnalysis.cpp

641

Not easily pushed into client - e.g. 'opt' simply invokes the BitcodeWriterPass passing in the flag as to whether to emit the summary index, and that pass simply adds the ModuleSummaryIndex building analysis pass as required. Unless by client you mean the bitcode writer? Since this should just be for debugging (and testing), making it an option on the builder analysis pass seemed to make sense.

[evgeny777]

LGTM. Thanks!

lib/Analysis/ModuleSummaryAnalysis.cpp

 cl::opt<FunctionSummary::ForceSummaryHotnessType, true> FSEC(
     "force-summary-edges-cold", cl::Hidden, cl::location(ForceSummaryEdgesCold),
     cl::desc("Force all edges in the function summary to cold"),
     cl::values(clEnumValN(FunctionSummary::FSHT_None, "none", "None."),
                clEnumValN(FunctionSummary::FSHT_AllNonCritical,
                           "all-non-critical", "All non-critical edges."),
                clEnumValN(FunctionSummary::FSHT_All, "all", "All edges.")));
 
+cl::opt<std::string> ModuleSummaryDotFile(
+    "module-summary-dot-file", cl::init(""), cl::Hidden,
+    cl::value_desc("filename"),
+    cl::desc("File to emit dot graph of new summary into."));
+
 // Walk through the operands of a given User via worklist iteration and populate
 // the set of GlobalValue references encountered. Invoked either on an
 // Instruction or a GlobalVariable (which walks its initializer).
 // Return true if any of the operands contains blockaddress. This is important
 // to know when computing summary for global var, because if global variable
 // references basic block address we can't import it separately from function
 // containing that basic block. For simplicity we currently don't import such
 // global vars at all. When importing function we aren't interested if any 
if (auto *FuncSummary = dyn_cast<FunctionSummary>(Summary.get())) {
           FuncSummary->calls(), [&](const FunctionSummary::EdgeTy &Edge) {
             return !CantBePromoted.count(Edge.first.getGUID());
           });
       if (!AllCallsCanBeExternallyReferenced)
         Summary->setNotEligibleToImport();
     }
   }
 
+  if (!ModuleSummaryDotFile.empty()) {
+    std::error_code EC;
+    raw_fd_ostream OSDot(ModuleSummaryDotFile, EC, sys::fs::OpenFlags::F_None);
+    if (EC)
+      report_fatal_error(Twine("Failed to open dot file ") +
+                         ModuleSummaryDotFile + ": " + EC.message() + "\n");
+    Index.exportToDot(OSDot);
+  }
+
   return Index;
 }
 
 AnalysisKey ModuleSummaryIndexAnalysis::Key;
 
 ModuleSummaryIndex
 ModuleSummaryIndexAnalysis::run(Module &M, ModuleAnalysisManager &AM) {
   ProfileSummaryInfo &PSI = AM.getResult<ProfileSummaryAnalysis>(M);

test/ThinLTO/X86/dot-dumper.ll

-; RUN: opt -module-summary %s -o %t1.bc
-; RUN: opt -module-summary %p/Inputs/dot-dumper.ll -o %t2.bc
+; RUN: opt -module-summary %s -o %t1.bc -module-summary-dot-file=%t1.dot
+; RUN: opt -module-summary %p/Inputs/dot-dumper.ll -o %t2.bc -module-summary-dot-file=%t2.dot
 ; RUN: llvm-lto2 run -save-temps %t1.bc %t2.bc -o %t3 \
 ; RUN:  -r=%t1.bc,main,px \
 ; RUN:  -r=%t1.bc,main_alias,p \
 ; RUN:  -r=%t1.bc,foo, \
 ; RUN:  -r=%t1.bc,A, \
 ; RUN:  -r=%t2.bc,foo,p \
 ; RUN:  -r=%t2.bc,bar,p \
 ; RUN:  -r=%t2.bc,A,p \
 ; RUN:  -r=%t2.bc,B,p
 
 ; Never assume specific order of clusters, nodes or edges
+; RUN: cat %t1.dot | FileCheck --check-prefix=STRUCTURE1 %s
+; RUN: cat %t1.dot | FileCheck --check-prefix=CLUSTER0 %s
+; RUN: cat %t2.dot | FileCheck --check-prefix=STRUCTURE2 %s
+; RUN: cat %t2.dot | FileCheck --check-prefix=CLUSTER1 %s
 ; RUN: cat %t3.index.dot | FileCheck --check-prefix=STRUCTURE %s
-; RUN: cat %t3.index.dot | FileCheck --check-prefix=CLUSTER0 %s
-; RUN: cat %t3.index.dot | FileCheck --check-prefix=CLUSTER1 %s
+; RUN: cat %t3.index.dot | FileCheck --check-prefix=CLUSTER0 --check-prefix=COMBINED0 %s
+; RUN: cat %t3.index.dot | FileCheck --check-prefix=CLUSTER1 --check-prefix=COMBINED1 %s
 
+; %t1 index
+; STRUCTURE1:        digraph Summary {
+; STRUCTURE1:          subgraph cluster_0
+; STRUCTURE1:          // Cross-module edges:
+; STRUCTURE1:          0 [label="@0"]; // defined externally
+; STRUCTURE1:          M0_{{[0-9]+}} -> 0 [style=dotted]; // alias
+; STRUCTURE1-DAG:      [[A:[0-9]+]] [label="A"]; // defined externally
+; STRUCTURE1-DAG:      [[FOO:[0-9]+]] [label="foo"]; // defined externally
+; STRUCTURE1-DAG:      M0_{{[0-9]+}} -> [[FOO]] // call
+; STRUCTURE1-DAG:      M0_{{[0-9]+}} -> [[A]] [{{.*}}]; // const-ref
+; STRUCTURE1-NEXT:   }
+
+; %t2 index
+; STRUCTURE2:        digraph Summary {
+; STRUCTURE2:          subgraph cluster_0
+; STRUCTURE2:          // Cross-module edges:
+; STRUCTURE2-NEXT:   }
+
+; Combined index
 ; STRUCTURE:        digraph Summary {
 ; STRUCTURE-DAG:      subgraph cluster_0
 ; STRUCTURE-DAG:      subgraph cluster_1
 ; STRUCTURE:          // Cross-module edges:
 ; STRUCTURE-DAG:      M0_{{[0-9]+}} -> M1_{{[0-9]+}} // call
 ; STRUCTURE-DAG:      M0_{{[0-9]+}} -> M1_{{[0-9]+}} [{{.*}}]; // const-ref
 ; STRUCTURE-NEXT:   }
 
-; CLUSTER0:         // Module: {{.*}}1.bc
-; CLUSTER0-NEXT:    subgraph cluster_0 {
-; CLUSTER0-DAG:       M0_[[MAIN_ALIAS:[0-9]+]] [{{.*}}main_alias{{.*}}]; // alias, dead
-; CLUSTER0-DAG:       M0_[[MAIN:[0-9]+]] [{{.*}}main|extern{{.*}}]; // function
+; CLUSTER0:         // Module: {{.*}}
+; CLUSTER0-NEXT:    subgraph cluster_[[ID0:[0-1]]] {
+; CLUSTER0-DAG:       M[[ID0]]_[[MAIN_ALIAS:[0-9]+]] [{{.*}}main_alias{{.*}}]; // alias, dead
+; CLUSTER0-DAG:       M[[ID0]]_[[MAIN:[0-9]+]] [{{.*}}main|extern{{.*}}]; // function
 ; CLUSTER0-NEXT:      // Edges:
-; CLUSTER0-NEXT:      M0_[[MAIN_ALIAS]] -> M0_[[MAIN]] [{{.*}}]; // alias
+; COMBINED0-NEXT:     M[[ID0]]_[[MAIN_ALIAS]] -> M[[ID0]]_[[MAIN]] [{{.*}}]; // alias
 ; CLUSTER0-NEXT:    }
 
-; CLUSTER1:         // Module: {{.*}}2.bc
-; CLUSTER1-NEXT:    subgraph cluster_1 {
-; CLUSTER1-DAG:       M1_[[A:[0-9]+]] [{{.*}}A|extern{{.*}}]; // variable, immutable
-; CLUSTER1-DAG:       M1_[[FOO:[0-9]+]] [{{.*}}foo|extern{{.*}} ffl: 00001{{.*}}]; // function
-; CLUSTER1-DAG:       M1_[[B:[0-9]+]] [{{.*}}B|extern{{.*}}]; // variable, immutable
-; CLUSTER1-DAG:       M1_[[BAR:[0-9]+]] [{{.*}}bar|extern{{.*}}]; // function, dead
+; For the combined index make sure we match the second cluster.
+; COMBINED1:      // Module: {{.*}}1.bc
+; CLUSTER1:         // Module:
+; COMBINED1-SAME: {{.*}}2.bc
+; CLUSTER1-NEXT:    subgraph cluster_[[ID1:[0-1]]] {
+; CLUSTER1-DAG:       M[[ID1]]_[[A:[0-9]+]] [{{.*}}A|extern{{.*}}]; // variable
+; COMBINED1-SAME: , immutable
+; CLUSTER1-DAG:       M[[ID1]]_[[FOO:[0-9]+]] [{{.*}}foo|extern{{.*}} ffl: 00001{{.*}}]; // function
+; CLUSTER1-DAG:       M[[ID1]]_[[B:[0-9]+]] [{{.*}}B|extern{{.*}}]; // variable
+; COMBINED1-SAME: , immutable
+; CLUSTER1-DAG:       M[[ID1]]_[[BAR:[0-9]+]] [{{.*}}bar|extern{{.*}}]; // function, dead
 ; CLUSTER1-NEXT:      // Edges:
-; CLUSTER1-DAG:       M1_[[FOO]] -> M1_[[B]] [{{.*}}]; // const-ref
-; CLUSTER1-DAG:       M1_[[FOO]] -> M1_[[A]] [{{.*}}]; // const-ref
+; CLUSTER1-DAG:       M[[ID1]]_[[FOO]] -> M[[ID1]]_[[B]] [{{.*}}]; // const-ref
+; CLUSTER1-DAG:       M[[ID1]]_[[FOO]] -> M[[ID1]]_[[A]] [{{.*}}]; // const-ref
 ; CLUSTER1-DAG:     }
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 @A = external local_unnamed_addr global i32, align 4
 
 ; Function Attrs: nounwind uwtable
 define i32 @main() local_unnamed_addr {
   %1 = tail call i32 (...) @foo()
   %2 = load i32, i32* @A, align 4
   %3 = add nsw i32 %2, %1
   ret i32 %3
 }
 @main_alias = weak_odr alias i32 (), i32 ()* @main
 declare i32 @foo(...) local_unnamed_addr