[llvm] [LTO] Fix a crash with thin LTO caching and asm output (PR #138203)

Alexey Karyakin via llvm-commits llvm-commits at lists.llvm.org
Mon May 12 15:00:29 PDT 2025


================
@@ -439,27 +439,33 @@ static void codegen(const Config &Conf, TargetMachine *TM,
   std::unique_ptr<CachedFileStream> &Stream = *StreamOrErr;
   TM->Options.ObjectFilenameForDebug = Stream->ObjectPathName;
 
-  legacy::PassManager CodeGenPasses;
-  TargetLibraryInfoImpl TLII(Mod.getTargetTriple());
-  CodeGenPasses.add(new TargetLibraryInfoWrapperPass(TLII));
-  // No need to make index available if the module is empty.
-  // In theory these passes should not use the index for an empty
-  // module, however, this guards against doing any unnecessary summary-based
-  // analysis in the case of a ThinLTO build where this might be an empty
-  // regular LTO combined module, with a large combined index from ThinLTO.
-  if (!isEmptyModule(Mod))
-    CodeGenPasses.add(
-        createImmutableModuleSummaryIndexWrapperPass(&CombinedIndex));
-  if (Conf.PreCodeGenPassesHook)
-    Conf.PreCodeGenPassesHook(CodeGenPasses);
-  if (TM->addPassesToEmitFile(CodeGenPasses, *Stream->OS,
-                              DwoOut ? &DwoOut->os() : nullptr,
-                              Conf.CGFileType))
-    report_fatal_error("Failed to setup codegen");
-  CodeGenPasses.run(Mod);
-
-  if (DwoOut)
-    DwoOut->keep();
+  // Create the LTO pipeline in its own scope so it gets deleted before
+  // Stream->commit() is called. The commit function of CacheFile deletes
----------------
quic-akaryaki wrote:

I think the primary question is if we are ok with the API change in #136121. Not only it requires the user to explicitly call commit() but also make sure to write all data before calling commit. There requirements are reasonable but require more explicit work, which previously was hidden in destructors. If that is fine, there is no way around other than flushing data from MCAsmStreamer either by destroying it before commit() (how I did here) or by adding another API. In that case we can keep #136121 and add this one on top because this seems to me like the easiest fix. If we want to revert and reimplement #136121, please integrate my changes from this PR along with the test.

https://github.com/llvm/llvm-project/pull/138203


More information about the llvm-commits mailing list