Stefan =?utf-8?q?Gränitz?= <stefan.graen...@gmail.com>, Stefan =?utf-8?q?Gränitz?= <stefan.graen...@gmail.com>, Stefan =?utf-8?q?Gränitz?= <stefan.graen...@gmail.com> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/84...@github.com>
https://github.com/vgvassilev updated https://github.com/llvm/llvm-project/pull/84758 >From 917e4e0e07c5a312543732d6101742e709a6b429 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graen...@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 b20e6efcebfd1..7bd44f8e046c0 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 6e73ed3927e81..534a54ed94fba 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 3f61ec08491d871fb6bc9c1cc03f26c33dd1df48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graen...@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 79ab8ed337ffe..d862b3354f61f 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 8b3c341480d3f3642a2c0b25456fb242523f4a1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graen...@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 b971cd550dc50..f14c2643cfae3 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 2a2185088cc4a05ddd5e225a0ff840f19e99a144 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graen...@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 970e0245417b5..1234608bb5864 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 7bd44f8e046c0..683f87e8c8c79 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 f14c2643cfae3..3651ba332124b 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()); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
