https://github.com/vgvassilev created https://github.com/llvm/llvm-project/pull/116610
This patch improves the code reuse of the actions system and adds several improvements for easier debugging via clang-repl --debug-only=clang-repl. The change inimproves the consistency of the TUKind when actions are handled within a WrapperFrontendAction. In this case instead of falling back to default TU_Complete, we forward to the TUKind of the ASTContext which presumably was created by the intended action. This enables the incremental infrastructure to reuse code. This patch also clones the first llvm::Module because the first PTU now can come from -include A.h and the presumption of llvm::Module being empty does not hold. The changes are a first step to fix the issues with `clang-repl --cuda`. >From 2b89c6b6cee528b45fe8276a9443f1c7cc467683 Mon Sep 17 00:00:00 2001 From: Vassil Vassilev <v.g.vassi...@gmail.com> Date: Sun, 17 Nov 2024 12:21:15 +0000 Subject: [PATCH] [clang-repl] Include consistency using the default clang actions. This patch improves the code reuse of the actions system and adds several improvements for easier debugging via clang-repl --debug-only=clang-repl. The change inimproves the consistency of the TUKind when actions are handled within a WrapperFrontendAction. In this case instead of falling back to default TU_Complete, we forward to the TUKind of the ASTContext which presumably was created by the intended action. This enables the incremental infrastructure to reuse code. This patch also clones the first llvm::Module because the first PTU now can come from -include A.h and the presumption of llvm::Module being empty does not hold. The changes are a first step to fix the issues with `clang-repl --cuda`. --- clang/include/clang/Frontend/FrontendAction.h | 8 ++- clang/include/clang/Interpreter/Interpreter.h | 3 +- .../Interpreter/PartialTranslationUnit.h | 3 + clang/lib/Interpreter/CMakeLists.txt | 3 +- clang/lib/Interpreter/Interpreter.cpp | 71 +++++++++++-------- 5 files changed, 57 insertions(+), 31 deletions(-) diff --git a/clang/include/clang/Frontend/FrontendAction.h b/clang/include/clang/Frontend/FrontendAction.h index 039f6f247b6d8c..718684a67771a2 100644 --- a/clang/include/clang/Frontend/FrontendAction.h +++ b/clang/include/clang/Frontend/FrontendAction.h @@ -21,6 +21,7 @@ #include "clang/Basic/LLVM.h" #include "clang/Basic/LangOptions.h" #include "clang/Frontend/ASTUnit.h" +#include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/FrontendOptions.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" @@ -185,7 +186,12 @@ class FrontendAction { virtual bool usesPreprocessorOnly() const = 0; /// For AST-based actions, the kind of translation unit we're handling. - virtual TranslationUnitKind getTranslationUnitKind() { return TU_Complete; } + virtual TranslationUnitKind getTranslationUnitKind() { + // The ASTContext, if exists, knows the exact TUKind of the frondend. + if (Instance && Instance->hasASTContext()) + return Instance->getASTContext().TUKind; + return TU_Complete; + } /// Does this action support use with PCH? virtual bool hasPCHSupport() const { return true; } diff --git a/clang/include/clang/Interpreter/Interpreter.h b/clang/include/clang/Interpreter/Interpreter.h index 1230a3a7016fae..b1b63aedf86aba 100644 --- a/clang/include/clang/Interpreter/Interpreter.h +++ b/clang/include/clang/Interpreter/Interpreter.h @@ -177,7 +177,8 @@ class Interpreter { CodeGenerator *getCodeGen() const; std::unique_ptr<llvm::Module> GenModule(); - PartialTranslationUnit &RegisterPTU(TranslationUnitDecl *TU); + PartialTranslationUnit &RegisterPTU(TranslationUnitDecl *TU, + std::unique_ptr<llvm::Module> M = {}); // A cache for the compiled destructors used to for de-allocation of managed // clang::Values. diff --git a/clang/include/clang/Interpreter/PartialTranslationUnit.h b/clang/include/clang/Interpreter/PartialTranslationUnit.h index bf91d559452b8a..c878e139fe70d0 100644 --- a/clang/include/clang/Interpreter/PartialTranslationUnit.h +++ b/clang/include/clang/Interpreter/PartialTranslationUnit.h @@ -31,6 +31,9 @@ struct PartialTranslationUnit { /// The llvm IR produced for the input. std::unique_ptr<llvm::Module> TheModule; + bool operator==(const PartialTranslationUnit &other) { + return other.TUPart == TUPart && other.TheModule == TheModule; + } }; } // namespace clang diff --git a/clang/lib/Interpreter/CMakeLists.txt b/clang/lib/Interpreter/CMakeLists.txt index d5ffe78251d253..df7ea82e0dada5 100644 --- a/clang/lib/Interpreter/CMakeLists.txt +++ b/clang/lib/Interpreter/CMakeLists.txt @@ -10,7 +10,8 @@ set(LLVM_LINK_COMPONENTS Support Target TargetParser - ) + TransformUtils + ) if (EMSCRIPTEN AND "lld" IN_LIST LLVM_ENABLE_PROJECTS) set(WASM_SRC Wasm.cpp) diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp index bc96da811d44cb..1859c6802c6f2c 100644 --- a/clang/lib/Interpreter/Interpreter.cpp +++ b/clang/lib/Interpreter/Interpreter.cpp @@ -50,6 +50,9 @@ #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/raw_ostream.h" #include "llvm/TargetParser/Host.h" +#include "llvm/Transforms/Utils/Cloning.h" // for CloneModule + +#define DEBUG_TYPE "clang-repl" using namespace clang; // FIXME: Figure out how to unify with namespace init_convenience from @@ -339,19 +342,8 @@ class IncrementalAction : public WrapperFrontendAction { } void ExecuteAction() override { - CompilerInstance &CI = getCompilerInstance(); - assert(CI.hasPreprocessor() && "No PP!"); - - // Use a code completion consumer? - CodeCompleteConsumer *CompletionConsumer = nullptr; - if (CI.hasCodeCompletionConsumer()) - CompletionConsumer = &CI.getCodeCompletionConsumer(); - - Preprocessor &PP = CI.getPreprocessor(); - PP.EnterMainSourceFile(); - - if (!CI.hasSema()) - CI.createSema(getTranslationUnitKind(), CompletionConsumer); + WrapperFrontendAction::ExecuteAction(); + getCompilerInstance().getSema().CurContext = nullptr; } // Do not terminate after processing the input. This allows us to keep various @@ -385,8 +377,6 @@ Interpreter::Interpreter(std::unique_ptr<CompilerInstance> Instance, return; CI->ExecuteAction(*Act); - ASTContext &C = CI->getASTContext(); - IncrParser = std::make_unique<IncrementalParser>(*CI, ErrOut); if (ErrOut) @@ -394,18 +384,22 @@ Interpreter::Interpreter(std::unique_ptr<CompilerInstance> Instance, if (getCodeGen()) { CachedInCodeGenModule = GenModule(); + // The initial PTU is filled by `-include` or by CUDA includes + // automatically. + if (!CI->getPreprocessorOpts().Includes.empty()) { + // We can't really directly pass the CachedInCodeGenModule to the Jit + // because it will steal it, causing dangling references as explained in + // Interpreter::Execute + auto M = llvm::CloneModule(*CachedInCodeGenModule); + ASTContext &C = CI->getASTContext(); + RegisterPTU(C.getTranslationUnitDecl(), std::move(M)); + } if (llvm::Error Err = CreateExecutor()) { ErrOut = joinErrors(std::move(ErrOut), std::move(Err)); return; } } - // The initial PTU is filled by `-include` or by CUDA includes automatically. - RegisterPTU(C.getTranslationUnitDecl()); - - // Prepare the IncrParser for input. - llvm::cantFail(Parse("")); - // Not all frontends support code-generation, e.g. ast-dump actions don't if (getCodeGen()) { // Process the PTUs that came from initialization. For example -include will @@ -535,14 +529,25 @@ size_t Interpreter::getEffectivePTUSize() const { return PTUs.size() - InitPTUSize; } -PartialTranslationUnit &Interpreter::RegisterPTU(TranslationUnitDecl *TU) { +PartialTranslationUnit & +Interpreter::RegisterPTU(TranslationUnitDecl *TU, + std::unique_ptr<llvm::Module> M /*={}*/) { PTUs.emplace_back(PartialTranslationUnit()); PartialTranslationUnit &LastPTU = PTUs.back(); LastPTU.TUPart = TU; - if (std::unique_ptr<llvm::Module> M = GenModule()) - LastPTU.TheModule = std::move(M); + if (!M) + M = GenModule(); + + assert((!getCodeGen() || M) && "Must have a llvm::Module at this point"); + LastPTU.TheModule = std::move(M); + LLVM_DEBUG(llvm::dbgs() << "compile-ptu " << PTUs.size() - 1 + << ": [TU=" << LastPTU.TUPart); + if (LastPTU.TheModule) + LLVM_DEBUG(llvm::dbgs() << ", M=" << LastPTU.TheModule.get() << " (" + << LastPTU.TheModule->getName() << ")"); + LLVM_DEBUG(llvm::dbgs() << "]\n"); return LastPTU; } @@ -615,6 +620,14 @@ void Interpreter::ResetExecutor() { IncrExecutor.reset(); } llvm::Error Interpreter::Execute(PartialTranslationUnit &T) { assert(T.TheModule); + LLVM_DEBUG(llvm::dbgs() + << "execute-ptu " + << ((std::find(PTUs.begin(), PTUs.end(), T) != PTUs.end()) + ? std::distance(PTUs.begin(), + std::find(PTUs.begin(), PTUs.end(), T)) + : -1) + << ": [TU=" << T.TUPart << ", M=" << T.TheModule.get() << " (" + << T.TheModule->getName() << ")]\n"); if (!IncrExecutor) { auto Err = CreateExecutor(); if (Err) @@ -723,10 +736,12 @@ std::unique_ptr<llvm::Module> Interpreter::GenModule() { // of the module which does not map well to CodeGen's design. To work this // around we created an empty module to make CodeGen happy. We should make // sure it always stays empty. - assert((!CachedInCodeGenModule || (CachedInCodeGenModule->empty() && - CachedInCodeGenModule->global_empty() && - CachedInCodeGenModule->alias_empty() && - CachedInCodeGenModule->ifunc_empty())) && + assert(((!CachedInCodeGenModule || + !getCompilerInstance()->getPreprocessorOpts().Includes.empty()) || + (CachedInCodeGenModule->empty() && + CachedInCodeGenModule->global_empty() && + CachedInCodeGenModule->alias_empty() && + CachedInCodeGenModule->ifunc_empty())) && "CodeGen wrote to a readonly module"); std::unique_ptr<llvm::Module> M(CG->ReleaseModule()); CG->StartModule("incr_module_" + std::to_string(ID++), M->getContext()); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits