[clang-repl] Simplify the value printing logic to enable out-of-process. #107737
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
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Vassil Vassilev (vgvassilev) ChangesThis patch improves the design of the IncrementalParser and Interpreter classes. Now the incremental parser is only responsible for building the partial translation unit declaration and the AST, while the Interpreter fills in the lower level llvm::Module and other JIT-related infrastructure. Finally the Interpreter class now orchestrates the AST and the LLVM IR with the IncrementalParser and IncrementalExecutor classes. The design improvement allows us to rework some of the logic that extracts an interpreter value into the clang::Value object. The new implementation simplifies use-cases which are used for out-of-process execution by allowing interpreter to be inherited or customized with an clang::ASTConsumer. This change will enable completing the pretty printing work which is in llvm/llvm-project#84769 Patch is 71.63 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/107737.diff 13 Files Affected:
diff --git a/clang/include/clang/Frontend/MultiplexConsumer.h b/clang/include/clang/Frontend/MultiplexConsumer.h
index 3a7670d7a51aa6..b190750bb29fb8 100644
--- a/clang/include/clang/Frontend/MultiplexConsumer.h
+++ b/clang/include/clang/Frontend/MultiplexConsumer.h
@@ -53,6 +53,7 @@ class MultiplexConsumer : public SemaConsumer {
public:
// Takes ownership of the pointers in C.
MultiplexConsumer(std::vector<std::unique_ptr<ASTConsumer>> C);
+ MultiplexConsumer(std::unique_ptr<ASTConsumer> C);
~MultiplexConsumer() override;
// ASTConsumer
@@ -80,7 +81,7 @@ class MultiplexConsumer : public SemaConsumer {
void InitializeSema(Sema &S) override;
void ForgetSema() override;
-private:
+protected:
std::vector<std::unique_ptr<ASTConsumer>> Consumers; // Owns these.
std::unique_ptr<MultiplexASTMutationListener> MutationListener;
std::unique_ptr<MultiplexASTDeserializationListener> DeserializationListener;
diff --git a/clang/include/clang/Interpreter/Interpreter.h b/clang/include/clang/Interpreter/Interpreter.h
index 1234608bb58647..cbb1cfd4ab02a8 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -14,11 +14,9 @@
#ifndef LLVM_CLANG_INTERPRETER_INTERPRETER_H
#define LLVM_CLANG_INTERPRETER_INTERPRETER_H
-#include "clang/AST/Decl.h"
#include "clang/AST/GlobalDecl.h"
#include "clang/Interpreter/PartialTranslationUnit.h"
#include "clang/Interpreter/Value.h"
-#include "clang/Sema/Ownership.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ExecutionEngine/JITSymbol.h"
@@ -38,6 +36,9 @@ class ThreadSafeContext;
namespace clang {
class CompilerInstance;
+class CodeGenerator;
+class CXXRecordDecl;
+class Decl;
class IncrementalExecutor;
class IncrementalParser;
@@ -77,26 +78,27 @@ class IncrementalCompilerBuilder {
llvm::StringRef CudaSDKPath;
};
-/// Generate glue code between the Interpreter's built-in runtime and user code.
-class RuntimeInterfaceBuilder {
-public:
- virtual ~RuntimeInterfaceBuilder() = default;
-
- using TransformExprFunction = ExprResult(RuntimeInterfaceBuilder *Builder,
- Expr *, ArrayRef<Expr *>);
- virtual TransformExprFunction *getPrintValueTransformer() = 0;
-};
+class IncrementalAction;
+class InProcessPrintingASTConsumer;
/// Provides top-level interfaces for incremental compilation and execution.
class Interpreter {
+ friend class Value;
+ friend InProcessPrintingASTConsumer;
+
std::unique_ptr<llvm::orc::ThreadSafeContext> TSCtx;
+ /// Long-lived, incremental parsing action.
+ std::unique_ptr<IncrementalAction> Act;
std::unique_ptr<IncrementalParser> IncrParser;
std::unique_ptr<IncrementalExecutor> IncrExecutor;
- std::unique_ptr<RuntimeInterfaceBuilder> RuntimeIB;
// An optional parser for CUDA offloading
std::unique_ptr<IncrementalParser> DeviceParser;
+ /// List containing every information about every incrementally parsed piece
+ /// of code.
+ std::list<PartialTranslationUnit> PTUs;
+
unsigned InitPTUSize = 0;
// This member holds the last result of the value printing. It's a class
@@ -104,15 +106,15 @@ class Interpreter {
// printing happens, it's in an invalid state.
Value LastValue;
- // Add a call to an Expr to report its result. We query the function from
- // RuntimeInterfaceBuilder once and store it as a function pointer to avoid
- // frequent virtual function calls.
- RuntimeInterfaceBuilder::TransformExprFunction *AddPrintValueCall = nullptr;
+ /// When CodeGen is created the first llvm::Module gets cached in many places
+ /// and we must keep it alive.
+ std::unique_ptr<llvm::Module> CachedInCodeGenModule;
protected:
// Derived classes can use an extended interface of the Interpreter.
Interpreter(std::unique_ptr<CompilerInstance> CI, llvm::Error &Err,
- std::unique_ptr<llvm::orc::LLJITBuilder> JITBuilder = nullptr);
+ std::unique_ptr<llvm::orc::LLJITBuilder> JITBuilder = nullptr,
+ std::unique_ptr<clang::ASTConsumer> Consumer = nullptr);
// Create the internal IncrementalExecutor, or re-create it after calling
// ResetExecutor().
@@ -122,15 +124,8 @@ class Interpreter {
// JIT engine. In particular, it doesn't run cleanup or destructors.
void ResetExecutor();
- // Lazily construct the RuntimeInterfaceBuilder. The provided instance will be
- // used for the entire lifetime of the interpreter. The default implementation
- // targets the in-process __clang_Interpreter runtime. Override this to use a
- // custom runtime.
- virtual std::unique_ptr<RuntimeInterfaceBuilder> FindRuntimeInterface();
-
public:
virtual ~Interpreter();
-
static llvm::Expected<std::unique_ptr<Interpreter>>
create(std::unique_ptr<CompilerInstance> CI);
static llvm::Expected<std::unique_ptr<Interpreter>>
@@ -145,7 +140,6 @@ class Interpreter {
llvm::Expected<PartialTranslationUnit &> Parse(llvm::StringRef Code);
llvm::Error Execute(PartialTranslationUnit &T);
llvm::Error ParseAndExecute(llvm::StringRef Code, Value *V = nullptr);
- llvm::Expected<llvm::orc::ExecutorAddr> CompileDtorCall(CXXRecordDecl *CXXRD);
/// Undo N previous incremental inputs.
llvm::Error Undo(unsigned N = 1);
@@ -167,8 +161,6 @@ class Interpreter {
llvm::Expected<llvm::orc::ExecutorAddr>
getSymbolAddressFromLinkerName(llvm::StringRef LinkerName) const;
- enum InterfaceKind { NoAlloc, WithAlloc, CopyArray, NewTag };
-
const llvm::SmallVectorImpl<Expr *> &getValuePrintingInfo() const {
return ValuePrintingInfo;
}
@@ -178,7 +170,14 @@ class Interpreter {
private:
size_t getEffectivePTUSize() const;
void markUserCodeStart();
+ llvm::Expected<Expr *> AttachValuePrinting(Expr *E);
+ llvm::Expected<llvm::orc::ExecutorAddr> CompileDtorCall(CXXRecordDecl *CXXRD);
+
+ CodeGenerator *getCodeGen() const;
+ std::unique_ptr<llvm::Module> GenModule();
+ // A cache for the compiled destructors used to for de-allocation of managed
+ // clang::Values.
llvm::DenseMap<CXXRecordDecl *, llvm::orc::ExecutorAddr> Dtors;
llvm::SmallVector<Expr *, 4> ValuePrintingInfo;
diff --git a/clang/lib/Frontend/MultiplexConsumer.cpp b/clang/lib/Frontend/MultiplexConsumer.cpp
index 2158d176d18929..3fd3c9bd69037a 100644
--- a/clang/lib/Frontend/MultiplexConsumer.cpp
+++ b/clang/lib/Frontend/MultiplexConsumer.cpp
@@ -298,6 +298,13 @@ MultiplexConsumer::MultiplexConsumer(
}
}
+MultiplexConsumer::MultiplexConsumer(std::unique_ptr<ASTConsumer> C)
+ : MultiplexConsumer([](std::unique_ptr<ASTConsumer> Consumer) {
+ std::vector<std::unique_ptr<ASTConsumer>> Consumers;
+ Consumers.push_back(std::move(Consumer));
+ return Consumers;
+ }(std::move(C))) {}
+
MultiplexConsumer::~MultiplexConsumer() {}
void MultiplexConsumer::Initialize(ASTContext &Context) {
diff --git a/clang/lib/Interpreter/CMakeLists.txt b/clang/lib/Interpreter/CMakeLists.txt
index 6a069659ebb8db..2cc7c59b61d318 100644
--- a/clang/lib/Interpreter/CMakeLists.txt
+++ b/clang/lib/Interpreter/CMakeLists.txt
@@ -22,6 +22,7 @@ add_clang_library(clangInterpreter
IncrementalExecutor.cpp
IncrementalParser.cpp
Interpreter.cpp
+ InterpreterValuePrinter.cpp
InterpreterUtils.cpp
Value.cpp
${WASM_SRC}
diff --git a/clang/lib/Interpreter/DeviceOffload.cpp b/clang/lib/Interpreter/DeviceOffload.cpp
index 07c9e3005e5fd3..4bf9ed28096893 100644
--- a/clang/lib/Interpreter/DeviceOffload.cpp
+++ b/clang/lib/Interpreter/DeviceOffload.cpp
@@ -15,6 +15,7 @@
#include "clang/Basic/TargetOptions.h"
#include "clang/CodeGen/ModuleBuilder.h"
#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Interpreter/PartialTranslationUnit.h"
#include "llvm/IR/LegacyPassManager.h"
#include "llvm/IR/Module.h"
@@ -24,11 +25,10 @@
namespace clang {
IncrementalCUDADeviceParser::IncrementalCUDADeviceParser(
- Interpreter &Interp, std::unique_ptr<CompilerInstance> Instance,
- IncrementalParser &HostParser, llvm::LLVMContext &LLVMCtx,
+ std::unique_ptr<CompilerInstance> Instance, IncrementalParser &HostParser,
llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> FS,
- llvm::Error &Err)
- : IncrementalParser(Interp, std::move(Instance), LLVMCtx, Err),
+ llvm::Error &Err, const std::list<PartialTranslationUnit> &PTUs)
+ : IncrementalParser(std::move(Instance), Err), PTUs(PTUs),
HostParser(HostParser), VFS(FS) {
if (Err)
return;
@@ -41,7 +41,7 @@ IncrementalCUDADeviceParser::IncrementalCUDADeviceParser(
}
}
-llvm::Expected<PartialTranslationUnit &>
+llvm::Expected<TranslationUnitDecl *>
IncrementalCUDADeviceParser::Parse(llvm::StringRef Input) {
auto PTU = IncrementalParser::Parse(Input);
if (!PTU)
diff --git a/clang/lib/Interpreter/DeviceOffload.h b/clang/lib/Interpreter/DeviceOffload.h
index ce4f218c94c79d..b84870474841a5 100644
--- a/clang/lib/Interpreter/DeviceOffload.h
+++ b/clang/lib/Interpreter/DeviceOffload.h
@@ -18,19 +18,19 @@
#include "llvm/Support/VirtualFileSystem.h"
namespace clang {
-
+struct PartialTranslationUnit;
class IncrementalCUDADeviceParser : public IncrementalParser {
+ const std::list<PartialTranslationUnit> &PTUs;
+
public:
IncrementalCUDADeviceParser(
- Interpreter &Interp, std::unique_ptr<CompilerInstance> Instance,
- IncrementalParser &HostParser, llvm::LLVMContext &LLVMCtx,
+ std::unique_ptr<CompilerInstance> Instance, IncrementalParser &HostParser,
llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> VFS,
- llvm::Error &Err);
+ llvm::Error &Err, const std::list<PartialTranslationUnit> &PTUs);
- llvm::Expected<PartialTranslationUnit &>
- Parse(llvm::StringRef Input) override;
+ llvm::Expected<TranslationUnitDecl *> Parse(llvm::StringRef Input) override;
- // Generate PTX for the last PTU
+ // Generate PTX for the last PTU.
llvm::Expected<llvm::StringRef> GeneratePTX();
// Generate fatbinary contents in memory
diff --git a/clang/lib/Interpreter/IncrementalExecutor.cpp b/clang/lib/Interpreter/IncrementalExecutor.cpp
index 1824a5b4570a93..4d2adecaafce74 100644
--- a/clang/lib/Interpreter/IncrementalExecutor.cpp
+++ b/clang/lib/Interpreter/IncrementalExecutor.cpp
@@ -118,4 +118,4 @@ IncrementalExecutor::getSymbolAddress(llvm::StringRef Name,
return SymOrErr->getAddress();
}
-} // end namespace clang
+} // namespace clang
diff --git a/clang/lib/Interpreter/IncrementalParser.cpp b/clang/lib/Interpreter/IncrementalParser.cpp
index b7c809c45098ca..615f61e9aec1bc 100644
--- a/clang/lib/Interpreter/IncrementalParser.cpp
+++ b/clang/lib/Interpreter/IncrementalParser.cpp
@@ -13,233 +13,33 @@
#include "IncrementalParser.h"
#include "clang/AST/DeclContextInternals.h"
-#include "clang/CodeGen/BackendUtil.h"
-#include "clang/CodeGen/CodeGenAction.h"
-#include "clang/CodeGen/ModuleBuilder.h"
#include "clang/Frontend/CompilerInstance.h"
-#include "clang/Frontend/FrontendAction.h"
-#include "clang/FrontendTool/Utils.h"
-#include "clang/Interpreter/Interpreter.h"
+#include "clang/Interpreter/PartialTranslationUnit.h"
#include "clang/Parse/Parser.h"
#include "clang/Sema/Sema.h"
-#include "llvm/Option/ArgList.h"
#include "llvm/Support/CrashRecoveryContext.h"
#include "llvm/Support/Error.h"
-#include "llvm/Support/Timer.h"
#include <sstream>
namespace clang {
-class IncrementalASTConsumer final : public ASTConsumer {
- Interpreter &Interp;
- std::unique_ptr<ASTConsumer> Consumer;
-
-public:
- IncrementalASTConsumer(Interpreter &InterpRef, std::unique_ptr<ASTConsumer> C)
- : Interp(InterpRef), Consumer(std::move(C)) {}
-
- bool HandleTopLevelDecl(DeclGroupRef DGR) override final {
- if (DGR.isNull())
- return true;
- if (!Consumer)
- return true;
-
- for (Decl *D : DGR)
- if (auto *TSD = llvm::dyn_cast<TopLevelStmtDecl>(D);
- TSD && TSD->isSemiMissing())
- TSD->setStmt(Interp.SynthesizeExpr(cast<Expr>(TSD->getStmt())));
-
- return Consumer->HandleTopLevelDecl(DGR);
- }
- void HandleTranslationUnit(ASTContext &Ctx) override final {
- Consumer->HandleTranslationUnit(Ctx);
- }
- void HandleInlineFunctionDefinition(FunctionDecl *D) override final {
- Consumer->HandleInlineFunctionDefinition(D);
- }
- void HandleInterestingDecl(DeclGroupRef D) override final {
- Consumer->HandleInterestingDecl(D);
- }
- void HandleTagDeclDefinition(TagDecl *D) override final {
- Consumer->HandleTagDeclDefinition(D);
- }
- void HandleTagDeclRequiredDefinition(const TagDecl *D) override final {
- Consumer->HandleTagDeclRequiredDefinition(D);
- }
- void HandleCXXImplicitFunctionInstantiation(FunctionDecl *D) override final {
- Consumer->HandleCXXImplicitFunctionInstantiation(D);
- }
- void HandleTopLevelDeclInObjCContainer(DeclGroupRef D) override final {
- Consumer->HandleTopLevelDeclInObjCContainer(D);
- }
- void HandleImplicitImportDecl(ImportDecl *D) override final {
- Consumer->HandleImplicitImportDecl(D);
- }
- void CompleteTentativeDefinition(VarDecl *D) override final {
- Consumer->CompleteTentativeDefinition(D);
- }
- void CompleteExternalDeclaration(DeclaratorDecl *D) override final {
- Consumer->CompleteExternalDeclaration(D);
- }
- void AssignInheritanceModel(CXXRecordDecl *RD) override final {
- Consumer->AssignInheritanceModel(RD);
- }
- void HandleCXXStaticMemberVarInstantiation(VarDecl *D) override final {
- Consumer->HandleCXXStaticMemberVarInstantiation(D);
- }
- void HandleVTable(CXXRecordDecl *RD) override final {
- Consumer->HandleVTable(RD);
- }
- ASTMutationListener *GetASTMutationListener() override final {
- return Consumer->GetASTMutationListener();
- }
- ASTDeserializationListener *GetASTDeserializationListener() override final {
- return Consumer->GetASTDeserializationListener();
- }
- void PrintStats() override final { Consumer->PrintStats(); }
- bool shouldSkipFunctionBody(Decl *D) override final {
- return Consumer->shouldSkipFunctionBody(D);
- }
- static bool classof(const clang::ASTConsumer *) { return true; }
-};
-
-/// A custom action enabling the incremental processing functionality.
-///
-/// The usual \p FrontendAction expects one call to ExecuteAction and once it
-/// sees a call to \p EndSourceFile it deletes some of the important objects
-/// such as \p Preprocessor and \p Sema assuming no further input will come.
-///
-/// \p IncrementalAction ensures it keep its underlying action's objects alive
-/// as long as the \p IncrementalParser needs them.
-///
-class IncrementalAction : public WrapperFrontendAction {
-private:
- bool IsTerminating = false;
-
-public:
- IncrementalAction(CompilerInstance &CI, llvm::LLVMContext &LLVMCtx,
- llvm::Error &Err)
- : WrapperFrontendAction([&]() {
- llvm::ErrorAsOutParameter EAO(&Err);
- std::unique_ptr<FrontendAction> Act;
- switch (CI.getFrontendOpts().ProgramAction) {
- default:
- Err = llvm::createStringError(
- std::errc::state_not_recoverable,
- "Driver initialization failed. "
- "Incremental mode for action %d is not supported",
- CI.getFrontendOpts().ProgramAction);
- return Act;
- case frontend::ASTDump:
- [[fallthrough]];
- case frontend::ASTPrint:
- [[fallthrough]];
- case frontend::ParseSyntaxOnly:
- Act = CreateFrontendAction(CI);
- break;
- case frontend::PluginAction:
- [[fallthrough]];
- case frontend::EmitAssembly:
- [[fallthrough]];
- case frontend::EmitBC:
- [[fallthrough]];
- case frontend::EmitObj:
- [[fallthrough]];
- case frontend::PrintPreprocessedInput:
- [[fallthrough]];
- case frontend::EmitLLVMOnly:
- Act.reset(new EmitLLVMOnlyAction(&LLVMCtx));
- break;
- }
- return Act;
- }()) {}
- FrontendAction *getWrapped() const { return WrappedAction.get(); }
- TranslationUnitKind getTranslationUnitKind() override {
- return TU_Incremental;
- }
-
- 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);
- }
-
- // Do not terminate after processing the input. This allows us to keep various
- // clang objects alive and to incrementally grow the current TU.
- void EndSourceFile() override {
- // The WrappedAction can be nullptr if we issued an error in the ctor.
- if (IsTerminating && getWrapped())
- WrapperFrontendAction::EndSourceFile();
- }
-
- void FinalizeAction() {
- assert(!IsTerminating && "Already finalized!");
- IsTerminating = true;
- EndSourceFile();
- }
-};
-
-CodeGenerator *IncrementalParser::getCodeGen() const {
- FrontendAction *WrappedAct = Act->getWrapped();
- if (!WrappedAct->hasIRSupport())
- return nullptr;
- return static_cast<CodeGenAction *>(WrappedAct)->getCodeGenerator();
-}
-
IncrementalParser::IncrementalParser() {}
-IncrementalParser::IncrementalParser(Interpreter &Interp,
- std::unique_ptr<CompilerInstance> Instance,
- llvm::LLVMContext &LLVMCtx,
+IncrementalParser::IncrementalParser(std::unique_ptr<CompilerInstance> Instance,
llvm::Error &Err)
: CI(std::move(Instance)) {
llvm::ErrorAsOutParameter EAO(&Err);
- Act = std::make_unique<IncrementalAction>(*CI, LLVMCtx, Err);
- if (Err)
- return;
- CI->ExecuteAction(*Act);
- if (getCodeGen())
- CachedInCodeGenModule = GenModule();
-
- std::unique_ptr<ASTConsumer> IncrConsumer =
- std::make_unique<IncrementalASTConsumer>(Interp, CI->takeASTConsumer());
- CI->setASTConsumer(std::move(IncrConsumer));
Consumer = &CI->getASTConsumer();
P.reset(
new Parser(CI->getPreprocessor(), CI->getSema(), /*SkipBodies=*/false));
P->Initialize();
-
- // An initial PTU is needed as CUDA includes some headers automatically
- auto PTU = ParseOrWrapTopLevelDecl();
- if (auto E = PTU.takeError()) {
- consumeError(std::move(E)); // FIXME
- return; // PTU.takeError();
- }
-
- if (getCodeGen()) {
- PTU->TheModule = GenModule();
- assert(PTU->TheModule && "Failed to create initial PTU");
- }
}
-IncrementalParser::~IncrementalParser() {
- P.reset();
- Act->FinalizeAction();
-}
+IncrementalParser::~IncrementalParser() { P.reset(); }
-llvm::Expected<PartialTranslationUnit &>
+llvm::Expected<TranslationUnitDecl *>
IncrementalParser::ParseOrWrapTopLevelDecl() {
// Recover resources if we crash before exiting this method.
Sema &S = CI->getSema();
@@ -247,12 +47,9 @@ IncrementalParser::ParseOrWrapTopLevelDecl() {
Sema::GlobalEagerInstantiationScope GlobalInstantiations(S, /*Enabled=*/true);
Sema::LocalEagerInstantiationScope LocalInstantiations(S);
- PTUs.emplace_back(PartialTranslationUnit());
- PartialTranslationUnit &LastPTU = PTUs.back();
// Add a new PTU.
ASTContext &C = S.getASTContext();
C.addTranslationUnitDecl();
- LastPTU.TUPart = C.getTranslationUnitDecl();
// Skip previous eof due to last incremental input.
if (P->getCurToken().is(tok::annot_repl_input_end)) {
@@ -278,9 +75,7 @@ IncrementalParser::ParseOrWrapTopLevelDecl() {
DiagnosticsEngine &Diags = getCI()->getDiagnostics();
if (Diags.hasErrorOccurred()) {
- PartialTranslationUnit MostRecentPTU = {C.getTranslationUnitDecl(),
- nullptr};
- CleanUpPTU(MostRecentPTU);
+ CleanUpPTU(C.getTranslationUnitDecl());
Diags.Reset(/*soft=*/true);
Diags.getClient()->clear();
@@ -299,10 +94,10 @@ IncrementalParser::ParseOrWrapTopLevelDecl() {
Consumer->HandleTranslationUnit(C);
- return LastPTU;
+ return C.getTranslationUnitDecl();
}
-llvm::Expected<PartialTranslationUnit &>
+llvm::Expected<TranslationUnitDecl *>
IncrementalParse...
[truncated]
|
vgvassilev
force-pushed
the
clang-repl-simplify-value-print
branch
from
d4aa06a to
2aa7527
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
vgvassilev
force-pushed
the
clang-repl-simplify-value-print
branch
from
9dab439 to
a407150
Compare
junaire
reviewed
Sep 10, 2024
AaronBallman
approved these changes
Sep 20, 2024
This patch improves the design of the IncrementalParser and Interpreter classes. Now the incremental parser is only responsible for building the partial translation unit declaration and the AST, while the Interpreter fills in the lower level llvm::Module and other JIT-related infrastructure. Finally the Interpreter class now orchestrates the AST and the LLVM IR with the IncrementalParser and IncrementalExecutor classes. The design improvement allows us to rework some of the logic that extracts an interpreter value into the clang::Value object. The new implementation simplifies use-cases which are used for out-of-process execution by allowing interpreter to be inherited or customized with an clang::ASTConsumer. This change will enable completing the pretty printing work which is in llvm#84769
vgvassilev
force-pushed
the
clang-repl-simplify-value-print
branch
from
7aef9c8 to
a2b51aa
Compare
| llvm::Error &Err) | ||
| : CI(std::move(Instance)) { | ||
| : S(Instance.getSema()) { |
There was a problem hiding this comment.
I am starting to see crashes on Windows in debug builds for this, by hitting this assertion:
assert(TheSema && "Compiler instance has no Sema object!");
in CompilerInstance::getSema(). IncrementalCUDADeviceParser is what's constructing the IncrementalParser object, and I have not determined that this commit is the one at fault.
Curiously, clang-repl is being run by llvm-lit when first starting up, which caught me by surprise. I was able to reproduce on Windows with: clang-repl --cuda
Someone at my office hours also was running into the same problem, also on Windows.
CC @vgvassilev
There was a problem hiding this comment.
Hm... perhaps we initialize the CompilerInstance (or call createSema) later? Can you check when the constructor for clang::Sema is being called for clang-repl and clang-repl --cuda respectively?
There was a problem hiding this comment.
Unfortunately, I'm at the dev meeting this week and am not at a machine capable of compiling Clang. I can try to investigate once I'm back in the office next week, but for folks with assert builds, this turns out to be surprisingly painful because we get about 30-50 dialog boxes telling us about the failed assertion when running on Windows, so if you're able to track down a fix sooner, that would be greatly appreciated.
There was a problem hiding this comment.
@argentite, could you help us with this?
There was a problem hiding this comment.
For clang-repl alone, there is no failed assertion. For clang-repl --cuda we create two IncrementalParser objects. The first one does not assert and its call stack is:
> clang-repl.exe!clang::IncrementalParser::IncrementalParser(clang::CompilerInstance & Instance, llvm::Error & Err) Line 31 C++
clang-repl.exe!std::make_unique<clang::IncrementalParser,clang::CompilerInstance &,llvm::Error &,0>(clang::CompilerInstance & <_Args_0>, llvm::Error & <_Args_1>) Line 3597 C++
clang-repl.exe!clang::Interpreter::Interpreter(std::unique_ptr<clang::CompilerInstance,std::default_delete<clang::CompilerInstance>> Instance, llvm::Error & ErrOut, std::unique_ptr<llvm::orc::LLJITBuilder,std::default_delete<llvm::orc::LLJITBuilder>> JITBuilder, std::unique_ptr<clang::ASTConsumer,std::default_delete<clang::ASTConsumer>> Consumer) Line 390 C++
clang-repl.exe!clang::Interpreter::create(std::unique_ptr<clang::CompilerInstance,std::default_delete<clang::CompilerInstance>> CI) Line 461 C++
clang-repl.exe!clang::Interpreter::createWithCUDA(std::unique_ptr<clang::CompilerInstance,std::default_delete<clang::CompilerInstance>> CI, std::unique_ptr<clang::CompilerInstance,std::default_delete<clang::CompilerInstance>> DCI) Line 489 C++
clang-repl.exe!main(int argc, const char * * argv) Line 206 C++
and the second one is:
clang-repl.exe!clang::IncrementalParser::IncrementalParser(clang::CompilerInstance & Instance, llvm::Error & Err) Line 31 C++
> clang-repl.exe!clang::IncrementalCUDADeviceParser::IncrementalCUDADeviceParser(std::unique_ptr<clang::CompilerInstance,std::default_delete<clang::CompilerInstance>> DeviceInstance, clang::CompilerInstance & HostInstance, llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> FS, llvm::Error & Err, const std::list<clang::PartialTranslationUnit,std::allocator<clang::PartialTranslationUnit>> & PTUs) Line 34 C++
clang-repl.exe!std::make_unique<clang::IncrementalCUDADeviceParser,std::unique_ptr<clang::CompilerInstance,std::default_delete<clang::CompilerInstance>>,clang::CompilerInstance &,llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> &,llvm::Error &,std::list<clang::PartialTranslationUnit,std::allocator<clang::PartialTranslationUnit>> &,0>(std::unique_ptr<clang::CompilerInstance,std::default_delete<clang::CompilerInstance>> && <_Args_0>, clang::CompilerInstance & <_Args_1>, llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> & <_Args_2>, llvm::Error & <_Args_3>, std::list<clang::PartialTranslationUnit,std::allocator<clang::PartialTranslationUnit>> & <_Args_4>) Line 3597 C++
clang-repl.exe!clang::Interpreter::createWithCUDA(std::unique_ptr<clang::CompilerInstance,std::default_delete<clang::CompilerInstance>> CI, std::unique_ptr<clang::CompilerInstance,std::default_delete<clang::CompilerInstance>> DCI) Line 494 C++
clang-repl.exe!main(int argc, const char * * argv) Line 206 C++
and this one fails because the DeviceInstance passed to IncrementalCUDADeviceParser::IncrementalCUDADeviceParser() is mostly empty, including an empty Sema pointer.
There was a problem hiding this comment.
I tried to reproduce it on my machine but I could not.
sdkrystian
added a commit
to sdkrystian/llvm-project
that referenced
this pull request
Nov 21, 2024
…of-process. (llvm#107737)" This reverts commit a72d7ee.
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 design of the IncrementalParser and Interpreter classes. Now the incremental parser is only responsible for building the partial translation unit declaration and the AST, while the Interpreter fills in the lower level llvm::Module and other JIT-related infrastructure. Finally the Interpreter class now orchestrates the AST and the LLVM IR with the IncrementalParser and IncrementalExecutor classes.
The design improvement allows us to rework some of the logic that extracts an interpreter value into the clang::Value object. The new implementation simplifies use-cases which are used for out-of-process execution by allowing interpreter to be inherited or customized with an clang::ASTConsumer.
This change will enable completing the pretty printing work which is in #84769