[llvm-branch-commits] [llvm] df4dc78 - [ThinLTO] Write distributed index summaries in GUID order (#234322)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Oct 22 09:48:10 PDT 2026
Author: Kyungwoo Lee
Date: 2026-10-22T09:13:05-07:00
New Revision: df4dc784725a3c4031efb9185d22cc1c5b55eeee
URL: https://github.com/llvm/llvm-project/commit/df4dc784725a3c4031efb9185d22cc1c5b55eeee
DIFF: https://github.com/llvm/llvm-project/commit/df4dc784725a3c4031efb9185d22cc1c5b55eeee.diff
LOG: [ThinLTO] Write distributed index summaries in GUID order (#234322)
The writer emitted each module's summaries in DenseMap order, which
depends on insertion order. Adding or changing an unrelated module can
change that order, so an unchanged module's .thinlto.bc gets different
bytes even though its contents are the same. Build systems that cache
backend compiles by input bytes then rerun them for nothing.
Sort the summaries by GUID before writing, as the full-index path
already does.
Assisted by Opus 5.5
Added:
Modified:
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/test/ThinLTO/X86/distributed_indexes.ll
llvm/unittests/Bitcode/BitReaderTest.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index e0280a15533c88..fed7408d7bf7f7 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -487,6 +487,13 @@ class IndexBitcodeWriter : public BitcodeWriterBase {
/// provides a map of modules to the corresponding GUIDs/summaries to write.
const ModuleToSummariesForIndexTy *ModuleToSummariesForIndex;
+ /// The summaries in ModuleToSummariesForIndex, sorted by GUID. The
+ /// per-module maps are DenseMaps whose iteration order depends on insertion
+ /// order, which can change with unrelated modules in the link. Writing in
+ /// this order makes logically identical indexes byte-identical.
+ SmallVector<std::pair<GlobalValue::GUID, GlobalValueSummary *>, 0>
+ SortedSummariesForIndex;
+
/// Map that holds the correspondence between the GUID used in the combined
/// index and a value id generated by this class to use in references.
std::map<GlobalValue::GUID, unsigned> GUIDToValueIdMap;
@@ -522,6 +529,15 @@ class IndexBitcodeWriter : public BitcodeWriterBase {
: BitcodeWriterBase(Stream, StrtabBuilder), Index(Index),
DecSummaries(DecSummaries),
ModuleToSummariesForIndex(ModuleToSummariesForIndex) {
+ if (ModuleToSummariesForIndex) {
+ for (const auto &[ModPath, Summaries] : *ModuleToSummariesForIndex)
+ llvm::append_range(SortedSummariesForIndex, Summaries);
+ // Each module's summaries are in DenseMap order, which depends on
+ // insertion order. Sort by GUID for byte-identical output. A GUID may
+ // appear under more than one module; the stable sort keeps those in the
+ // std::map's module path order.
+ llvm::stable_sort(SortedSummariesForIndex, llvm::less_first());
+ }
// See if the StackIdIndex was already added to the StackId map and
// vector. If not, record it.
@@ -588,15 +604,14 @@ class IndexBitcodeWriter : public BitcodeWriterBase {
template<typename Functor>
void forEachSummary(Functor Callback) {
if (ModuleToSummariesForIndex) {
- for (auto &M : *ModuleToSummariesForIndex)
- for (auto &[GUID, GVS] : M.second) {
- Callback({GUID, GVS}, false);
- // Ensure aliasee is handled, e.g. for assigning a valueId,
- // even if we are not importing the aliasee directly (the
- // imported alias will contain a copy of aliasee).
- if (auto *AS = dyn_cast<AliasSummary>(GVS))
- Callback({AS->getAliaseeGUID(), &AS->getAliasee()}, true);
- }
+ for (auto &[GUID, GVS] : SortedSummariesForIndex) {
+ Callback({GUID, GVS}, false);
+ // Ensure aliasee is handled, e.g. for assigning a valueId,
+ // even if we are not importing the aliasee directly (the
+ // imported alias will contain a copy of aliasee).
+ if (auto *AS = dyn_cast<AliasSummary>(GVS))
+ Callback({AS->getAliaseeGUID(), &AS->getAliasee()}, true);
+ }
} else {
// Sort by GUID for deterministic output.
for (const auto &Summaries : Index.sortedGlobalValueSummariesRange())
diff --git a/llvm/test/ThinLTO/X86/distributed_indexes.ll b/llvm/test/ThinLTO/X86/distributed_indexes.ll
index 824c582c2025eb..59a084bc144676 100644
--- a/llvm/test/ThinLTO/X86/distributed_indexes.ll
+++ b/llvm/test/ThinLTO/X86/distributed_indexes.ll
@@ -21,9 +21,10 @@
; BACKEND1-DAG: <VALUE_GUID {{.*}} op0={{.*}}
; BACKEND1-DAG: <VALUE_GUID {{.*}} op0={{.*}}
; BACKEND1-DAG: <VALUE_GUID {{.*}} op0={{.*}}
-; BACKEND1-NEXT: <COMBINED_PROFILE {{.*}} op1=0
-; BACKEND1-NEXT: <COMBINED_PROFILE {{.*}} op1=0
-; BACKEND1-NEXT: <COMBINED_PROFILE {{.*}} op1=1
+; Summaries are written in GUID order, so they may interleave across modules.
+; BACKEND1-DAG: <COMBINED_PROFILE {{.*}} op1=0
+; BACKEND1-DAG: <COMBINED_PROFILE {{.*}} op1=0
+; BACKEND1-DAG: <COMBINED_PROFILE {{.*}} op1=1
; BACKEND1-NEXT: <COMBINED_ALIAS {{.*}} op1=1
; BACKEND1-NEXT: </GLOBALVAL_SUMMARY_BLOCK
diff --git a/llvm/unittests/Bitcode/BitReaderTest.cpp b/llvm/unittests/Bitcode/BitReaderTest.cpp
index 3f9e8ef2d94698..0d6a1cf72eb92d 100644
--- a/llvm/unittests/Bitcode/BitReaderTest.cpp
+++ b/llvm/unittests/Bitcode/BitReaderTest.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "BitReaderTestCode.h"
+#include "llvm/ADT/DenseMapInfo.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/AsmParser/Parser.h"
@@ -22,6 +23,7 @@
#include "llvm/IR/LLVMContext.h"
#include "llvm/IR/Metadata.h"
#include "llvm/IR/Module.h"
+#include "llvm/IR/ModuleSummaryIndex.h"
#include "llvm/IR/Type.h"
#include "llvm/IR/Verifier.h"
#include "llvm/Support/Debug.h"
@@ -440,6 +442,105 @@ TEST(BitReaderTest, AccessMetadataTypeInfo) {
"!{0, i32}}}}");
}
+// A distributed ThinLTO index must not depend on the insertion order of the
+// per-module summary maps it is written from. That order follows the import
+// discovery order, which can change with unrelated modules in the link.
+TEST(BitWriterTest, DistributedIndexIgnoresSummaryInsertionOrder) {
+ // Pick GUIDs that collide in a DenseMap so that their iteration order
+ // depends on insertion order.
+ SmallVector<GlobalValue::GUID, 32> GUIDs;
+ for (uint64_t Candidate = 1; GUIDs.size() != 32; ++Candidate)
+ if ((DenseMapInfo<GlobalValue::GUID>::getHashValue(Candidate) & 255) == 0)
+ GUIDs.push_back(Candidate);
+
+ // GUIDs[0] calls GUIDs[1] and references the variable GUIDs[2]. GUIDs[3]
+ // aliases GUIDs[4]. GUIDs[5] and GUIDs[6] have callsites with
diff erent
+ // stack ids, so the stack id table also depends on the summary order.
+ std::string Assembly =
+ "^0 = module: (path: \"m.o\", hash: (0, 0, 0, 0, 0))\n";
+ auto AddGV = [&](unsigned I, StringRef Summary) {
+ Assembly += "^" + std::to_string(I + 1) +
+ " = gv: (guid: " + std::to_string(GUIDs[I]) + ", summaries: (" +
+ Summary.str() + "))\n";
+ };
+ auto Function = [](StringRef Extra = "") {
+ return "function: (module: ^0, flags: (linkage: external), insts: 1" +
+ Extra.str() + ")";
+ };
+ AddGV(0, Function(", calls: ((callee: ^2)), refs: (^3)"));
+ AddGV(1, Function());
+ AddGV(2, "variable: (module: ^0, flags: (linkage: external), varFlags: "
+ "(readonly: 1))");
+ AddGV(3, "alias: (module: ^0, flags: (linkage: external), aliasee: ^5)");
+ AddGV(4, Function());
+ AddGV(5, Function(", calls: ((callee: ^2)), callsites: ((callee: ^2, "
+ "clones: (0), stackIds: (101))))"));
+ AddGV(6, Function(", calls: ((callee: ^2)), callsites: ((callee: ^2, "
+ "clones: (0), stackIds: (202))))"));
+ for (unsigned I = 7; I != GUIDs.size(); ++I)
+ AddGV(I, Function());
+
+ SMDiagnostic Err;
+ std::unique_ptr<ModuleSummaryIndex> Index =
+ parseSummaryIndexAssemblyString(Assembly, Err);
+ ASSERT_TRUE(Index) << Err.getMessage().str();
+
+ SmallVector<SmallVector<GlobalValue::GUID, 32>, 3> InsertionOrders;
+ InsertionOrders.push_back(GUIDs);
+ InsertionOrders.emplace_back(GUIDs.rbegin(), GUIDs.rend());
+ auto &Strided = InsertionOrders.emplace_back();
+ for (unsigned I = 0; I != GUIDs.size(); ++I)
+ Strided.push_back(GUIDs[(I * 7 + 3) % GUIDs.size()]);
+
+ SmallVector<SmallVector<GlobalValue::GUID, 32>, 3> IterationOrders;
+ SmallVector<SmallString<0>, 3> Bitcodes;
+ for (ArrayRef<GlobalValue::GUID> Order : InsertionOrders) {
+ ModuleToSummariesForIndexTy ModuleToSummariesForIndex;
+ GVSummaryMapTy &Summaries = ModuleToSummariesForIndex["m.o"];
+ for (GlobalValue::GUID GUID : Order)
+ Summaries[GUID] = Index->getGlobalValueSummary(GUID);
+ IterationOrders.emplace_back(make_first_range(Summaries));
+
+ raw_svector_ostream OS(Bitcodes.emplace_back());
+ writeIndexToFile(*Index, OS, &ModuleToSummariesForIndex);
+ }
+
+ // Make sure the inputs really iterate in
diff erent orders.
+ EXPECT_TRUE(any_of(drop_begin(IterationOrders), [&](const auto &Order) {
+ return Order != IterationOrders.front();
+ }));
+ for (const SmallString<0> &Bitcode : drop_begin(Bitcodes))
+ EXPECT_TRUE(Bitcode == Bitcodes.front())
+ << "index bytes depend on summary insertion order";
+
+ // The written index still has the expected edges.
+ Expected<std::unique_ptr<ModuleSummaryIndex>> Written =
+ getModuleSummaryIndex(MemoryBufferRef(Bitcodes.front(), "index"));
+ if (!Written)
+ FAIL() << toString(Written.takeError());
+ auto *Caller =
+ dyn_cast<FunctionSummary>((*Written)->getGlobalValueSummary(GUIDs[0]));
+ ASSERT_NE(Caller, nullptr);
+ ASSERT_EQ(Caller->calls().size(), 1u);
+ EXPECT_EQ(Caller->calls().front().first.getGUID(), GUIDs[1]);
+ ASSERT_EQ(Caller->refs().size(), 1u);
+ EXPECT_EQ(Caller->refs().front().getGUID(), GUIDs[2]);
+ auto *Alias =
+ dyn_cast<AliasSummary>((*Written)->getGlobalValueSummary(GUIDs[3]));
+ ASSERT_NE(Alias, nullptr);
+ EXPECT_EQ(Alias->getAliaseeGUID(), GUIDs[4]);
+ for (auto [I, StackId] : {std::pair<unsigned, uint64_t>{5, 101}, {6, 202}}) {
+ auto *FS =
+ dyn_cast<FunctionSummary>((*Written)->getGlobalValueSummary(GUIDs[I]));
+ ASSERT_NE(FS, nullptr);
+ ASSERT_EQ(FS->callsites().size(), 1u);
+ ASSERT_EQ(FS->callsites().front().StackIdIndices.size(), 1u);
+ EXPECT_EQ((*Written)->getStackIdAtIndex(
+ FS->callsites().front().StackIdIndices.front()),
+ StackId);
+ }
+}
+
// Counts calls to the given debug intrinsic in F.
static unsigned countIntrinsicCalls(Function &F, Intrinsic::ID ID) {
unsigned Count = 0;
More information about the llvm-branch-commits
mailing list