[ThinLTO] Add an option to force summary call edges cold for debugging

[tejohnson]

Summary

Useful to selectively disable importing into specific modules for
debugging/triaging/workarounds.

[tejohnson]

Remove change accidentlly included

[eraman]

lib/Analysis/ModuleSummaryAnalysis.cpp

331–332

Is this the desired behavior or should the flag not affect these edges that are forced to be inlined in AutoFDO? One can argue for preserving critical edges so that the profile gets applied correctly. If you think this behavior is desired, please mention this in the description text of the flag.

[tejohnson]

lib/Analysis/ModuleSummaryAnalysis.cpp

331–332

I think both options are potentially useful, so I have changed this to an enum so that we can support both.

[tejohnson]

Address comment

lib/Analysis/ModuleSummaryAnalysis.cpp

 #include "llvm/IR/Module.h"
 #include "llvm/IR/ModuleSummaryIndex.h"
 #include "llvm/IR/Use.h"
 #include "llvm/IR/User.h"
 #include "llvm/Object/ModuleSymbolTable.h"
 #include "llvm/Object/SymbolicFile.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/CommandLine.h"
 #include <algorithm>
 #include <cassert>
 #include <cstdint>
 #include <vector>
 
 using namespace llvm;
 
 #define DEBUG_TYPE "module-summary-analysis"
 
+// Option to force edges cold which will block importing when the
+// -import-cold-multiplier is set to 0. Useful for debugging.
+bool ForceSummaryEdgesCold = false;
+cl::opt<bool, true> FSEC(
+    "force-summary-edges-cold", cl::Hidden, cl::location(ForceSummaryEdgesCold),
+    cl::desc("Force all edges in the function summary to cold"));
+
 // 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).
 static void findRefEdges(ModuleSummaryIndex &Index, const User *CurUser,
                          SetVector<ValueInfo> &RefEdges,
                          SmallPtrSet<const User *, 8> &Visited) {
   SmallVector<const User *, 32> Worklist;
   Worklist.push_back(CurUser);
for (const Instruction &I : BB) {
               TypeTestAssumeConstVCalls, TypeCheckedLoadConstVCalls);
           continue;
         }
         // We should have named any anonymous globals
         assert(CalledFunction->hasName());
         auto ScaledCount = PSI->getProfileCount(&I, BFI);
         auto Hotness = ScaledCount ? getHotness(ScaledCount.getValue(), PSI)
                                    : CalleeInfo::HotnessType::Unknown;
+        if (ForceSummaryEdgesCold)
+          Hotness = CalleeInfo::HotnessType::Cold;
 
         // Use the original CalledValue, in case it was an alias. We want
         // to record the call edge to the alias in that case. Eventually
         // an alias summary will be created to associate the alias and
         // aliasee.
         auto &ValueInfo = CallGraphEdges[Index.getOrInsertValueInfo(
             cast<GlobalValue>(CalledValue))];
         ValueInfo.updateHotness(Hotness);
for (const Instruction &I : BB) {
               .updateHotness(getHotness(Candidate.Count, PSI));
       }
     }
 
   // Explicit add hot edges to enforce importing for designated GUIDs for
   // sample PGO, to enable the same inlines as the profiled optimized binary.
   for (auto &I : F.getImportGUIDs())
     CallGraphEdges[Index.getOrInsertValueInfo(I)].updateHotness(
-        CalleeInfo::HotnessType::Critical);
+        ForceSummaryEdgesCold ? CalleeInfo::HotnessType::Cold
+                              : CalleeInfo::HotnessType::Critical);
 
   bool NonRenamableLocal = isNonRenamableLocal(F);
   bool NotEligibleForImport =
       NonRenamableLocal || HasInlineAsmMaybeReferencingInternal ||
       // Inliner doesn't handle variadic functions.
       // FIXME: refactor this to use the same code that inliner is using.
       F.isVarArg() ||
       // Don't try to import functions with noinline attribute.

lib/Bitcode/Writer/BitcodeWriter.cpp

 static cl::opt<unsigned>
     IndexThreshold("bitcode-mdindex-threshold", cl::Hidden, cl::init(25),
                    cl::desc("Number of metadatas above which we emit an index "
                             "to enable lazy-loading"));
 
 cl::opt<bool> WriteRelBFToSummary(
     "write-relbf-to-summary", cl::Hidden, cl::init(false),
     cl::desc("Write relative block frequency to function summary "));
+
+extern bool ForceSummaryEdgesCold;
+
 namespace {
 
 /// These are manifest constants used by the bitcode writer. They do not need to
 /// be kept in sync with the reader, but need to be consistent within this file.
 enum {
   // VALUE_SYMTAB_BLOCK abbrev id's.
   VST_ENTRY_8_ABBREV = bitc::FIRST_APPLICATION_ABBREV,
   VST_ENTRY_7_ABBREV,
void ModuleBitcodeWriterBase::writePerModuleFunctionSummaryRecord(
   NameVals.push_back(getEncodedGVSummaryFlags(FS->flags()));
   NameVals.push_back(FS->instCount());
   NameVals.push_back(getEncodedFFlags(FS->fflags()));
   NameVals.push_back(FS->refs().size());
 
   for (auto &RI : FS->refs())
     NameVals.push_back(VE.getValueID(RI.getValue()));
 
-  bool HasProfileData = F.hasProfileData();
+  bool HasProfileData = F.hasProfileData() || ForceSummaryEdgesCold;
   for (auto &ECI : FS->calls()) {
     NameVals.push_back(getValueId(ECI.first));
     if (HasProfileData)
       NameVals.push_back(static_cast<uint8_t>(ECI.second.Hotness));
     else if (WriteRelBFToSummary)
       NameVals.push_back(ECI.second.RelBlockFreq);
   }
 

lib/Transforms/IPO/FunctionImport.cpp

auto GetBonusMultiplier = [](CalleeInfo::HotnessType Hotness) -> float {
       if (Hotness == CalleeInfo::HotnessType::Critical)
         return ImportCriticalMultiplier;
       return 1.0;
     };
 
     const auto NewThreshold =
         Threshold * GetBonusMultiplier(Edge.second.getHotness());
 
+    DEBUG(dbgs() << " edge -> " << VI.getGUID() << " NewThreshold:" << NewThreshold
+                 << " Hotness: " << (int)Edge.second.getHotness() << "\n");
     auto *CalleeSummary = selectCallee(Index, VI.getSummaryList(), NewThreshold,
                                        Summary.modulePath());
     if (!CalleeSummary) {
       DEBUG(dbgs() << "ignored! No qualifying callee with summary found.\n");
       continue;
     }
 
     // "Resolve" the summary

test/Transforms/FunctionImport/Inputs/funcimport_forcecold.ll

+define void @foo() {
+entry:
+  ret void
+}

test/Transforms/FunctionImport/funcimport_forcecold.ll

+; Test to ensure that building summary with -force-summary-edges-cold=true
+; blocks importing as expected.
+
+; "-stats" and "-debug-only" require +Asserts.
+; REQUIRES: asserts
+
+; First do with default options, which should import
+; RUN: opt -module-summary %s -o %t.bc
+; RUN: opt -module-summary %p/Inputs/funcimport_forcecold.ll -o %t2.bc
+; RUN: llvm-lto -thinlto -o %t3 %t.bc %t2.bc
+; RUN: opt -function-import -stats -print-imports -summary-file %t3.thinlto.bc %t.bc -S 2>&1 | FileCheck %s --check-prefix=IMPORT
+
+; Next rebuild caller module summary with edges forced cold.
+; Make sure we don't import.
+; RUN: opt -force-summary-edges-cold=true -module-summary %s -o %t.bc
+; RUN: llvm-lto -thinlto -o %t3 %t.bc %t2.bc
+; RUN: opt -function-import -stats -print-imports -summary-file %t3.thinlto.bc %t.bc -S 2>&1 | FileCheck %s --check-prefix=NOIMPORT
+
+define i32 @main() #0 {
+entry:
+  call void @foo()
+  ret i32 0
+}
+
+; IMPORT: Import foo
+; NOIMPORT-NOT: Import foo
+; IMPORT: define available_externally void @foo()
+; NOIMPORT: declare void @foo()
+declare void @foo()