[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 05:52:47 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/3] [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/3] [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/3] [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());



More information about the cfe-commits mailing list