From 9fbf5cfebcd770fbe0e453f36ee7c74809339f18 Mon Sep 17 00:00:00 2001 From: Ben Langmuir Date: Wed, 22 Jan 2025 16:24:56 -0800 Subject: [PATCH] [clang][modules] Partially revert 48d0eb518 to fix -gmodules output (#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 --- 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 78d1e5ee8e6d..92e0d13bf25b 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 f60f8672e6a0..3e65eeb3755d 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()) {} @@ -1364,14 +1365,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()) { @@ -1411,7 +1412,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 15311fb20781..7aa3639cabf3 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().getDataLayoutString(), TheModule.get(), BA, + CI.getFileManager().getVirtualFileSystemPtr(), std::move(OS)); if (OptRecordFile) OptRecordFile->keep(); diff --git a/clang/lib/CodeGen/ObjectFilePCHContainerWriter.cpp b/clang/lib/CodeGen/ObjectFilePCHContainerWriter.cpp index 02635ce235a1..788c8b932ab5 100644 --- a/clang/lib/CodeGen/ObjectFilePCHContainerWriter.cpp +++ b/clang/lib/CodeGen/ObjectFilePCHContainerWriter.cpp @@ -322,16 +322,16 @@ public: // Print the IR for the PCH container to the debug output. llvm::SmallString<0> Buffer; clang::emitBackendOutput( - CI, Ctx.getTargetInfo().getDataLayoutString(), M.get(), + CI, CodeGenOpts, Ctx.getTargetInfo().getDataLayoutString(), M.get(), BackendAction::Backend_EmitLL, FS, std::make_unique(Buffer)); llvm::dbgs() << Buffer; }); // Use the LLVM backend to emit the pch container. - clang::emitBackendOutput(CI, Ctx.getTargetInfo().getDataLayoutString(), - M.get(), BackendAction::Backend_EmitObj, FS, - std::move(OS)); + clang::emitBackendOutput(CI, CodeGenOpts, + Ctx.getTargetInfo().getDataLayoutString(), M.get(), + BackendAction::Backend_EmitObj, FS, std::move(OS)); // Free the memory for the temporary buffer. llvm::SmallVector Empty; diff --git a/clang/test/Modules/gmodules-codegenopts.c b/clang/test/Modules/gmodules-codegenopts.c new file mode 100644 index 000000000000..417bbb9cecef --- /dev/null +++ b/clang/test/Modules/gmodules-codegenopts.c @@ -0,0 +1,18 @@ +// UNSUPPORTED: target={{.*}}-zos{{.*}}, target={{.*}}-aix{{.*}} +// Check that the output from -gmodules can be loaded back by the compiler in +// the presence of certain options like optimization level that could break +// output. Note: without compiling twice the module is loaded from the in-memory +// module cache not load it from the object container. + +// RUN: rm -rf %t +// RUN: %clang_cc1 -x objective-c -fmodules -fmodule-format=obj \ +// RUN: -fimplicit-module-maps -fmodules-cache-path=%t %s \ +// RUN: -I %S/Inputs -verify -O2 + +// Compile again, confirming we can load the module. +// RUN: %clang_cc1 -x objective-c -fmodules -fmodule-format=obj \ +// RUN: -fimplicit-module-maps -fmodules-cache-path=%t %s \ +// RUN: -I %S/Inputs -verify -O2 + +@import DebugObjC; +// expected-no-diagnostics \ No newline at end of file