[clang][fatlto] Don't set ThinLTO module flag with FatLTO

[ilovepi]

Since FatLTO now uses the UnifiedLTO pipeline, we should not set the ThinLTO module flag to true, since it may cause an assertion failure. See #70703 for context.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Paul Kirth (ilovepi)

Changes

Since FatLTO now uses the UnifiedLTO pipeline, we should not set the ThinLTO module flag to true, since it may cause an assertion failure. See #70703 for context.

---

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

2 Files Affected:

• (modified) clang/lib/CodeGen/BackendUtil.cpp (+1-4)
• (modified) clang/test/CodeGen/fat-lto-objects.c (+1-2)

diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index 77455c075cab0d..5ffda8117db1b3 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -1063,11 +1063,8 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
     }
   }
   if (CodeGenOpts.FatLTO) {
-    // Set module flags, like EnableSplitLTOUnit and UnifiedLTO, since FatLTO
+    // Set the EnableSplitLTOUnit and UnifiedLTO module flags, since FatLTO
     // uses a different action than Backend_EmitBC or Backend_EmitLL.
-    if (!TheModule->getModuleFlag("ThinLTO"))
-      TheModule->addModuleFlag(llvm::Module::Error, "ThinLTO",
-                               uint32_t(CodeGenOpts.PrepareForThinLTO));
     if (!TheModule->getModuleFlag("EnableSplitLTOUnit"))
       TheModule->addModuleFlag(llvm::Module::Error, "EnableSplitLTOUnit",
                                uint32_t(CodeGenOpts.EnableSplitLTOUnit));
diff --git a/clang/test/CodeGen/fat-lto-objects.c b/clang/test/CodeGen/fat-lto-objects.c
index 95207e77c244cc..5c8ad1fd93c4b3 100644
--- a/clang/test/CodeGen/fat-lto-objects.c
+++ b/clang/test/CodeGen/fat-lto-objects.c
@@ -35,8 +35,7 @@
 //   SPLIT: ![[#]] = !{i32 1, !"EnableSplitLTOUnit", i32 1}
 // NOSPLIT: ![[#]] = !{i32 1, !"EnableSplitLTOUnit", i32 0}
 
-/// Check that the ThinLTO metadata is set true for both full and thin LTO, since FatLTO is based on UnifiedLTO.
-//     FULL: ![[#]] = !{i32 1, !"ThinLTO", i32 1}
+// FULL-NOT: ![[#]] = !{i32 1, !"ThinLTO", i32 0}
 // THIN-NOT: ![[#]] = !{i32 1, !"ThinLTO", i32 0}
 
 /// FatLTO always uses UnifiedLTO. It's an error if they aren't set together

[llvmbot]

@llvm/pr-subscribers-clang-codegen

Author: Paul Kirth (ilovepi)

Changes

Since FatLTO now uses the UnifiedLTO pipeline, we should not set the ThinLTO module flag to true, since it may cause an assertion failure. See #70703 for context.

---

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

2 Files Affected:

• (modified) clang/lib/CodeGen/BackendUtil.cpp (+1-4)
• (modified) clang/test/CodeGen/fat-lto-objects.c (+1-2)

diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index 77455c075cab0d..5ffda8117db1b3 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -1063,11 +1063,8 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
     }
   }
   if (CodeGenOpts.FatLTO) {
-    // Set module flags, like EnableSplitLTOUnit and UnifiedLTO, since FatLTO
+    // Set the EnableSplitLTOUnit and UnifiedLTO module flags, since FatLTO
     // uses a different action than Backend_EmitBC or Backend_EmitLL.
-    if (!TheModule->getModuleFlag("ThinLTO"))
-      TheModule->addModuleFlag(llvm::Module::Error, "ThinLTO",
-                               uint32_t(CodeGenOpts.PrepareForThinLTO));
     if (!TheModule->getModuleFlag("EnableSplitLTOUnit"))
       TheModule->addModuleFlag(llvm::Module::Error, "EnableSplitLTOUnit",
                                uint32_t(CodeGenOpts.EnableSplitLTOUnit));
diff --git a/clang/test/CodeGen/fat-lto-objects.c b/clang/test/CodeGen/fat-lto-objects.c
index 95207e77c244cc..5c8ad1fd93c4b3 100644
--- a/clang/test/CodeGen/fat-lto-objects.c
+++ b/clang/test/CodeGen/fat-lto-objects.c
@@ -35,8 +35,7 @@
 //   SPLIT: ![[#]] = !{i32 1, !"EnableSplitLTOUnit", i32 1}
 // NOSPLIT: ![[#]] = !{i32 1, !"EnableSplitLTOUnit", i32 0}
 
-/// Check that the ThinLTO metadata is set true for both full and thin LTO, since FatLTO is based on UnifiedLTO.
-//     FULL: ![[#]] = !{i32 1, !"ThinLTO", i32 1}
+// FULL-NOT: ![[#]] = !{i32 1, !"ThinLTO", i32 0}
 // THIN-NOT: ![[#]] = !{i32 1, !"ThinLTO", i32 0}
 
 /// FatLTO always uses UnifiedLTO. It's an error if they aren't set together

[teresajohnson]

Added a comment to that issue, I think it would be good to understand why unified LTO is not expected in that case (for the assertion).

[nikic]

LGTM -- I think this change is clearly right, independently of the ModuleID issue.