[ThinLTO] Record in index whether IR used a flattened sample PGO profile

[tejohnson]

Summary

This will be used to adjust importing threshold defaults, since
flattened profiles are less accurate about hotness.

It required moving the flattened-profile-use flag definition to a
different location that is accessible by the pass managers as well as
from the module summary index.

[tejohnson]

lib/ProfileData/SampleProf.cpp

28

@wmi Would it be possible to record this information in the sample profile file itself? That would be cleaner.

[wmi]

LGTM.

lib/ProfileData/SampleProf.cpp

28

That is a possiblity. But have to consider what is the best way to category the information we want to encode.

[tejohnson]

Decided to take a different approach for now.

include/llvm/IR/ModuleSummaryIndex.h

private:
 
   // True if the index was created for a module compiled with -fsplit-lto-unit.
   bool EnableSplitLTOUnit;
 
   // True if some of the modules were compiled with -fsplit-lto-unit and
   // some were not. Set when the combined index is created during the thin link.
   bool PartiallySplitLTOUnits = false;
 
+  // True if compiled with a flattened sample PGO profile. This is set in the
+  // per module index during module summary building. For the combined index
+  // it contains the OR of all input summary index flags.
+  bool FlattenedProfileUsed = false;
+
   std::set<std::string> CfiFunctionDefs;
   std::set<std::string> CfiFunctionDecls;
 
   // Used in cases where we want to record the name of a global, but
   // don't have the string owned elsewhere (e.g. the Strtab on a module).
   StringSaver Saver;
   BumpPtrAllocator Alloc;
 
   // YAML I/O support.
   friend yaml::MappingTraits<ModuleSummaryIndex>;
 
   GlobalValueSummaryMapTy::value_type *
   getOrInsertValuePtr(GlobalValue::GUID GUID) {
     return &*GlobalValueMap.emplace(GUID, GlobalValueSummaryInfo(HaveGVs))
                  .first;
   }
 
 public:
   // See HaveGVs variable comment.
-  ModuleSummaryIndex(bool HaveGVs, bool EnableSplitLTOUnit = false)
-      : HaveGVs(HaveGVs), EnableSplitLTOUnit(EnableSplitLTOUnit), Saver(Alloc) {
-  }
+  ModuleSummaryIndex(bool HaveGVs, bool EnableSplitLTOUnit = false,
+                     bool FlattenedProfileUsed = false)
+      : HaveGVs(HaveGVs), EnableSplitLTOUnit(EnableSplitLTOUnit),
+        FlattenedProfileUsed(FlattenedProfileUsed), Saver(Alloc) {}
 
   bool haveGVs() const { return HaveGVs; }
 
   gvsummary_iterator begin() { return GlobalValueMap.begin(); }
   const_gvsummary_iterator begin() const { return GlobalValueMap.begin(); }
   gvsummary_iterator end() { return GlobalValueMap.end(); }
   const_gvsummary_iterator end() const { return GlobalValueMap.end(); }
   size_t size() const { return GlobalValueMap.size(); }
public:
   }
 
   bool enableSplitLTOUnit() const { return EnableSplitLTOUnit; }
   void setEnableSplitLTOUnit() { EnableSplitLTOUnit = true; }
 
   bool partiallySplitLTOUnits() const { return PartiallySplitLTOUnits; }
   void setPartiallySplitLTOUnits() { PartiallySplitLTOUnits = true; }
 
+  bool flattenedProfileUsed() const { return FlattenedProfileUsed; }
+  void setFlattenedProfileUsed() { FlattenedProfileUsed = true; }
+
   bool isGlobalValueLive(const GlobalValueSummary *GVS) const {
     return !WithGlobalValueDeadStripping || GVS->isLive();
   }
   bool isGUIDLive(GlobalValue::GUID GUID) const;
 
   /// Return a ValueInfo for the index value_type (convenient when iterating
   /// index).
   ValueInfo getValueInfo(const GlobalValueSummaryMapTy::value_type &R) const {

lib/Analysis/ModuleSummaryAnalysis.cpp

cl::values(clEnumValN(FunctionSummary::FSHT_None, "none", "None."),
                           "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."));
 
+extern cl::opt<bool> FlattenedProfileUsed;
+
 // 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 
ModuleSummaryIndex llvm::buildModuleSummaryIndex(
     const Module &M,
     std::function<BlockFrequencyInfo *(const Function &F)> GetBFICallback,
     ProfileSummaryInfo *PSI) {
   assert(PSI);
   bool EnableSplitLTOUnit = false;
   if (auto *MD = mdconst::extract_or_null<ConstantInt>(
           M.getModuleFlag("EnableSplitLTOUnit")))
     EnableSplitLTOUnit = MD->getZExtValue();
-  ModuleSummaryIndex Index(/*HaveGVs=*/true, EnableSplitLTOUnit);
+  ModuleSummaryIndex Index(/*HaveGVs=*/true, EnableSplitLTOUnit,
+                           FlattenedProfileUsed);
 
   // Identify the local values in the llvm.used and llvm.compiler.used sets,
   // which should not be exported as they would then require renaming and
   // promotion, but we may have opaque uses e.g. in inline asm. We collect them
   // here because we use this information to mark functions containing inline
   // assembly calls as not importable.
   SmallPtrSet<GlobalValue *, 8> LocalsUsed;
   SmallPtrSet<GlobalValue *, 8> Used;

lib/Bitcode/Reader/BitcodeReader.cpp

while (true) {
     Record.clear();
     auto BitCode = Stream.readRecord(Entry.ID, Record);
     switch (BitCode) {
     default: // Default behavior: ignore.
       break;
     case bitc::FS_FLAGS: {  // [flags]
       uint64_t Flags = Record[0];
       // Scan flags.
-      assert(Flags <= 0x1f && "Unexpected bits in flag");
+      assert(Flags <= 0x2f && "Unexpected bits in flag");
 
       // 1 bit: WithGlobalValueDeadStripping flag.
       // Set on combined index only.
       if (Flags & 0x1)
         TheIndex.setWithGlobalValueDeadStripping();
       // 1 bit: SkipModuleByDistributedBackend flag.
       // Set on combined index only.
       if (Flags & 0x2)
         TheIndex.setSkipModuleByDistributedBackend();
       // 1 bit: HasSyntheticEntryCounts flag.
       // Set on combined index only.
       if (Flags & 0x4)
         TheIndex.setHasSyntheticEntryCounts();
       // 1 bit: DisableSplitLTOUnit flag.
       // Set on per module indexes. It is up to the client to validate
       // the consistency of this flag across modules being linked.
       if (Flags & 0x8)
         TheIndex.setEnableSplitLTOUnit();
       // 1 bit: PartiallySplitLTOUnits flag.
       // Set on combined index only.
       if (Flags & 0x10)
         TheIndex.setPartiallySplitLTOUnits();
+      // 1 bit: FlattenedProfileUsed flag.
+      // Set on per module indexes. This effectively ORs the flag across all
+      // modules being linked.
+      if (Flags & 0x20)
+        TheIndex.setFlattenedProfileUsed();
       break;
     }
     case bitc::FS_VALUE_GUID: { // [valueid, refguid]
       uint64_t ValueID = Record[0];
       GlobalValue::GUID RefGUID = Record[1];
       ValueIdToValueInfoMap[ValueID] =
           std::make_pair(TheIndex.getOrInsertValueInfo(RefGUID), RefGUID);
       break;
while (true) {
     Record.clear();
     auto BitCode = Stream.readRecord(Entry.ID, Record);
     switch (BitCode) {
     default: // Default behavior: ignore.
       break;
     case bitc::FS_FLAGS: { // [flags]
       uint64_t Flags = Record[0];
       // Scan flags.
-      assert(Flags <= 0x1f && "Unexpected bits in flag");
+      assert(Flags <= 0x2f && "Unexpected bits in flag");
 
       return Flags & 0x8;
     }
     }
   }
   llvm_unreachable("Exit infinite loop");
 }
 

lib/Bitcode/Writer/BitcodeWriter.cpp

void ModuleBitcodeWriterBase::writePerModuleGlobalValueSummary() {
   Stream.EnterSubblock(IsThinLTO ? bitc::GLOBALVAL_SUMMARY_BLOCK_ID
                                  : bitc::FULL_LTO_GLOBALVAL_SUMMARY_BLOCK_ID,
                        4);
 
   Stream.EmitRecord(bitc::FS_VERSION, ArrayRef<uint64_t>{INDEX_VERSION});
 
   // Write the index flags.
   uint64_t Flags = 0;
-  // Bits 1-3 are set only in the combined index, skip them.
+  // Bits 1-3 and 5 are set only in the combined index, skip them.
   if (Index->enableSplitLTOUnit())
     Flags |= 0x8;
+  if (Index->flattenedProfileUsed())
+    Flags |= 0x20;
   Stream.EmitRecord(bitc::FS_FLAGS, ArrayRef<uint64_t>{Flags});
 
   if (Index->begin() == Index->end()) {
     Stream.ExitBlock();
     return;
   }
 
   for (const auto &GVI : valueIds()) {
void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
   if (Index.skipModuleByDistributedBackend())
     Flags |= 0x2;
   if (Index.hasSyntheticEntryCounts())
     Flags |= 0x4;
   if (Index.enableSplitLTOUnit())
     Flags |= 0x8;
   if (Index.partiallySplitLTOUnits())
     Flags |= 0x10;
+  if (Index.flattenedProfileUsed())
+    Flags |= 0x20;
   Stream.EmitRecord(bitc::FS_FLAGS, ArrayRef<uint64_t>{Flags});
 
   for (const auto &GVI : valueIds()) {
     Stream.EmitRecord(bitc::FS_VALUE_GUID,
                       ArrayRef<uint64_t>{GVI.second, GVI.first});
   }
 
   // Abbrev for FS_COMBINED.

lib/ProfileData/SampleProf.cpp

 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/raw_ostream.h"
 #include <string>
 #include <system_error>
 
 using namespace llvm;
 using namespace sampleprof;
 
+cl::opt<bool> FlattenedProfileUsed(
+    "flattened-profile-used", cl::init(false), cl::Hidden,
+    cl::desc("Indicate the sample profile being used is flattened, i.e., "
+             "no inline hierachy exists in the profile. "));
+
 namespace llvm {
 namespace sampleprof {
 SampleProfileFormat FunctionSamples::Format;
 DenseMap<uint64_t, StringRef> FunctionSamples::GUIDToFuncNameMap;
 Module *FunctionSamples::CurrentModule;
 } // namespace sampleprof
 } // namespace llvm
 

lib/Transforms/IPO/PassManagerBuilder.cpp

 static cl::opt<bool> EnableGVNSink(
     "enable-gvn-sink", cl::init(false), cl::Hidden,
     cl::desc("Enable the GVN sinking pass (default = off)"));
 
 static cl::opt<bool>
     EnableCHR("enable-chr", cl::init(true), cl::Hidden,
               cl::desc("Enable control height reduction optimization (CHR)"));
 
-cl::opt<bool> FlattenedProfileUsed(
-    "flattened-profile-used", cl::init(false), cl::Hidden,
-    cl::desc("Indicate the sample profile being used is flattened, i.e., "
-             "no inline hierachy exists in the profile. "));
+extern cl::opt<bool> FlattenedProfileUsed;
 
 PassManagerBuilder::PassManagerBuilder() {
     OptLevel = 2;
     SizeLevel = 0;
     LibraryInfo = nullptr;
     Inliner = nullptr;
     DisableUnrollLoops = false;
     SLPVectorize = RunSLPVectorization;

test/Bitcode/thinlto-flattened-flag.ll

+; REQUIRES: x86-registered-target
+
+; First generate four ThinLTO versions of this file, two without flattened
+; profiles and two with. Confirm that the flattened profile flag is set
+; approprately. It doesn't matter for the purposes of the summary analysis
+; whether there actually is a profile passed in.
+; RUN: opt -module-summary %s -o %tNF1.o
+; RUN: llvm-bcanalyzer -dump %tNF1.o | FileCheck %s --check-prefix=NOFLAT
+; RUN: opt -module-summary %s -o %tNF2.o
+; RUN: llvm-bcanalyzer -dump %tNF2.o | FileCheck %s --check-prefix=NOFLAT
+; RUN: opt -module-summary -flattened-profile-used %s -o %tF1.o
+; RUN: llvm-bcanalyzer -dump %tF1.o | FileCheck %s --check-prefix=FLAT
+; RUN: opt -module-summary -flattened-profile-used %s -o %tF2.o
+; RUN: llvm-bcanalyzer -dump %tF2.o | FileCheck %s --check-prefix=FLAT
+
+; NOFLAT: <FLAGS op0=0/>
+; FLAT: <FLAGS op0=32/>
+
+; Next check that the flag is propagated onto the combined index as expected.
+; The result should be an OR of the input flags.
+
+; RUN: llvm-lto2 run %tNF1.o %tNF2.o -o %t.out -save-temps
+; RUN: llvm-bcanalyzer -dump %t.out.index.bc | FileCheck %s --check-prefix=NOFLAT
+
+; RUN: llvm-lto2 run %tF1.o %tF2.o -o %t.out -save-temps
+; RUN: llvm-bcanalyzer -dump %t.out.index.bc | FileCheck %s --check-prefix=FLAT
+
+; RUN: llvm-lto2 run %tNF1.o %tF2.o -o %t.out -save-temps
+; RUN: llvm-bcanalyzer -dump %t.out.index.bc | FileCheck %s --check-prefix=FLAT
+
+; RUN: llvm-lto2 run %tF1.o %tNF2.o -o %t.out -save-temps
+; RUN: llvm-bcanalyzer -dump %t.out.index.bc | FileCheck %s --check-prefix=FLAT
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"