[clang] [clang-repl] Set up executor implicitly to account for init PTUs (PR #84758)
Stefan Gränitz via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 22 06:50:30 PDT 2024
https://github.com/weliveindetail updated https://github.com/llvm/llvm-project/pull/84758
>From 7ee5d29f69daf626a4fdc2fced802fe7e881f31e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Sun, 10 Mar 2024 18:17:48 +0100
Subject: [PATCH 1/4] [clang-repl] Set up executor implicitly to account for
init PTUs
---
clang/lib/Interpreter/Interpreter.cpp | 32 +++++++++++++++++++++++----
clang/test/Interpreter/execute.cpp | 4 ++--
2 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp
index b20e6efcebfd10..7bd44f8e046c02 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -229,12 +229,30 @@ IncrementalCompilerBuilder::CreateCudaHost() {
}
Interpreter::Interpreter(std::unique_ptr<CompilerInstance> CI,
- llvm::Error &Err) {
- llvm::ErrorAsOutParameter EAO(&Err);
+ llvm::Error &ErrOut) {
+ llvm::ErrorAsOutParameter EAO(&ErrOut);
auto LLVMCtx = std::make_unique<llvm::LLVMContext>();
TSCtx = std::make_unique<llvm::orc::ThreadSafeContext>(std::move(LLVMCtx));
- IncrParser = std::make_unique<IncrementalParser>(*this, std::move(CI),
- *TSCtx->getContext(), Err);
+ IncrParser = std::make_unique<IncrementalParser>(
+ *this, std::move(CI), *TSCtx->getContext(), ErrOut);
+ if (ErrOut)
+ return;
+
+ // Not all frontends support code-generation, e.g. ast-dump actions don't
+ if (IncrParser->getCodeGen()) {
+ if (llvm::Error Err = CreateExecutor()) {
+ ErrOut = joinErrors(std::move(ErrOut), std::move(Err));
+ return;
+ }
+
+ // Process the PTUs that came from initialization. For example -include will
+ // give us a header that's processed at initialization of the preprocessor.
+ for (PartialTranslationUnit &PTU : IncrParser->getPTUs())
+ if (llvm::Error Err = Execute(PTU)) {
+ ErrOut = joinErrors(std::move(ErrOut), std::move(Err));
+ return;
+ }
+ }
}
Interpreter::~Interpreter() {
@@ -395,10 +413,16 @@ llvm::Error Interpreter::CreateExecutor() {
return llvm::make_error<llvm::StringError>("Operation failed. "
"Execution engine exists",
std::error_code());
+ if (!IncrParser->getCodeGen())
+ return llvm::make_error<llvm::StringError>("Operation failed. "
+ "No code generator available",
+ std::error_code());
+
llvm::Expected<std::unique_ptr<llvm::orc::LLJITBuilder>> JB =
CreateJITBuilder(*getCompilerInstance());
if (!JB)
return JB.takeError();
+
llvm::Error Err = llvm::Error::success();
auto Executor = std::make_unique<IncrementalExecutor>(*TSCtx, **JB, Err);
if (!Err)
diff --git a/clang/test/Interpreter/execute.cpp b/clang/test/Interpreter/execute.cpp
index 6e73ed3927e815..534a54ed94fba2 100644
--- a/clang/test/Interpreter/execute.cpp
+++ b/clang/test/Interpreter/execute.cpp
@@ -7,6 +7,8 @@
// RUN: cat %s | clang-repl | FileCheck %s
// RUN: cat %s | clang-repl -Xcc -O2 | FileCheck %s
+// RUN: clang-repl -Xcc -include -Xcc %s | FileCheck %s
+// RUN: clang-repl -Xcc -fsyntax-only -Xcc -include -Xcc %s
extern "C" int printf(const char *, ...);
int i = 42;
auto r1 = printf("i = %d\n", i);
@@ -19,5 +21,3 @@ auto r2 = printf("S[f=%f, m=0x%llx]\n", s.f, reinterpret_cast<unsigned long long
inline int foo() { return 42; }
int r3 = foo();
-
-%quit
>From e375d14471cd37491863c9761397abe80c3b5047 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Mon, 11 Mar 2024 14:10:58 +0100
Subject: [PATCH 2/4] [tmp] Add crash note
---
clang/test/Interpreter/inline-virtual.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/test/Interpreter/inline-virtual.cpp b/clang/test/Interpreter/inline-virtual.cpp
index 79ab8ed337ffea..d862b3354f61fe 100644
--- a/clang/test/Interpreter/inline-virtual.cpp
+++ b/clang/test/Interpreter/inline-virtual.cpp
@@ -14,7 +14,7 @@ struct A { int a; A(int a) : a(a) {} virtual ~A(); };
// PartialTranslationUnit.
inline A::~A() { printf("~A(%d)\n", a); }
-// Create one instance with new and delete it.
+// Create one instance with new and delete it. We crash here now:
A *a1 = new A(1);
delete a1;
// CHECK: ~A(1)
>From ae9316108d8aca7d122c7e8dc4170f7445069ed4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Mon, 22 Apr 2024 14:24:33 +0200
Subject: [PATCH 3/4] [wip] Fix unittests and highlight CreateJITBuilder()
issue
The test for CreateJITBuilder() now fails with:
```
[ RUN ] InterpreterExtensionsTest.CustomCrossJIT
/home/ez/Develop/llvm-project/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp:254: Failure
Expected: (JIT) != (nullptr), actual: NULL vs (nullptr)
```
---
.../Interpreter/InterpreterExtensionsTest.cpp | 34 +++++++++----------
1 file changed, 16 insertions(+), 18 deletions(-)
diff --git a/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp b/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp
index b971cd550dc507..f14c2643cfae3b 100644
--- a/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp
+++ b/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp
@@ -108,14 +108,14 @@ TEST(InterpreterExtensionsTest, ExecutorCreateReset) {
llvm::Error ErrOut = llvm::Error::success();
TestCreateResetExecutor Interp(cantFail(CB.CreateCpp()), ErrOut);
cantFail(std::move(ErrOut));
+ EXPECT_THAT_ERROR(Interp.testCreateExecutor(),
+ llvm::FailedWithMessage("Operation failed. "
+ "Execution engine exists"));
+ Interp.resetExecutor();
EXPECT_THAT_ERROR(Interp.testCreateJITBuilderError(),
llvm::FailedWithMessage("TestError"));
cantFail(Interp.testCreateExecutor());
Interp.resetExecutor();
- cantFail(Interp.testCreateExecutor());
- EXPECT_THAT_ERROR(Interp.testCreateExecutor(),
- llvm::FailedWithMessage("Operation failed. "
- "Execution engine exists"));
}
class RecordRuntimeIBMetrics : public Interpreter {
@@ -173,18 +173,16 @@ class CustomJBInterpreter : public Interpreter {
CustomJITBuilderCreatorFunction JBCreator = nullptr;
public:
- CustomJBInterpreter(std::unique_ptr<CompilerInstance> CI, llvm::Error &ErrOut)
- : Interpreter(std::move(CI), ErrOut) {}
+ CustomJBInterpreter(std::unique_ptr<CompilerInstance> CI,
+ CustomJITBuilderCreatorFunction JBCreator,
+ llvm::Error &ErrOut)
+ : Interpreter(std::move(CI), ErrOut), JBCreator(std::move(JBCreator)) {}
~CustomJBInterpreter() override {
// Skip cleanUp() because it would trigger LLJIT default dtors
Interpreter::ResetExecutor();
}
- void setCustomJITBuilderCreator(CustomJITBuilderCreatorFunction Fn) {
- JBCreator = std::move(Fn);
- }
-
llvm::Expected<std::unique_ptr<llvm::orc::LLJITBuilder>>
CreateJITBuilder(CompilerInstance &CI) override {
if (JBCreator)
@@ -207,9 +205,8 @@ TEST(InterpreterExtensionsTest, DefaultCrossJIT) {
CB.SetTargetTriple("armv6-none-eabi");
auto CI = cantFail(CB.CreateCpp());
llvm::Error ErrOut = llvm::Error::success();
- CustomJBInterpreter Interp(std::move(CI), ErrOut);
+ CustomJBInterpreter Interp(std::move(CI), nullptr, ErrOut);
cantFail(std::move(ErrOut));
- cantFail(Interp.CreateExecutor());
}
#ifdef CLANG_INTERPRETER_PLATFORM_CANNOT_CREATE_LLJIT
@@ -225,14 +222,11 @@ TEST(InterpreterExtensionsTest, CustomCrossJIT) {
IncrementalCompilerBuilder CB;
CB.SetTargetTriple(TargetTriple);
auto CI = cantFail(CB.CreateCpp());
- llvm::Error ErrOut = llvm::Error::success();
- CustomJBInterpreter Interp(std::move(CI), ErrOut);
- cantFail(std::move(ErrOut));
using namespace llvm::orc;
LLJIT *JIT = nullptr;
std::vector<std::unique_ptr<llvm::MemoryBuffer>> Objs;
- Interp.setCustomJITBuilderCreator([&]() {
+ auto JBCreator = [&]() {
auto JTMB = JITTargetMachineBuilder(llvm::Triple(TargetTriple));
JTMB.setCPU("cortex-m0plus");
auto JB = std::make_unique<LLJITBuilder>();
@@ -249,11 +243,15 @@ TEST(InterpreterExtensionsTest, CustomCrossJIT) {
return llvm::Error::success();
});
return JB;
- });
+ };
+
+ llvm::Error ErrOut = llvm::Error::success();
+ CustomJBInterpreter Interp(std::move(CI), std::move(JBCreator), ErrOut);
+ cantFail(std::move(ErrOut));
EXPECT_EQ(0U, Objs.size());
- cantFail(Interp.CreateExecutor());
cantFail(Interp.ParseAndExecute("int a = 1;"));
+ ASSERT_NE(JIT, nullptr); // But it is, because JBCreator was never called
ExecutorAddr Addr = cantFail(JIT->lookup("a"));
EXPECT_NE(0U, Addr.getValue());
EXPECT_EQ(1U, Objs.size());
>From b60f871ec16b47568f62695002cb6dec26d8329f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Mon, 22 Apr 2024 15:49:21 +0200
Subject: [PATCH 4/4] [clang][Interpreter] Take LLJITBuilder as an argument in
the constructor
---
clang/include/clang/Interpreter/Interpreter.h | 15 +--
clang/lib/Interpreter/Interpreter.cpp | 30 ++---
.../Interpreter/InterpreterExtensionsTest.cpp | 104 ++++--------------
3 files changed, 41 insertions(+), 108 deletions(-)
diff --git a/clang/include/clang/Interpreter/Interpreter.h b/clang/include/clang/Interpreter/Interpreter.h
index 970e0245417b51..1234608bb58647 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -110,9 +110,9 @@ class Interpreter {
RuntimeInterfaceBuilder::TransformExprFunction *AddPrintValueCall = nullptr;
protected:
- // Derived classes can make use an extended interface of the Interpreter.
- // That's useful for testing and out-of-tree clients.
- Interpreter(std::unique_ptr<CompilerInstance> CI, llvm::Error &Err);
+ // 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);
// Create the internal IncrementalExecutor, or re-create it after calling
// ResetExecutor().
@@ -128,13 +128,6 @@ class Interpreter {
// custom runtime.
virtual std::unique_ptr<RuntimeInterfaceBuilder> FindRuntimeInterface();
- // Lazily construct thev ORCv2 JITBuilder. This called when the internal
- // IncrementalExecutor is created. The default implementation populates an
- // in-process JIT with debugging support. Override this to configure the JIT
- // engine used for execution.
- virtual llvm::Expected<std::unique_ptr<llvm::orc::LLJITBuilder>>
- CreateJITBuilder(CompilerInstance &CI);
-
public:
virtual ~Interpreter();
@@ -189,6 +182,8 @@ class Interpreter {
llvm::DenseMap<CXXRecordDecl *, llvm::orc::ExecutorAddr> Dtors;
llvm::SmallVector<Expr *, 4> ValuePrintingInfo;
+
+ std::unique_ptr<llvm::orc::LLJITBuilder> JITBuilder;
};
} // namespace clang
diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp
index 7bd44f8e046c02..683f87e8c8c79e 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -229,7 +229,9 @@ IncrementalCompilerBuilder::CreateCudaHost() {
}
Interpreter::Interpreter(std::unique_ptr<CompilerInstance> CI,
- llvm::Error &ErrOut) {
+ llvm::Error &ErrOut,
+ std::unique_ptr<llvm::orc::LLJITBuilder> JITBuilder)
+ : JITBuilder(std::move(JITBuilder)) {
llvm::ErrorAsOutParameter EAO(&ErrOut);
auto LLVMCtx = std::make_unique<llvm::LLVMContext>();
TSCtx = std::make_unique<llvm::orc::ThreadSafeContext>(std::move(LLVMCtx));
@@ -400,14 +402,6 @@ createJITTargetMachineBuilder(const std::string &TT) {
return llvm::orc::JITTargetMachineBuilder(llvm::Triple(TT));
}
-llvm::Expected<std::unique_ptr<llvm::orc::LLJITBuilder>>
-Interpreter::CreateJITBuilder(CompilerInstance &CI) {
- auto JTMB = createJITTargetMachineBuilder(CI.getTargetOpts().Triple);
- if (!JTMB)
- return JTMB.takeError();
- return IncrementalExecutor::createDefaultJITBuilder(std::move(*JTMB));
-}
-
llvm::Error Interpreter::CreateExecutor() {
if (IncrExecutor)
return llvm::make_error<llvm::StringError>("Operation failed. "
@@ -417,14 +411,20 @@ llvm::Error Interpreter::CreateExecutor() {
return llvm::make_error<llvm::StringError>("Operation failed. "
"No code generator available",
std::error_code());
-
- llvm::Expected<std::unique_ptr<llvm::orc::LLJITBuilder>> JB =
- CreateJITBuilder(*getCompilerInstance());
- if (!JB)
- return JB.takeError();
+ if (!JITBuilder) {
+ const std::string &TT = getCompilerInstance()->getTargetOpts().Triple;
+ auto JTMB = createJITTargetMachineBuilder(TT);
+ if (!JTMB)
+ return JTMB.takeError();
+ auto JB = IncrementalExecutor::createDefaultJITBuilder(std::move(*JTMB));
+ if (!JB)
+ return JB.takeError();
+ JITBuilder = std::move(*JB);
+ }
llvm::Error Err = llvm::Error::success();
- auto Executor = std::make_unique<IncrementalExecutor>(*TSCtx, **JB, Err);
+ auto Executor =
+ std::make_unique<IncrementalExecutor>(*TSCtx, *JITBuilder, Err);
if (!Err)
IncrExecutor = std::move(Executor);
diff --git a/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp b/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp
index f14c2643cfae3b..3651ba332124b6 100644
--- a/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp
+++ b/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp
@@ -66,58 +66,6 @@ struct LLVMInitRAII {
~LLVMInitRAII() { llvm::llvm_shutdown(); }
} LLVMInit;
-class TestCreateResetExecutor : public Interpreter {
-public:
- TestCreateResetExecutor(std::unique_ptr<CompilerInstance> CI,
- llvm::Error &Err)
- : Interpreter(std::move(CI), Err) {}
-
- llvm::Error testCreateJITBuilderError() {
- JB = nullptr;
- return Interpreter::CreateExecutor();
- }
-
- llvm::Error testCreateExecutor() {
- JB = std::make_unique<llvm::orc::LLJITBuilder>();
- return Interpreter::CreateExecutor();
- }
-
- void resetExecutor() { Interpreter::ResetExecutor(); }
-
-private:
- llvm::Expected<std::unique_ptr<llvm::orc::LLJITBuilder>>
- CreateJITBuilder(CompilerInstance &CI) override {
- if (JB)
- return std::move(JB);
- return llvm::make_error<llvm::StringError>("TestError", std::error_code());
- }
-
- std::unique_ptr<llvm::orc::LLJITBuilder> JB;
-};
-
-#ifdef CLANG_INTERPRETER_PLATFORM_CANNOT_CREATE_LLJIT
-TEST(InterpreterExtensionsTest, DISABLED_ExecutorCreateReset) {
-#else
-TEST(InterpreterExtensionsTest, ExecutorCreateReset) {
-#endif
- // Make sure we can create the executer on the platform.
- if (!HostSupportsJit())
- GTEST_SKIP();
-
- clang::IncrementalCompilerBuilder CB;
- llvm::Error ErrOut = llvm::Error::success();
- TestCreateResetExecutor Interp(cantFail(CB.CreateCpp()), ErrOut);
- cantFail(std::move(ErrOut));
- EXPECT_THAT_ERROR(Interp.testCreateExecutor(),
- llvm::FailedWithMessage("Operation failed. "
- "Execution engine exists"));
- Interp.resetExecutor();
- EXPECT_THAT_ERROR(Interp.testCreateJITBuilderError(),
- llvm::FailedWithMessage("TestError"));
- cantFail(Interp.testCreateExecutor());
- Interp.resetExecutor();
-}
-
class RecordRuntimeIBMetrics : public Interpreter {
struct NoopRuntimeInterfaceBuilder : public RuntimeInterfaceBuilder {
NoopRuntimeInterfaceBuilder(Sema &S) : S(S) {}
@@ -173,23 +121,15 @@ class CustomJBInterpreter : public Interpreter {
CustomJITBuilderCreatorFunction JBCreator = nullptr;
public:
- CustomJBInterpreter(std::unique_ptr<CompilerInstance> CI,
- CustomJITBuilderCreatorFunction JBCreator,
- llvm::Error &ErrOut)
- : Interpreter(std::move(CI), ErrOut), JBCreator(std::move(JBCreator)) {}
+ CustomJBInterpreter(std::unique_ptr<CompilerInstance> CI, llvm::Error &ErrOut,
+ std::unique_ptr<llvm::orc::LLJITBuilder> JB)
+ : Interpreter(std::move(CI), ErrOut, std::move(JB)) {}
~CustomJBInterpreter() override {
// Skip cleanUp() because it would trigger LLJIT default dtors
Interpreter::ResetExecutor();
}
- llvm::Expected<std::unique_ptr<llvm::orc::LLJITBuilder>>
- CreateJITBuilder(CompilerInstance &CI) override {
- if (JBCreator)
- return JBCreator();
- return Interpreter::CreateJITBuilder(CI);
- }
-
llvm::Error CreateExecutor() { return Interpreter::CreateExecutor(); }
};
@@ -205,7 +145,7 @@ TEST(InterpreterExtensionsTest, DefaultCrossJIT) {
CB.SetTargetTriple("armv6-none-eabi");
auto CI = cantFail(CB.CreateCpp());
llvm::Error ErrOut = llvm::Error::success();
- CustomJBInterpreter Interp(std::move(CI), nullptr, ErrOut);
+ CustomJBInterpreter Interp(std::move(CI), ErrOut, nullptr);
cantFail(std::move(ErrOut));
}
@@ -226,27 +166,25 @@ TEST(InterpreterExtensionsTest, CustomCrossJIT) {
using namespace llvm::orc;
LLJIT *JIT = nullptr;
std::vector<std::unique_ptr<llvm::MemoryBuffer>> Objs;
- auto JBCreator = [&]() {
- auto JTMB = JITTargetMachineBuilder(llvm::Triple(TargetTriple));
- JTMB.setCPU("cortex-m0plus");
- auto JB = std::make_unique<LLJITBuilder>();
- JB->setJITTargetMachineBuilder(JTMB);
- JB->setPlatformSetUp(setUpInactivePlatform);
- JB->setNotifyCreatedCallback([&](LLJIT &J) {
- ObjectLayer &ObjLayer = J.getObjLinkingLayer();
- auto *JITLinkObjLayer = llvm::dyn_cast<ObjectLinkingLayer>(&ObjLayer);
- JITLinkObjLayer->setReturnObjectBuffer(
- [&Objs](std::unique_ptr<llvm::MemoryBuffer> MB) {
- Objs.push_back(std::move(MB));
- });
- JIT = &J;
- return llvm::Error::success();
- });
- return JB;
- };
+ auto JTMB = JITTargetMachineBuilder(llvm::Triple(TargetTriple));
+ JTMB.setCPU("cortex-m0plus");
+
+ auto JB = std::make_unique<LLJITBuilder>();
+ JB->setJITTargetMachineBuilder(JTMB);
+ JB->setPlatformSetUp(setUpInactivePlatform);
+ JB->setNotifyCreatedCallback([&](LLJIT &J) {
+ ObjectLayer &ObjLayer = J.getObjLinkingLayer();
+ auto *JITLinkObjLayer = llvm::dyn_cast<ObjectLinkingLayer>(&ObjLayer);
+ JITLinkObjLayer->setReturnObjectBuffer(
+ [&Objs](std::unique_ptr<llvm::MemoryBuffer> MB) {
+ Objs.push_back(std::move(MB));
+ });
+ JIT = &J;
+ return llvm::Error::success();
+ });
llvm::Error ErrOut = llvm::Error::success();
- CustomJBInterpreter Interp(std::move(CI), std::move(JBCreator), ErrOut);
+ CustomJBInterpreter Interp(std::move(CI), ErrOut, std::move(JB));
cantFail(std::move(ErrOut));
EXPECT_EQ(0U, Objs.size());
More information about the cfe-commits
mailing list