[clang] [clang][modules] Partially revert 48d0eb518 to fix -gmodules output (PR #124003)

2025-01-23 Thread Michael Buch via cfe-commits 

Michael137 wrote:

FYI, this also fixed the macOS public matrix bot. Some tests where we built 
with `-gmodules` were crashing. Though weirdly just the clang-17 tests (which 
build against clang-17 aligned libc++, so might have something to do with 
that). Just wanted to mentioned that here in case parts of this get relanded, 
be sure to run the LLDB tests (or at least keep an eye out on that matrix bot)

https://github.com/llvm/llvm-project/pull/124003
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][modules] Partially revert 48d0eb518 to fix -gmodules output (PR #124003)

2025-01-22 Thread Fangrui Song via cfe-commits 

MaskRay wrote:

Thanks!

https://github.com/llvm/llvm-project/pull/124003
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][modules] Partially revert 48d0eb518 to fix -gmodules output (PR #124003)

2025-01-22 Thread Steven Wu via cfe-commits 

https://github.com/cachemeifyoucan approved this pull request.


https://github.com/llvm/llvm-project/pull/124003
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][modules] Partially revert 48d0eb518 to fix -gmodules output (PR #124003)

2025-01-22 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Ben Langmuir (benlangmuir)


Changes

With the changes in 48d0eb518, the CodeGenOptions used to emit .pcm files with 
-fmodule-format=obj (-gmodules) were the ones from the original invocation, 
rather than the ones specifically crafted for outputting the pcm. This was 
causing the pcm to be written with only the debug info and without the 
__clangast section in some cases (e.g. -O2). This unforunately was not covered 
by existing tests, because compiling and loading a module within a single 
compilation load the ast content from the in-memory module cache rather than 
reading it from the pcm file that was written.  This broke bootstrapping a 
build of clang with modules enabled on Darwin.

rdar://143418834

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


5 Files Affected:

- (modified) clang/include/clang/CodeGen/BackendUtil.h (+2-2) 
- (modified) clang/lib/CodeGen/BackendUtil.cpp (+7-6) 
- (modified) clang/lib/CodeGen/CodeGenAction.cpp (+5-3) 
- (modified) clang/lib/CodeGen/ObjectFilePCHContainerWriter.cpp (+4-4) 
- (added) clang/test/Modules/gmodules-codegenopts.c (+18) 


``diff
diff --git a/clang/include/clang/CodeGen/BackendUtil.h 
b/clang/include/clang/CodeGen/BackendUtil.h
index 78d1e5ee8e6d59..92e0d13bf25b69 100644
--- a/clang/include/clang/CodeGen/BackendUtil.h
+++ b/clang/include/clang/CodeGen/BackendUtil.h
@@ -39,8 +39,8 @@ enum BackendAction {
   Backend_EmitObj   ///< Emit native object files
 };
 
-void emitBackendOutput(CompilerInstance &CI, StringRef TDesc, llvm::Module *M,
-   BackendAction Action,
+void emitBackendOutput(CompilerInstance &CI, CodeGenOptions &CGOpts,
+   StringRef TDesc, llvm::Module *M, BackendAction Action,
llvm::IntrusiveRefCntPtr VFS,
std::unique_ptr OS,
BackendConsumer *BC = nullptr);
diff --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index 3951ad01497cca..4bbcd22d4fc030 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -206,9 +206,10 @@ class EmitAssemblyHelper {
   }
 
 public:
-  EmitAssemblyHelper(CompilerInstance &CI, llvm::Module *M,
+  EmitAssemblyHelper(CompilerInstance &CI, CodeGenOptions &CGOpts,
+ llvm::Module *M,
  IntrusiveRefCntPtr VFS)
-  : CI(CI), Diags(CI.getDiagnostics()), CodeGenOpts(CI.getCodeGenOpts()),
+  : CI(CI), Diags(CI.getDiagnostics()), CodeGenOpts(CGOpts),
 TargetOpts(CI.getTargetOpts()), LangOpts(CI.getLangOpts()),
 TheModule(M), VFS(std::move(VFS)),
 TargetTriple(TheModule->getTargetTriple()) {}
@@ -1363,14 +1364,14 @@ runThinLTOBackend(CompilerInstance &CI, 
ModuleSummaryIndex *CombinedIndex,
   }
 }
 
-void clang::emitBackendOutput(CompilerInstance &CI, StringRef TDesc,
-  llvm::Module *M, BackendAction Action,
+void clang::emitBackendOutput(CompilerInstance &CI, CodeGenOptions &CGOpts,
+  StringRef TDesc, llvm::Module *M,
+  BackendAction Action,
   IntrusiveRefCntPtr VFS,
   std::unique_ptr OS,
   BackendConsumer *BC) {
   llvm::TimeTraceScope TimeScope("Backend");
   DiagnosticsEngine &Diags = CI.getDiagnostics();
-  const auto &CGOpts = CI.getCodeGenOpts();
 
   std::unique_ptr EmptyModule;
   if (!CGOpts.ThinLTOIndexFile.empty()) {
@@ -1410,7 +1411,7 @@ void clang::emitBackendOutput(CompilerInstance &CI, 
StringRef TDesc,
 }
   }
 
-  EmitAssemblyHelper AsmHelper(CI, M, VFS);
+  EmitAssemblyHelper AsmHelper(CI, CGOpts, M, VFS);
   AsmHelper.emitAssembly(Action, std::move(OS), BC);
 
   // Verify clang's TargetInfo DataLayout against the LLVM TargetMachine's
diff --git a/clang/lib/CodeGen/CodeGenAction.cpp 
b/clang/lib/CodeGen/CodeGenAction.cpp
index 15311fb2078101..7aa3639cabf392 100644
--- a/clang/lib/CodeGen/CodeGenAction.cpp
+++ b/clang/lib/CodeGen/CodeGenAction.cpp
@@ -312,7 +312,8 @@ void BackendConsumer::HandleTranslationUnit(ASTContext &C) {
 
   EmbedBitcode(getModule(), CodeGenOpts, llvm::MemoryBufferRef());
 
-  emitBackendOutput(CI, C.getTargetInfo().getDataLayoutString(), getModule(),
+  emitBackendOutput(CI, CI.getCodeGenOpts(),
+C.getTargetInfo().getDataLayoutString(), getModule(),
 Action, FS, std::move(AsmOutStream), this);
 
   Ctx.setDiagnosticHandler(std::move(OldDiagnosticHandler));
@@ -1173,8 +1174,9 @@ void CodeGenAction::ExecuteAction() {
   std::unique_ptr OptRecordFile =
   std::move(*OptRecordFileOrErr);
 
-  emitBackendOutput(CI, CI.getTarget().getDataLayoutString(), TheModule.get(),
-BA, CI.getFileManager().getVirtualFileSystemPtr(),
+  emitBackendOutput(CI, CI.getCodeGenOpts(),
+CI.getTarget().getDataLayoutSt

[clang] [clang][modules] Partially revert 48d0eb518 to fix -gmodules output (PR #124003)

2025-01-22 Thread Ben Langmuir via cfe-commits 

https://github.com/benlangmuir created 
https://github.com/llvm/llvm-project/pull/124003

With the changes in 48d0eb518, the CodeGenOptions used to emit .pcm files with 
-fmodule-format=obj (-gmodules) were the ones from the original invocation, 
rather than the ones specifically crafted for outputting the pcm. This was 
causing the pcm to be written with only the debug info and without the 
__clangast section in some cases (e.g. -O2). This unforunately was not covered 
by existing tests, because compiling and loading a module within a single 
compilation load the ast content from the in-memory module cache rather than 
reading it from the pcm file that was written.  This broke bootstrapping a 
build of clang with modules enabled on Darwin.

rdar://143418834

>From 40879d967ff8b7463f3eb56d437cacde6e2689ef Mon Sep 17 00:00:00 2001
From: Ben Langmuir 
Date: Wed, 22 Jan 2025 12:52:53 -0800
Subject: [PATCH] [clang][modules] Partially revert 48d0eb518 to fix -gmodules
 output

With the changes in 48d0eb518, the CodeGenOptions used to emit .pcm
files with -fmodule-format=obj (-gmodules) were the ones from the
original invocation, rather than the ones specifically crafted for
outputting the pcm. This was causing the pcm to be written with only the
debug info and without the __clangast section in some cases (e.g. -O2).
This unforunately was not covered by existing tests, because compiling
and loading a module within a single compilation load the ast content
from the in-memory module cache rather than reading it from the pcm file
that was written.  This broke bootstrapping a build of clang with
modules enabled on Darwin.

rdar://143418834
---
 clang/include/clang/CodeGen/BackendUtil.h  |  4 ++--
 clang/lib/CodeGen/BackendUtil.cpp  | 13 +++--
 clang/lib/CodeGen/CodeGenAction.cpp|  8 +---
 .../CodeGen/ObjectFilePCHContainerWriter.cpp   |  8 
 clang/test/Modules/gmodules-codegenopts.c  | 18 ++
 5 files changed, 36 insertions(+), 15 deletions(-)
 create mode 100644 clang/test/Modules/gmodules-codegenopts.c

diff --git a/clang/include/clang/CodeGen/BackendUtil.h 
b/clang/include/clang/CodeGen/BackendUtil.h
index 78d1e5ee8e6d59..92e0d13bf25b69 100644
--- a/clang/include/clang/CodeGen/BackendUtil.h
+++ b/clang/include/clang/CodeGen/BackendUtil.h
@@ -39,8 +39,8 @@ enum BackendAction {
   Backend_EmitObj   ///< Emit native object files
 };
 
-void emitBackendOutput(CompilerInstance &CI, StringRef TDesc, llvm::Module *M,
-   BackendAction Action,
+void emitBackendOutput(CompilerInstance &CI, CodeGenOptions &CGOpts,
+   StringRef TDesc, llvm::Module *M, BackendAction Action,
llvm::IntrusiveRefCntPtr VFS,
std::unique_ptr OS,
BackendConsumer *BC = nullptr);
diff --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index 3951ad01497cca..4bbcd22d4fc030 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -206,9 +206,10 @@ class EmitAssemblyHelper {
   }
 
 public:
-  EmitAssemblyHelper(CompilerInstance &CI, llvm::Module *M,
+  EmitAssemblyHelper(CompilerInstance &CI, CodeGenOptions &CGOpts,
+ llvm::Module *M,
  IntrusiveRefCntPtr VFS)
-  : CI(CI), Diags(CI.getDiagnostics()), CodeGenOpts(CI.getCodeGenOpts()),
+  : CI(CI), Diags(CI.getDiagnostics()), CodeGenOpts(CGOpts),
 TargetOpts(CI.getTargetOpts()), LangOpts(CI.getLangOpts()),
 TheModule(M), VFS(std::move(VFS)),
 TargetTriple(TheModule->getTargetTriple()) {}
@@ -1363,14 +1364,14 @@ runThinLTOBackend(CompilerInstance &CI, 
ModuleSummaryIndex *CombinedIndex,
   }
 }
 
-void clang::emitBackendOutput(CompilerInstance &CI, StringRef TDesc,
-  llvm::Module *M, BackendAction Action,
+void clang::emitBackendOutput(CompilerInstance &CI, CodeGenOptions &CGOpts,
+  StringRef TDesc, llvm::Module *M,
+  BackendAction Action,
   IntrusiveRefCntPtr VFS,
   std::unique_ptr OS,
   BackendConsumer *BC) {
   llvm::TimeTraceScope TimeScope("Backend");
   DiagnosticsEngine &Diags = CI.getDiagnostics();
-  const auto &CGOpts = CI.getCodeGenOpts();
 
   std::unique_ptr EmptyModule;
   if (!CGOpts.ThinLTOIndexFile.empty()) {
@@ -1410,7 +1411,7 @@ void clang::emitBackendOutput(CompilerInstance &CI, 
StringRef TDesc,
 }
   }
 
-  EmitAssemblyHelper AsmHelper(CI, M, VFS);
+  EmitAssemblyHelper AsmHelper(CI, CGOpts, M, VFS);
   AsmHelper.emitAssembly(Action, std::move(OS), BC);
 
   // Verify clang's TargetInfo DataLayout against the LLVM TargetMachine's
diff --git a/clang/lib/CodeGen/CodeGenAction.cpp 
b/clang/lib/CodeGen/CodeGenAction.cpp
index 15311fb2078101..7aa3639cabf392 100644
--- a/clang/lib/CodeGen/CodeGenAction.c