[clang-repl] Include consistency using the default clang actions. #116610
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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`.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Vassil Vassilev (vgvassilev) ChangesThis 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 Full diff: https://github.com/llvm/llvm-project/pull/116610.diff 5 Files Affected:
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());
|
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/17/builds/3994 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.