ThinLTO: Add flag to print uselistorder in bitcode writer pass #133230

arsenm · 2025-03-27T10:37:06Z

This is needed in llvm-reduce to avoid perturbing the uselistorder in
intermediate steps. Really llvm-reduce wants pure serialization with
no dependency on the pass manager. There are other optimizations mixed
in to the serialization here depending on metadata in the module, which
is also bad.

Part of #63621

arsenm · 2025-03-27T10:37:22Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

llvmbot · 2025-03-27T10:38:50Z

@llvm/pr-subscribers-llvm-transforms

Author: Matt Arsenault (arsenm)

Changes

Use it in llvm-reduce to avoid perturbing the use order in the intermediate
step.

Fixes #63621

Full diff: https://github.com/llvm/llvm-project/pull/133230.diff

5 Files Affected:

(modified) llvm/include/llvm/Transforms/IPO/ThinLTOBitcodeWriter.h (+5-2)
(modified) llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp (+11-7)
(added) llvm/test/tools/llvm-reduce/thinlto-preserve-uselistorder.ll (+19)
(modified) llvm/tools/llvm-reduce/ReducerWorkItem.cpp (+6-3)
(modified) llvm/tools/opt/NewPMDriver.cpp (+2-1)

diff --git a/llvm/include/llvm/Transforms/IPO/ThinLTOBitcodeWriter.h b/llvm/include/llvm/Transforms/IPO/ThinLTOBitcodeWriter.h
index 9bcb01c9dbe43..b8fed10404dfa 100644
--- a/llvm/include/llvm/Transforms/IPO/ThinLTOBitcodeWriter.h
+++ b/llvm/include/llvm/Transforms/IPO/ThinLTOBitcodeWriter.h
@@ -26,12 +26,15 @@ class ThinLTOBitcodeWriterPass
     : public PassInfoMixin<ThinLTOBitcodeWriterPass> {
   raw_ostream &OS;
   raw_ostream *ThinLinkOS;
+  const bool ShouldPreserveUseListOrder;
 
 public:
   // Writes bitcode to OS. Also write thin link file to ThinLinkOS, if
   // it's not nullptr.
-  ThinLTOBitcodeWriterPass(raw_ostream &OS, raw_ostream *ThinLinkOS)
-      : OS(OS), ThinLinkOS(ThinLinkOS) {}
+  ThinLTOBitcodeWriterPass(raw_ostream &OS, raw_ostream *ThinLinkOS,
+                           bool ShouldPreserveUseListOrder = false)
+      : OS(OS), ThinLinkOS(ThinLinkOS),
+        ShouldPreserveUseListOrder(ShouldPreserveUseListOrder) {}
 
   PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
 
diff --git a/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp b/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
index cd0e412bdf353..c0bb641f23ed6 100644
--- a/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
+++ b/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
@@ -274,7 +274,8 @@ static bool enableUnifiedLTO(Module &M) {
 // regular LTO bitcode file to OS.
 void splitAndWriteThinLTOBitcode(
     raw_ostream &OS, raw_ostream *ThinLinkOS,
-    function_ref<AAResults &(Function &)> AARGetter, Module &M) {
+    function_ref<AAResults &(Function &)> AARGetter, Module &M,
+    const bool ShouldPreserveUseListOrder) {
   std::string ModuleId = getUniqueModuleId(&M);
   if (ModuleId.empty()) {
     assert(!enableUnifiedLTO(M));
@@ -487,9 +488,9 @@ void splitAndWriteThinLTOBitcode(
   // be used in the backends, and use that in the minimized bitcode
   // produced for the full link.
   ModuleHash ModHash = {{0}};
-  W.writeModule(M, /*ShouldPreserveUseListOrder=*/false, &Index,
+  W.writeModule(M, ShouldPreserveUseListOrder, &Index,
                 /*GenerateHash=*/true, &ModHash);
-  W.writeModule(*MergedM, /*ShouldPreserveUseListOrder=*/false, &MergedMIndex);
+  W.writeModule(*MergedM, ShouldPreserveUseListOrder, &MergedMIndex);
   W.writeSymtab();
   W.writeStrtab();
   OS << Buffer;
@@ -530,13 +531,15 @@ bool hasTypeMetadata(Module &M) {
 
 bool writeThinLTOBitcode(raw_ostream &OS, raw_ostream *ThinLinkOS,
                          function_ref<AAResults &(Function &)> AARGetter,
-                         Module &M, const ModuleSummaryIndex *Index) {
+                         Module &M, const ModuleSummaryIndex *Index,
+                         const bool ShouldPreserveUseListOrder) {
   std::unique_ptr<ModuleSummaryIndex> NewIndex = nullptr;
   // See if this module has any type metadata. If so, we try to split it
   // or at least promote type ids to enable WPD.
   if (hasTypeMetadata(M)) {
     if (enableSplitLTOUnit(M)) {
-      splitAndWriteThinLTOBitcode(OS, ThinLinkOS, AARGetter, M);
+      splitAndWriteThinLTOBitcode(OS, ThinLinkOS, AARGetter, M,
+                                  ShouldPreserveUseListOrder);
       return true;
     }
     // Promote type ids as needed for index-based WPD.
@@ -564,7 +567,7 @@ bool writeThinLTOBitcode(raw_ostream &OS, raw_ostream *ThinLinkOS,
   // be used in the backends, and use that in the minimized bitcode
   // produced for the full link.
   ModuleHash ModHash = {{0}};
-  WriteBitcodeToFile(M, OS, /*ShouldPreserveUseListOrder=*/false, Index,
+  WriteBitcodeToFile(M, OS, ShouldPreserveUseListOrder, Index,
                      /*GenerateHash=*/true, &ModHash);
   // If a minimized bitcode module was requested for the thin link, only
   // the information that is needed by thin link will be written in the
@@ -591,7 +594,8 @@ llvm::ThinLTOBitcodeWriterPass::run(Module &M, ModuleAnalysisManager &AM) {
       [&FAM](Function &F) -> AAResults & {
         return FAM.getResult<AAManager>(F);
       },
-      M, &AM.getResult<ModuleSummaryIndexAnalysis>(M));
+      M, &AM.getResult<ModuleSummaryIndexAnalysis>(M),
+      ShouldPreserveUseListOrder);
 
   return Changed ? PreservedAnalyses::none() : PreservedAnalyses::all();
 }
diff --git a/llvm/test/tools/llvm-reduce/thinlto-preserve-uselistorder.ll b/llvm/test/tools/llvm-reduce/thinlto-preserve-uselistorder.ll
new file mode 100644
index 0000000000000..2332f2d632911
--- /dev/null
+++ b/llvm/test/tools/llvm-reduce/thinlto-preserve-uselistorder.ll
@@ -0,0 +1,19 @@
+; RUN: opt --thinlto-bc --thinlto-split-lto-unit %s -o %t.0
+; RUN: llvm-reduce -write-tmp-files-as-bitcode --delta-passes=instructions %t.0 -o %t.1 \
+; RUN:     --test %python --test-arg %p/Inputs/llvm-dis-and-filecheck.py --test-arg llvm-dis --test-arg FileCheck --test-arg --check-prefix=INTERESTING --test-arg %s
+; RUN: llvm-dis --preserve-ll-uselistorder %t.1 -o %t.2
+; RUN: FileCheck --check-prefix=RESULT %s < %t.2
+
+define i32 @func(i32 %arg0, i32 %arg1) {
+entry:
+  %add0 = add i32 %arg0, 0
+  %add1 = add i32 %add0, 0
+  %add2 = add i32 %add1, 0
+  %add3 = add i32 %arg1, 0
+  %add4 = add i32 %add2, %add3
+  ret i32 %add4
+}
+
+; INTERESTING: uselistorder i32 0
+; RESULT: uselistorder i32 0, { 0, 2, 1 }
+uselistorder i32 0, { 3, 2, 1, 0 }
diff --git a/llvm/tools/llvm-reduce/ReducerWorkItem.cpp b/llvm/tools/llvm-reduce/ReducerWorkItem.cpp
index ad40d8d8baa36..c93af57f0bd6f 100644
--- a/llvm/tools/llvm-reduce/ReducerWorkItem.cpp
+++ b/llvm/tools/llvm-reduce/ReducerWorkItem.cpp
@@ -769,6 +769,8 @@ void ReducerWorkItem::readBitcode(MemoryBufferRef Data, LLVMContext &Ctx,
 }
 
 void ReducerWorkItem::writeBitcode(raw_ostream &OutStream) const {
+  const bool ShouldPreserveUseListOrder = true;
+
   if (LTOInfo && LTOInfo->IsThinLTO && LTOInfo->EnableSplitLTOUnit) {
     PassBuilder PB;
     LoopAnalysisManager LAM;
@@ -781,7 +783,8 @@ void ReducerWorkItem::writeBitcode(raw_ostream &OutStream) const {
     PB.registerLoopAnalyses(LAM);
     PB.crossRegisterProxies(LAM, FAM, CGAM, MAM);
     ModulePassManager MPM;
-    MPM.addPass(ThinLTOBitcodeWriterPass(OutStream, nullptr));
+    MPM.addPass(ThinLTOBitcodeWriterPass(OutStream, nullptr,
+                                         ShouldPreserveUseListOrder));
     MPM.run(*M, MAM);
   } else {
     std::unique_ptr<ModuleSummaryIndex> Index;
@@ -790,8 +793,8 @@ void ReducerWorkItem::writeBitcode(raw_ostream &OutStream) const {
       Index = std::make_unique<ModuleSummaryIndex>(
           buildModuleSummaryIndex(*M, nullptr, &PSI));
     }
-    WriteBitcodeToFile(getModule(), OutStream,
-                       /*ShouldPreserveUseListOrder=*/true, Index.get());
+    WriteBitcodeToFile(getModule(), OutStream, ShouldPreserveUseListOrder,
+                       Index.get());
   }
 }
 
diff --git a/llvm/tools/opt/NewPMDriver.cpp b/llvm/tools/opt/NewPMDriver.cpp
index b0840bb5b392f..cff77bbd78ca3 100644
--- a/llvm/tools/opt/NewPMDriver.cpp
+++ b/llvm/tools/opt/NewPMDriver.cpp
@@ -517,7 +517,8 @@ bool llvm::runPassPipeline(
     break;
   case OK_OutputThinLTOBitcode:
     MPM.addPass(ThinLTOBitcodeWriterPass(
-        Out->os(), ThinLTOLinkOut ? &ThinLTOLinkOut->os() : nullptr));
+        Out->os(), ThinLTOLinkOut ? &ThinLTOLinkOut->os() : nullptr,
+        ShouldPreserveBitcodeUseListOrder));
     break;
   }

teresajohnson

A few suggestions below. Suggest making the reduce changes a follow on patch, and just test the writer alone for this one.

teresajohnson · 2025-03-27T16:18:31Z

llvm/test/tools/llvm-reduce/thinlto-preserve-uselistorder.ll

@@ -0,0 +1,19 @@
+; RUN: opt --thinlto-bc --thinlto-split-lto-unit %s -o %t.0


I assume the change should affect %t.0 so that can be checked too?

Not really relevant here, and redundant with the final result check

llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp

teresajohnson · 2025-03-31T15:29:02Z

llvm/test/Bitcode/thinlto-preserve-uselistorder.ll

+; RUN: opt --preserve-bc-uselistorder --thinlto-bc --thinlto-split-lto-unit < %s | llvm-dis --preserve-ll-uselistorder | FileCheck %s
+
+; Test with -thin-link-bitcode-file
+; RUN: opt --preserve-bc-uselistorder --thinlto-bc --thinlto-split-lto-unit -thin-link-bitcode-file=%t.thinlink.bc < %s | llvm-dis --preserve-ll-uselistorder | FileCheck %s


You don't need to test with -thin-link-bitcode-file, which is a file just containing the ThinLTO summary.

As noted, the second invocation here is redundant.

arsenm · 2025-04-14T18:29:49Z

Merge activity

Apr 14, 2:29 PM EDT: A user started a stack merge that includes this pull request via Graphite.
Apr 14, 2:32 PM EDT: Graphite rebased this pull request as part of a merge.
Apr 14, 2:34 PM EDT: A user merged this pull request with Graphite.

This is needed in llvm-reduce to avoid perturbing the uselistorder in intermediate steps. Really llvm-reduce wants pure serialization with no dependency on the pass manager. There are other optimizations mixed in to the serialization here depending on metadata in the module, which is also bad. Part of #63621

…133230) This is needed in llvm-reduce to avoid perturbing the uselistorder in intermediate steps. Really llvm-reduce wants pure serialization with no dependency on the pass manager. There are other optimizations mixed in to the serialization here depending on metadata in the module, which is also bad. Part of llvm#63621

arsenm added the llvm-reduce label Mar 27, 2025 — with Graphite App

arsenm requested review from aeubanks, ormris, pcc, samitolvanen and teresajohnson March 27, 2025 10:38

arsenm marked this pull request as ready for review March 27, 2025 10:38

llvmbot added the llvm:transforms label Mar 27, 2025

teresajohnson reviewed Mar 27, 2025

View reviewed changes

arsenm force-pushed the users/arsenm/issue63621/add-option-preserve-uselistorder-thinlto-bitcode-writer-pass branch from 85aa697 to f323086 Compare March 28, 2025 05:22

arsenm mentioned this pull request Mar 28, 2025

llvm-reduce: Preserve uselistorder when writing thinlto bitcode #133369

Merged

arsenm force-pushed the users/arsenm/issue63621/add-option-preserve-uselistorder-thinlto-bitcode-writer-pass branch from f323086 to 2df298a Compare March 28, 2025 05:26

teresajohnson reviewed Mar 31, 2025

View reviewed changes

arsenm force-pushed the users/arsenm/issue63621/add-option-preserve-uselistorder-thinlto-bitcode-writer-pass branch 2 times, most recently from 660836c to d0e10cb Compare April 12, 2025 17:37

teresajohnson approved these changes Apr 13, 2025

View reviewed changes

arsenm force-pushed the users/arsenm/issue63621/add-option-preserve-uselistorder-thinlto-bitcode-writer-pass branch from d0e10cb to 5b797c4 Compare April 14, 2025 14:19

arsenm added 4 commits April 14, 2025 18:31

Test another place

86954ec

Split test

d700707

Remove a run line

dae6bf4

arsenm force-pushed the users/arsenm/issue63621/add-option-preserve-uselistorder-thinlto-bitcode-writer-pass branch from 5b797c4 to dae6bf4 Compare April 14, 2025 18:31

arsenm merged commit c5ac63e into main Apr 14, 2025
6 of 10 checks passed

arsenm deleted the users/arsenm/issue63621/add-option-preserve-uselistorder-thinlto-bitcode-writer-pass branch April 14, 2025 18:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ThinLTO: Add flag to print uselistorder in bitcode writer pass #133230

ThinLTO: Add flag to print uselistorder in bitcode writer pass #133230

Uh oh!

arsenm commented Mar 27, 2025 •

edited

Loading

Uh oh!

arsenm commented Mar 27, 2025 •

edited

Loading

Uh oh!

llvmbot commented Mar 27, 2025

Uh oh!

teresajohnson left a comment

Uh oh!

teresajohnson Mar 27, 2025

Uh oh!

arsenm Mar 28, 2025

Uh oh!

Uh oh!

Uh oh!

teresajohnson Mar 31, 2025

Uh oh!

teresajohnson Apr 12, 2025

Uh oh!

arsenm commented Apr 14, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

		@@ -0,0 +1,19 @@
		; RUN: opt --thinlto-bc --thinlto-split-lto-unit %s -o %t.0

ThinLTO: Add flag to print uselistorder in bitcode writer pass #133230

ThinLTO: Add flag to print uselistorder in bitcode writer pass #133230

Uh oh!

Conversation

arsenm commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arsenm commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Mar 27, 2025

Uh oh!

teresajohnson left a comment

Choose a reason for hiding this comment

Uh oh!

teresajohnson Mar 27, 2025

Choose a reason for hiding this comment

Uh oh!

arsenm Mar 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

teresajohnson Mar 31, 2025

Choose a reason for hiding this comment

Uh oh!

teresajohnson Apr 12, 2025

Choose a reason for hiding this comment

Uh oh!

arsenm commented Apr 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

Uh oh!

arsenm commented Mar 27, 2025 •

edited

Loading

arsenm commented Mar 27, 2025 •

edited

Loading

arsenm commented Apr 14, 2025 •

edited

Loading