[NFCI]Remove EntryCount from FunctionSummary and clean up surrounding…

… synthetic count passes. (llvm#107471) The primary motivation is to remove `EntryCount` from `FunctionSummary`. This frees 8 bytes out of `sizeof(FunctionSummary)` (136 bytes as of llvm@64498c5). While I'm at it, this PR clean up {SummaryBasedOptimizations, SyntheticCountsPropagation} since they were not used and there are no plans to further invest on them. With this patch, bitcode writer writes a placeholder 0 at the byte offset of `EntryCount` and bitcode reader can parse the function entry count at the correct byte offset. Added a TODO to stop writing `EntryCount` and bump bitcode version
kraj · Sep 6, 2024 · d4ddf06 · d4ddf06
1 parent 3a13c5a
commit d4ddf06
Show file tree

Hide file tree

Showing 27 changed files with 23 additions and 567 deletions.
diff --git a/clang/docs/tools/clang-formatted-files.txt b/clang/docs/tools/clang-formatted-files.txt
@@ -5349,7 +5349,6 @@ llvm/include/llvm/IR/SSAContext.h
 llvm/include/llvm/IR/StructuralHash.h
 llvm/include/llvm/IR/TrackingMDRef.h
 llvm/include/llvm/IR/UseListOrder.h
-llvm/include/llvm/LTO/SummaryBasedOptimizations.h
 llvm/include/llvm/MC/MCAsmInfoCOFF.h
 llvm/include/llvm/MC/MCAsmInfoDarwin.h
 llvm/include/llvm/MC/MCAsmInfoELF.h
@@ -5586,7 +5585,6 @@ llvm/include/llvm/Transforms/IPO/SampleProfile.h
 llvm/include/llvm/Transforms/IPO/SampleProfileProbe.h
 llvm/include/llvm/Transforms/IPO/SCCP.h
 llvm/include/llvm/Transforms/IPO/StripSymbols.h
-llvm/include/llvm/Transforms/IPO/SyntheticCountsPropagation.h
 llvm/include/llvm/Transforms/IPO/ThinLTOBitcodeWriter.h
 llvm/include/llvm/Transforms/IPO/WholeProgramDevirt.h
 llvm/include/llvm/Transforms/Scalar/ADCE.h
@@ -6070,7 +6068,6 @@ llvm/lib/IR/SSAContext.cpp
 llvm/lib/IR/Statepoint.cpp
 llvm/lib/IR/StructuralHash.cpp
 llvm/lib/IR/ValueSymbolTable.cpp
-llvm/lib/LTO/SummaryBasedOptimizations.cpp
 llvm/lib/MC/MCAsmInfoCOFF.cpp
 llvm/lib/MC/MCAsmInfoELF.cpp
 llvm/lib/MC/MCAsmInfoGOFF.cpp
@@ -6861,7 +6858,6 @@ llvm/lib/Transforms/IPO/OpenMPOpt.cpp
 llvm/lib/Transforms/IPO/SampleContextTracker.cpp
 llvm/lib/Transforms/IPO/SampleProfileProbe.cpp
 llvm/lib/Transforms/IPO/StripDeadPrototypes.cpp
-llvm/lib/Transforms/IPO/SyntheticCountsPropagation.cpp
 llvm/lib/Transforms/ObjCARC/BlotMapVector.h
 llvm/lib/Transforms/ObjCARC/ObjCARCExpand.cpp
 llvm/lib/Transforms/ObjCARC/ProvenanceAnalysis.h

diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -858,9 +858,8 @@ class FunctionSummary : public GlobalValueSummary {
             GlobalValue::DefaultVisibility,
             /*NotEligibleToImport=*/true, /*Live=*/true, /*IsLocal=*/false,
             /*CanAutoHide=*/false, GlobalValueSummary::ImportKind::Definition),
-        /*NumInsts=*/0, FunctionSummary::FFlags{}, /*EntryCount=*/0,
-        SmallVector<ValueInfo, 0>(), std::move(Edges),
-        std::vector<GlobalValue::GUID>(),
+        /*NumInsts=*/0, FunctionSummary::FFlags{}, SmallVector<ValueInfo, 0>(),
+        std::move(Edges), std::vector<GlobalValue::GUID>(),
         std::vector<FunctionSummary::VFuncId>(),
         std::vector<FunctionSummary::VFuncId>(),
         std::vector<FunctionSummary::ConstVCall>(),
@@ -880,11 +879,6 @@ class FunctionSummary : public GlobalValueSummary {
   /// Function summary specific flags.
   FFlags FunFlags;
 
-  /// The synthesized entry count of the function.
-  /// This is only populated during ThinLink phase and remains unused while
-  /// generating per-module summaries.
-  uint64_t EntryCount = 0;
-
   /// List of <CalleeValueInfo, CalleeInfo> call edge pairs from this function.
   std::vector<EdgeTy> CallGraphEdgeList;
 
@@ -915,7 +909,7 @@ class FunctionSummary : public GlobalValueSummary {
 
 public:
   FunctionSummary(GVFlags Flags, unsigned NumInsts, FFlags FunFlags,
-                  uint64_t EntryCount, SmallVectorImpl<ValueInfo> &&Refs,
+                  SmallVectorImpl<ValueInfo> &&Refs,
                   std::vector<EdgeTy> CGEdges,
                   std::vector<GlobalValue::GUID> TypeTests,
                   std::vector<VFuncId> TypeTestAssumeVCalls,
@@ -925,7 +919,7 @@ class FunctionSummary : public GlobalValueSummary {
                   std::vector<ParamAccess> Params, CallsitesTy CallsiteList,
                   AllocsTy AllocList)
       : GlobalValueSummary(FunctionKind, Flags, std::move(Refs)),
-        InstCount(NumInsts), FunFlags(FunFlags), EntryCount(EntryCount),
+        InstCount(NumInsts), FunFlags(FunFlags),
         CallGraphEdgeList(std::move(CGEdges)) {
     if (!TypeTests.empty() || !TypeTestAssumeVCalls.empty() ||
         !TypeCheckedLoadVCalls.empty() || !TypeTestAssumeConstVCalls.empty() ||
@@ -960,12 +954,6 @@ class FunctionSummary : public GlobalValueSummary {
   /// Get the instruction count recorded for this function.
   unsigned instCount() const { return InstCount; }
 
-  /// Get the synthetic entry count for this function.
-  uint64_t entryCount() const { return EntryCount; }
-
-  /// Set the synthetic entry count for this function.
-  void setEntryCount(uint64_t EC) { EntryCount = EC; }
-
   /// Return the list of <CalleeValueInfo, CalleeInfo> pairs.
   ArrayRef<EdgeTy> calls() const { return CallGraphEdgeList; }
 
@@ -1587,9 +1575,6 @@ class ModuleSummaryIndex {
     return WithAttributePropagation && GVS->maybeWriteOnly();
   }
 
-  bool hasSyntheticEntryCounts() const { return HasSyntheticEntryCounts; }
-  void setHasSyntheticEntryCounts() { HasSyntheticEntryCounts = true; }
-
   bool withSupportsHotColdNew() const { return WithSupportsHotColdNew; }
   void setWithSupportsHotColdNew() { WithSupportsHotColdNew = true; }
 

diff --git a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
@@ -229,9 +229,9 @@ template <> struct CustomMappingTraits<GlobalValueSummaryMapTy> {
               FSum.NotEligibleToImport, FSum.Live, FSum.IsLocal,
               FSum.CanAutoHide,
               static_cast<GlobalValueSummary::ImportKind>(FSum.ImportType)),
-          /*NumInsts=*/0, FunctionSummary::FFlags{}, /*EntryCount=*/0,
-          std::move(Refs), ArrayRef<FunctionSummary::EdgeTy>{},
-          std::move(FSum.TypeTests), std::move(FSum.TypeTestAssumeVCalls),
+          /*NumInsts=*/0, FunctionSummary::FFlags{}, std::move(Refs),
+          ArrayRef<FunctionSummary::EdgeTy>{}, std::move(FSum.TypeTests),
+          std::move(FSum.TypeTestAssumeVCalls),
           std::move(FSum.TypeCheckedLoadVCalls),
           std::move(FSum.TypeTestAssumeConstVCalls),
           std::move(FSum.TypeCheckedLoadConstVCalls),

diff --git a/llvm/include/llvm/LTO/SummaryBasedOptimizations.h b/llvm/include/llvm/LTO/SummaryBasedOptimizations.h
diff --git a/llvm/include/llvm/Transforms/IPO/SyntheticCountsPropagation.h b/llvm/include/llvm/Transforms/IPO/SyntheticCountsPropagation.h
diff --git a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
@@ -673,9 +673,9 @@ static void computeFunctionSummary(
   if (auto *SSI = GetSSICallback(F))
     ParamAccesses = SSI->getParamAccesses(Index);
   auto FuncSummary = std::make_unique<FunctionSummary>(
-      Flags, NumInsts, FunFlags, /*EntryCount=*/0, std::move(Refs),
-      CallGraphEdges.takeVector(), TypeTests.takeVector(),
-      TypeTestAssumeVCalls.takeVector(), TypeCheckedLoadVCalls.takeVector(),
+      Flags, NumInsts, FunFlags, std::move(Refs), CallGraphEdges.takeVector(),
+      TypeTests.takeVector(), TypeTestAssumeVCalls.takeVector(),
+      TypeCheckedLoadVCalls.takeVector(),
       TypeTestAssumeConstVCalls.takeVector(),
       TypeCheckedLoadConstVCalls.takeVector(), std::move(ParamAccesses),
       std::move(Callsites), std::move(Allocs));
@@ -963,7 +963,7 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex(
                         /* MayThrow */ true,
                         /* HasUnknownCall */ true,
                         /* MustBeUnreachable */ false},
-                    /*EntryCount=*/0, SmallVector<ValueInfo, 0>{},
+                    SmallVector<ValueInfo, 0>{},
                     ArrayRef<FunctionSummary::EdgeTy>{},
                     ArrayRef<GlobalValue::GUID>{},
                     ArrayRef<FunctionSummary::VFuncId>{},

diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
@@ -9453,8 +9453,8 @@ bool LLParser::parseFunctionSummary(std::string Name, GlobalValue::GUID GUID,
     return true;
 
   auto FS = std::make_unique<FunctionSummary>(
-      GVFlags, InstCount, FFlags, /*EntryCount=*/0, std::move(Refs),
-      std::move(Calls), std::move(TypeIdInfo.TypeTests),
+      GVFlags, InstCount, FFlags, std::move(Refs), std::move(Calls),
+      std::move(TypeIdInfo.TypeTests),
       std::move(TypeIdInfo.TypeTestAssumeVCalls),
       std::move(TypeIdInfo.TypeCheckedLoadVCalls),
       std::move(TypeIdInfo.TypeTestAssumeConstVCalls),

diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -7686,8 +7686,8 @@ Error ModuleSummaryIndexBitcodeReader::parseEntireSummary(unsigned ID) {
         PendingAllocs.clear();
       }
       auto FS = std::make_unique<FunctionSummary>(
-          Flags, InstCount, getDecodedFFlags(RawFunFlags), /*EntryCount=*/0,
-          std::move(Refs), std::move(Calls), std::move(PendingTypeTests),
+          Flags, InstCount, getDecodedFFlags(RawFunFlags), std::move(Refs),
+          std::move(Calls), std::move(PendingTypeTests),
           std::move(PendingTypeTestAssumeVCalls),
           std::move(PendingTypeCheckedLoadVCalls),
           std::move(PendingTypeTestAssumeConstVCalls),
@@ -7832,8 +7832,8 @@ Error ModuleSummaryIndexBitcodeReader::parseEntireSummary(unsigned ID) {
       ValueInfo VI = std::get<0>(getValueInfoFromValueId(ValueID));
       setSpecialRefs(Refs, NumRORefs, NumWORefs);
       auto FS = std::make_unique<FunctionSummary>(
-          Flags, InstCount, getDecodedFFlags(RawFunFlags), EntryCount,
-          std::move(Refs), std::move(Edges), std::move(PendingTypeTests),
+          Flags, InstCount, getDecodedFFlags(RawFunFlags), std::move(Refs),
+          std::move(Edges), std::move(PendingTypeTests),
           std::move(PendingTypeTestAssumeVCalls),
           std::move(PendingTypeCheckedLoadVCalls),
           std::move(PendingTypeTestAssumeConstVCalls),

diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -4740,7 +4740,8 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
         getEncodedGVSummaryFlags(FS->flags(), shouldImportValueAsDecl(FS)));
     NameVals.push_back(FS->instCount());
     NameVals.push_back(getEncodedFFlags(FS->fflags()));
-    NameVals.push_back(FS->entryCount());
+    // TODO: Stop writing entry count and bump bitcode version.
+    NameVals.push_back(0 /* EntryCount */);
 
     // Fill in below
     NameVals.push_back(0); // numrefs

diff --git a/llvm/lib/IR/ModuleSummaryIndex.cpp b/llvm/lib/IR/ModuleSummaryIndex.cpp
@@ -91,12 +91,11 @@ constexpr uint64_t ModuleSummaryIndex::BitcodeSummaryVersion;
 
 uint64_t ModuleSummaryIndex::getFlags() const {
   uint64_t Flags = 0;
+  // Flags & 0x4 is reserved. DO NOT REUSE.
   if (withGlobalValueDeadStripping())
     Flags |= 0x1;
   if (skipModuleByDistributedBackend())
     Flags |= 0x2;
-  if (hasSyntheticEntryCounts())
-    Flags |= 0x4;
   if (enableSplitLTOUnit())
     Flags |= 0x8;
   if (partiallySplitLTOUnits())
@@ -124,10 +123,7 @@ void ModuleSummaryIndex::setFlags(uint64_t Flags) {
   // Set on combined index only.
   if (Flags & 0x2)
     setSkipModuleByDistributedBackend();
-  // 1 bit: HasSyntheticEntryCounts flag.
-  // Set on combined index only.
-  if (Flags & 0x4)
-    setHasSyntheticEntryCounts();
+  // Flags & 0x4 is reserved. DO NOT REUSE.
   // 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.

diff --git a/llvm/lib/LTO/CMakeLists.txt b/llvm/lib/LTO/CMakeLists.txt
@@ -3,7 +3,6 @@ add_llvm_component_library(LLVMLTO
   LTOBackend.cpp
   LTOModule.cpp
   LTOCodeGenerator.cpp
-  SummaryBasedOptimizations.cpp
   UpdateCompilerUsed.cpp
   ThinLTOCodeGenerator.cpp
 

diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
@@ -32,7 +32,6 @@
 #include "llvm/IR/Metadata.h"
 #include "llvm/IR/RuntimeLibcalls.h"
 #include "llvm/LTO/LTOBackend.h"
-#include "llvm/LTO/SummaryBasedOptimizations.h"
 #include "llvm/Linker/IRMover.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Object/IRObjectFile.h"
@@ -1693,9 +1692,6 @@ Error LTO::runThinLTO(AddStreamFn AddStream, FileCache Cache,
     if (!ModuleToDefinedGVSummaries.count(Mod.first))
       ModuleToDefinedGVSummaries.try_emplace(Mod.first);
 
-  // Synthesize entry counts for functions in the CombinedIndex.
-  computeSyntheticCounts(ThinLTO.CombinedIndex);
-
   FunctionImporter::ImportListsTy ImportLists(ThinLTO.ModuleMap.size());
   DenseMap<StringRef, FunctionImporter::ExportSetTy> ExportLists(
       ThinLTO.ModuleMap.size());

diff --git a/llvm/lib/LTO/SummaryBasedOptimizations.cpp b/llvm/lib/LTO/SummaryBasedOptimizations.cpp
diff --git a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
@@ -27,15 +27,14 @@
 #include "llvm/Config/llvm-config.h"
 #include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/DiagnosticPrinter.h"
-#include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/LLVMRemarkStreamer.h"
+#include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/Mangler.h"
 #include "llvm/IR/PassTimingInfo.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/IRReader/IRReader.h"
 #include "llvm/LTO/LTO.h"
-#include "llvm/LTO/SummaryBasedOptimizations.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Object/IRObjectFile.h"
 #include "llvm/Passes/PassBuilder.h"
@@ -1042,9 +1041,6 @@ void ThinLTOCodeGenerator::run() {
   // Compute "dead" symbols, we don't want to import/export these!
   computeDeadSymbolsInIndex(*Index, GUIDPreservedSymbols);
 
-  // Synthesize entry counts for functions in the combined index.
-  computeSyntheticCounts(*Index);
-
   // Currently there is no support for enabling whole program visibility via a
   // linker option in the old LTO API, but this call allows it to be specified
   // via the internal option. Must be done before WPD below.

diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
@@ -180,7 +180,6 @@
 #include "llvm/Transforms/IPO/SampleProfileProbe.h"
 #include "llvm/Transforms/IPO/StripDeadPrototypes.h"
 #include "llvm/Transforms/IPO/StripSymbols.h"
-#include "llvm/Transforms/IPO/SyntheticCountsPropagation.h"
 #include "llvm/Transforms/IPO/WholeProgramDevirt.h"
 #include "llvm/Transforms/InstCombine/InstCombine.h"
 #include "llvm/Transforms/Instrumentation.h"