[clang] [clang-repl] Enable native CPU detection by default (PR #77491)

2024-01-09 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail created 
https://github.com/llvm/llvm-project/pull/77491

We can pass `-mcpu=native` to the clang driver to let it consider the host CPU 
when choosing the compile target for `clang-repl`. We can already achieve this 
behavior with `clang-repl -Xcc -mcpu=native`, but it seems like a reasonable 
default actually.

The trade-off between optimizing for a specific CPU and maximum compatibility 
often leans towards the latter for static binaries, because distributing many 
versions is cumbersome. However, when compiling at runtime, we know the exact 
target CPU and we can use that to optimize the generated code.

This patch makes a difference especially for "scattered" architectures like 
ARM. When cross-compiling for a Raspberry Pi for example, we may use a stock 
toolchain like arm-linux-gnueabihf-gcc. The resulting binary will be compatible 
with all hardware versions. This is handy, but they will all have 
`arm-linux-gnueabihf` as their host triple. Previously, this caused the clang 
driver to select triple `armv6kz-linux-gnueabihf` and CPU `arm1176jzf-s` as the 
REPL target. After this patch the default triple and CPU on Raspberry Pi 4b 
will be `armv8a-linux-gnueabihf` and `cortex-a72` respectively.

From 2a6847e9d1b2b91e7236208b3a62458a4fc7c78b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Tue, 9 Jan 2024 14:52:29 +0100
Subject: [PATCH] [clang-repl] Enable native CPU detection by default

We can pass `-mcpu=native` to the clang driver to let it consider the host CPU 
when choosing the compile target for `clang-repl`. We can already achieve this 
behavior with `clang-repl -Xcc -mcpu=native`, but it seems like a reasonable 
default actually.

The trade-off between optimizing for a specific CPU and maximum compatibility 
often leans towards the latter for static binaries, because distributing many 
versions is cumbersome. However, when compiling at runtime, we know the exact 
target CPU and we can use that to optimize the generated code.

This patch makes a difference especially for "scattered" architectures like 
ARM. When cross-compiling for a Raspberry Pi for example, we may use a stock 
toolchain like arm-linux-gnueabihf-gcc. The resulting binary will be compatible 
with all hardware versions. This is handy, but they will all have 
`arm-linux-gnueabihf` as their host triple. Previously, this caused the clang 
driver to select triple `armv6kz-linux-gnueabihf` and CPU `arm1176jzf-s` as the 
REPL target. After this patch the default triple and CPU on Raspberry Pi 4b 
will be `armv8a-linux-gnueabihf` and `cortex-a72` respectively.
---
 clang/lib/Interpreter/Interpreter.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/clang/lib/Interpreter/Interpreter.cpp 
b/clang/lib/Interpreter/Interpreter.cpp
index c9fcef5b5b5af1..734fe90d0d89b4 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -148,6 +148,7 @@ IncrementalCompilerBuilder::create(std::vector &ClangArgv) {
   // We do C++ by default; append right after argv[0] if no "-x" given
   ClangArgv.insert(ClangArgv.end(), "-Xclang");
   ClangArgv.insert(ClangArgv.end(), "-fincremental-extensions");
+  ClangArgv.insert(ClangArgv.end(), "-mcpu=native");
   ClangArgv.insert(ClangArgv.end(), "-c");
 
   // Put a dummy C++ file on to ensure there's at least one compile job for the

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Enable native CPU detection by default (PR #77491)

2024-01-09 Thread Stefan Gränitz via cfe-commits

weliveindetail wrote:

Oh and this matches the default behavior in Orc host detection btw: 
https://github.com/llvm/llvm-project/blob/release/17.x/llvm/lib/ExecutionEngine/Orc/JITTargetMachineBuilder.cpp#L35

https://github.com/llvm/llvm-project/pull/77491
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Enable native CPU detection by default (PR #77491)

2024-01-10 Thread Stefan Gränitz via cfe-commits

weliveindetail wrote:

Thanks for the quick review!

https://github.com/llvm/llvm-project/pull/77491
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Enable native CPU detection by default (PR #77491)

2024-01-10 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail closed 
https://github.com/llvm/llvm-project/pull/77491
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 1e30820 - [WinEH] Apply funclet operand bundles to nounwind intrinsics that lower to function calls in the course of IR transforms

2022-07-26 Thread Stefan Gränitz via cfe-commits

Author: Stefan Gränitz
Date: 2022-07-26T17:52:43+02:00
New Revision: 1e308204838b5edc5ffbd775896a004edb08c60a

URL: 
https://github.com/llvm/llvm-project/commit/1e308204838b5edc5ffbd775896a004edb08c60a
DIFF: 
https://github.com/llvm/llvm-project/commit/1e308204838b5edc5ffbd775896a004edb08c60a.diff

LOG: [WinEH] Apply funclet operand bundles to nounwind intrinsics that lower to 
function calls in the course of IR transforms

WinEHPrepare marks any function call from EH funclets as unreachable, if it's 
not a nounwind intrinsic or has no proper funclet bundle operand. This
affects ARC intrinsics on Windows, because they are lowered to regular function 
calls in the PreISelIntrinsicLowering pass. It caused silent binary truncations 
and crashes during unwinding with the GNUstep ObjC runtime: 
https://github.com/gnustep/libobjc2/issues/222

This patch adds a new function `llvm::IntrinsicInst::mayLowerToFunctionCall()` 
that aims to collect all affected intrinsic IDs.
* Clang CodeGen uses it to determine whether or not it must emit a funclet 
bundle operand.
* PreISelIntrinsicLowering asserts that the function returns true for all ObjC 
runtime calls it lowers.
* LLVM uses it to determine whether or not a funclet bundle operand must be 
propagated to inlined call sites.

Reviewed By: theraven

Differential Revision: https://reviews.llvm.org/D128190

Added: 
clang/test/CodeGenObjCXX/arc-exceptions-seh.mm
llvm/test/CodeGen/X86/win64-funclet-preisel-intrinsics.ll
llvm/test/Feature/OperandBundles/inliner-funclet-wineh.ll

Modified: 
clang/lib/CodeGen/CGCall.cpp
llvm/include/llvm/IR/IntrinsicInst.h
llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
llvm/lib/IR/IntrinsicInst.cpp
llvm/lib/Transforms/Utils/InlineFunction.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 104a30dd6b256..dfa78bf59c658 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -4473,17 +4473,22 @@ llvm::CallInst 
*CodeGenFunction::EmitRuntimeCall(llvm::FunctionCallee callee,
 // they are nested within.
 SmallVector
 CodeGenFunction::getBundlesForFunclet(llvm::Value *Callee) {
-  SmallVector BundleList;
   // There is no need for a funclet operand bundle if we aren't inside a
   // funclet.
   if (!CurrentFuncletPad)
-return BundleList;
-
-  // Skip intrinsics which cannot throw.
-  auto *CalleeFn = dyn_cast(Callee->stripPointerCasts());
-  if (CalleeFn && CalleeFn->isIntrinsic() && CalleeFn->doesNotThrow())
-return BundleList;
+return (SmallVector());
+
+  // Skip intrinsics which cannot throw (as long as they don't lower into
+  // regular function calls in the course of IR transformations).
+  if (auto *CalleeFn = dyn_cast(Callee->stripPointerCasts())) {
+if (CalleeFn->isIntrinsic() && CalleeFn->doesNotThrow()) {
+  auto IID = CalleeFn->getIntrinsicID();
+  if (!llvm::IntrinsicInst::mayLowerToFunctionCall(IID))
+return (SmallVector());
+}
+  }
 
+  SmallVector BundleList;
   BundleList.emplace_back("funclet", CurrentFuncletPad);
   return BundleList;
 }

diff  --git a/clang/test/CodeGenObjCXX/arc-exceptions-seh.mm 
b/clang/test/CodeGenObjCXX/arc-exceptions-seh.mm
new file mode 100644
index 0..267fbd65cc510
--- /dev/null
+++ b/clang/test/CodeGenObjCXX/arc-exceptions-seh.mm
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -emit-llvm -fobjc-arc 
-fexceptions -fobjc-exceptions -fobjc-runtime=gnustep-2.0 -o - %s | FileCheck %s
+
+// WinEH requires funclet tokens on nounwind intrinsics if they can lower to
+// regular function calls in the course of IR transformations.
+//
+// This is the case for ObjC ARC runtime intrinsics. Test that clang emits the
+// funclet tokens for llvm.objc.retain and llvm.objc.storeStrong and that they
+// refer to their catchpad's SSA value.
+
+@class Ety;
+void opaque(void);
+void test_catch_with_objc_intrinsic(void) {
+  @try {
+opaque();
+  } @catch (Ety *ex) {
+// Destroy ex when leaving catchpad. This emits calls to intrinsic 
functions
+// llvm.objc.retain and llvm.objc.storeStrong
+  }
+}
+
+// CHECK-LABEL: define{{.*}} void {{.*}}test_catch_with_objc_intrinsic
+//...
+// CHECK:   catch.dispatch:
+// CHECK-NEXT:[[CATCHSWITCH:%[0-9]+]] = catchswitch within none
+//...
+// CHECK:   catch:
+// CHECK-NEXT:[[CATCHPAD:%[0-9]+]] = catchpad within [[CATCHSWITCH]]
+// CHECK: {{%[0-9]+}} = call {{.*}} @llvm.objc.retain{{.*}} [ 
"funclet"(token [[CATCHPAD]]) ]
+// CHECK: call {{.*}} @llvm.objc.storeStrong{{.*}} [ "funclet"(token 
[[CATCHPAD]]) ]

diff  --git a/llvm/include/llvm/IR/IntrinsicInst.h 
b/llvm/include/llvm/IR/IntrinsicInst.h
index 7eaf8eaa3b023..4ff48c3669d50 100644
--- a/llvm/include/llvm/IR/IntrinsicInst.h
+++ b/llvm/include/llvm/IR/IntrinsicInst.h
@@ -84,7 +84,7 @@ class IntrinsicInst :

[clang] [clang-repl] Fix assertion failure in CleanUpPTU() (PR #85378)

2024-03-15 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail created 
https://github.com/llvm/llvm-project/pull/85378

This assertion failure is one (fortunate) symptom:
```
clang/include/clang/AST/DeclBase.h:1337: reference 
clang::DeclListNode::iterator::operator*() const: Assertion `Ptr && 
"dereferencing end() iterator"' failed.
```

Using `remove()` from `DeclContext::lookup_result` list invalidates iterators. 
It can't be used while iterating.

From 6fbd6e36133af7af008c84a1a2e44c243aed26be Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Fri, 15 Mar 2024 11:01:09 +0100
Subject: [PATCH] [clang-repl] Fix assertion failure in CleanUpPTU()

`remove()` from `DeclContext::lookup_result` list invalidates iterators. This 
assertion failure is one symptom:
```
clang/include/clang/AST/DeclBase.h:1337: reference 
clang::DeclListNode::iterator::operator*() const: Assertion `Ptr && 
"dereferencing end() iterator"' failed.
```
---
 clang/lib/Interpreter/IncrementalParser.cpp | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/clang/lib/Interpreter/IncrementalParser.cpp 
b/clang/lib/Interpreter/IncrementalParser.cpp
index 370bcbfee8b014..fdb7b686ecfdcc 100644
--- a/clang/lib/Interpreter/IncrementalParser.cpp
+++ b/clang/lib/Interpreter/IncrementalParser.cpp
@@ -13,6 +13,7 @@
 #include "IncrementalParser.h"
 
 #include "clang/AST/DeclContextInternals.h"
+#include "clang/AST/DeclarationName.h"
 #include "clang/CodeGen/BackendUtil.h"
 #include "clang/CodeGen/CodeGenAction.h"
 #include "clang/CodeGen/ModuleBuilder.h"
@@ -375,16 +376,22 @@ void IncrementalParser::CleanUpPTU(PartialTranslationUnit 
&PTU) {
   TranslationUnitDecl *MostRecentTU = PTU.TUPart;
   TranslationUnitDecl *FirstTU = MostRecentTU->getFirstDecl();
   if (StoredDeclsMap *Map = FirstTU->getPrimaryContext()->getLookupPtr()) {
-for (auto I = Map->begin(); I != Map->end(); ++I) {
-  StoredDeclsList &List = I->second;
+for (auto &&[Key, List] : *Map) {
   DeclContextLookupResult R = List.getLookupResult();
+  std::vector NamedDeclsToRemove;
+  bool RemoveAll = true;
   for (NamedDecl *D : R) {
-if (D->getTranslationUnitDecl() == MostRecentTU) {
+if (D->getTranslationUnitDecl() == MostRecentTU)
+  NamedDeclsToRemove.push_back(D);
+else
+  RemoveAll = false;
+  }
+  if (LLVM_LIKELY(RemoveAll)) {
+Map->erase(Key);
+  } else {
+for (NamedDecl *D : NamedDeclsToRemove)
   List.remove(D);
-}
   }
-  if (List.isNull())
-Map->erase(I);
 }
   }
 }

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Fix assertion failure in CleanUpPTU() (PR #85378)

2024-03-15 Thread Stefan Gränitz via cfe-commits

weliveindetail wrote:

I can reproduce it downstream in the implicit Undo after including a header 
that causes a lot of parse errors in nested headers. I am afraid it's 
unpractical to emulate in the test suite upstream.

The code is exercised in all Undo tests. They don't reproduce the issue right 
now, but it's just a matter of likeliness. I think this fix is fine without an 
additional test.

https://github.com/llvm/llvm-project/pull/85378
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Fix assertion failure in CleanUpPTU() (PR #85378)

2024-03-15 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail updated 
https://github.com/llvm/llvm-project/pull/85378

From 6fbd6e36133af7af008c84a1a2e44c243aed26be Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Fri, 15 Mar 2024 11:01:09 +0100
Subject: [PATCH 1/2] [clang-repl] Fix assertion failure in CleanUpPTU()

`remove()` from `DeclContext::lookup_result` list invalidates iterators. This 
assertion failure is one symptom:
```
clang/include/clang/AST/DeclBase.h:1337: reference 
clang::DeclListNode::iterator::operator*() const: Assertion `Ptr && 
"dereferencing end() iterator"' failed.
```
---
 clang/lib/Interpreter/IncrementalParser.cpp | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/clang/lib/Interpreter/IncrementalParser.cpp 
b/clang/lib/Interpreter/IncrementalParser.cpp
index 370bcbfee8b014..fdb7b686ecfdcc 100644
--- a/clang/lib/Interpreter/IncrementalParser.cpp
+++ b/clang/lib/Interpreter/IncrementalParser.cpp
@@ -13,6 +13,7 @@
 #include "IncrementalParser.h"
 
 #include "clang/AST/DeclContextInternals.h"
+#include "clang/AST/DeclarationName.h"
 #include "clang/CodeGen/BackendUtil.h"
 #include "clang/CodeGen/CodeGenAction.h"
 #include "clang/CodeGen/ModuleBuilder.h"
@@ -375,16 +376,22 @@ void IncrementalParser::CleanUpPTU(PartialTranslationUnit 
&PTU) {
   TranslationUnitDecl *MostRecentTU = PTU.TUPart;
   TranslationUnitDecl *FirstTU = MostRecentTU->getFirstDecl();
   if (StoredDeclsMap *Map = FirstTU->getPrimaryContext()->getLookupPtr()) {
-for (auto I = Map->begin(); I != Map->end(); ++I) {
-  StoredDeclsList &List = I->second;
+for (auto &&[Key, List] : *Map) {
   DeclContextLookupResult R = List.getLookupResult();
+  std::vector NamedDeclsToRemove;
+  bool RemoveAll = true;
   for (NamedDecl *D : R) {
-if (D->getTranslationUnitDecl() == MostRecentTU) {
+if (D->getTranslationUnitDecl() == MostRecentTU)
+  NamedDeclsToRemove.push_back(D);
+else
+  RemoveAll = false;
+  }
+  if (LLVM_LIKELY(RemoveAll)) {
+Map->erase(Key);
+  } else {
+for (NamedDecl *D : NamedDeclsToRemove)
   List.remove(D);
-}
   }
-  if (List.isNull())
-Map->erase(I);
 }
   }
 }

From fe92a1ce9bebc055d35ce91d92579e013225cc1c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Fri, 15 Mar 2024 13:46:35 +0100
Subject: [PATCH 2/2] Remove unused include and fix related comment (NFC)

---
 clang/include/clang/AST/DeclContextInternals.h | 2 +-
 clang/lib/Interpreter/IncrementalParser.cpp| 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/clang/include/clang/AST/DeclContextInternals.h 
b/clang/include/clang/AST/DeclContextInternals.h
index 903cdb7bfcc822..c4734ab5789538 100644
--- a/clang/include/clang/AST/DeclContextInternals.h
+++ b/clang/include/clang/AST/DeclContextInternals.h
@@ -205,7 +205,7 @@ class StoredDeclsList {
 Data.setPointer(Head);
   }
 
-  /// Return an array of all the decls that this list represents.
+  /// Return the list of all the decls.
   DeclContext::lookup_result getLookupResult() const {
 return DeclContext::lookup_result(Data.getPointer());
   }
diff --git a/clang/lib/Interpreter/IncrementalParser.cpp 
b/clang/lib/Interpreter/IncrementalParser.cpp
index fdb7b686ecfdcc..5eec2a2fd6d1a6 100644
--- a/clang/lib/Interpreter/IncrementalParser.cpp
+++ b/clang/lib/Interpreter/IncrementalParser.cpp
@@ -13,7 +13,6 @@
 #include "IncrementalParser.h"
 
 #include "clang/AST/DeclContextInternals.h"
-#include "clang/AST/DeclarationName.h"
 #include "clang/CodeGen/BackendUtil.h"
 #include "clang/CodeGen/CodeGenAction.h"
 #include "clang/CodeGen/ModuleBuilder.h"

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Fix assertion failure in CleanUpPTU() (PR #85378)

2024-03-15 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail edited 
https://github.com/llvm/llvm-project/pull/85378
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Fix assertion failure in CleanUpPTU() (PR #85378)

2024-03-15 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail commented:

Two minor remarks

https://github.com/llvm/llvm-project/pull/85378
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Fix assertion failure in CleanUpPTU() (PR #85378)

2024-03-15 Thread Stefan Gränitz via cfe-commits


@@ -375,16 +375,22 @@ void IncrementalParser::CleanUpPTU(PartialTranslationUnit 
&PTU) {
   TranslationUnitDecl *MostRecentTU = PTU.TUPart;
   TranslationUnitDecl *FirstTU = MostRecentTU->getFirstDecl();
   if (StoredDeclsMap *Map = FirstTU->getPrimaryContext()->getLookupPtr()) {
-for (auto I = Map->begin(); I != Map->end(); ++I) {
-  StoredDeclsList &List = I->second;
+for (auto &&[Key, List] : *Map) {
   DeclContextLookupResult R = List.getLookupResult();
+  std::vector NamedDeclsToRemove;
+  bool RemoveAll = true;
   for (NamedDecl *D : R) {
-if (D->getTranslationUnitDecl() == MostRecentTU) {
+if (D->getTranslationUnitDecl() == MostRecentTU)
+  NamedDeclsToRemove.push_back(D);
+else
+  RemoveAll = false;
+  }
+  if (LLVM_LIKELY(RemoveAll)) {
+Map->erase(Key);
+  } else {
+for (NamedDecl *D : NamedDeclsToRemove)
   List.remove(D);

weliveindetail wrote:

BTW it would have been nice to use `llvm::remove_if`, but the iterator doesn't 
support assignment unfortunately.

https://github.com/llvm/llvm-project/pull/85378
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Fix assertion failure in CleanUpPTU() (PR #85378)

2024-03-15 Thread Stefan Gränitz via cfe-commits


@@ -205,7 +205,7 @@ class StoredDeclsList {
 Data.setPointer(Head);
   }
 
-  /// Return an array of all the decls that this list represents.
+  /// Return the list of all the decls.

weliveindetail wrote:

This is not an array (otherwise we could have done erase-remove)

https://github.com/llvm/llvm-project/pull/85378
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Fix assertion failure in CleanUpPTU() (PR #85378)

2024-03-15 Thread Stefan Gränitz via cfe-commits


@@ -375,16 +375,22 @@ void IncrementalParser::CleanUpPTU(PartialTranslationUnit 
&PTU) {
   TranslationUnitDecl *MostRecentTU = PTU.TUPart;
   TranslationUnitDecl *FirstTU = MostRecentTU->getFirstDecl();
   if (StoredDeclsMap *Map = FirstTU->getPrimaryContext()->getLookupPtr()) {
-for (auto I = Map->begin(); I != Map->end(); ++I) {
-  StoredDeclsList &List = I->second;
+for (auto &&[Key, List] : *Map) {
   DeclContextLookupResult R = List.getLookupResult();
+  std::vector NamedDeclsToRemove;
+  bool RemoveAll = true;
   for (NamedDecl *D : R) {
-if (D->getTranslationUnitDecl() == MostRecentTU) {
+if (D->getTranslationUnitDecl() == MostRecentTU)
+  NamedDeclsToRemove.push_back(D);
+else
+  RemoveAll = false;
+  }
+  if (LLVM_LIKELY(RemoveAll)) {
+Map->erase(Key);
+  } else {

weliveindetail wrote:

The else branch is no one-liner. Let's keep the braces.

https://github.com/llvm/llvm-project/pull/85378
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Fix assertion failure in CleanUpPTU() (PR #85378)

2024-03-15 Thread Stefan Gränitz via cfe-commits


@@ -375,16 +375,22 @@ void IncrementalParser::CleanUpPTU(PartialTranslationUnit 
&PTU) {
   TranslationUnitDecl *MostRecentTU = PTU.TUPart;
   TranslationUnitDecl *FirstTU = MostRecentTU->getFirstDecl();
   if (StoredDeclsMap *Map = FirstTU->getPrimaryContext()->getLookupPtr()) {
-for (auto I = Map->begin(); I != Map->end(); ++I) {
-  StoredDeclsList &List = I->second;
+for (auto &&[Key, List] : *Map) {
   DeclContextLookupResult R = List.getLookupResult();
+  std::vector NamedDeclsToRemove;
+  bool RemoveAll = true;
   for (NamedDecl *D : R) {
-if (D->getTranslationUnitDecl() == MostRecentTU) {
+if (D->getTranslationUnitDecl() == MostRecentTU)
+  NamedDeclsToRemove.push_back(D);
+else
+  RemoveAll = false;
+  }
+  if (LLVM_LIKELY(RemoveAll)) {
+Map->erase(Key);
+  } else {
+for (NamedDecl *D : NamedDeclsToRemove)
   List.remove(D);
-}
   }
-  if (List.isNull())
-Map->erase(I);

weliveindetail wrote:

It happens above on `RemoveAll`. If we find that all `NamedDecl`s must be 
removed from `List`, we can just remove `List` from `Map` directly right? 
(Instead of removing all `NamedDeclsToRemove` from `List` first.)

https://github.com/llvm/llvm-project/pull/85378
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Fix Value for platforms where unqualified char is unsigned (PR #86118)

2024-03-21 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail created 
https://github.com/llvm/llvm-project/pull/86118

Signedness of unqualified `char` is unspecified and varies between platforms. 
This patch adds `Char_U` in `REPL_BUILTIN_TYPES` to account for platforms that 
default to `unsigned char`.

From e0cdd176e35d0bd255d1394eff6bee314c0e3cd6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Thu, 21 Mar 2024 14:04:10 +0100
Subject: [PATCH 1/3] Add reproducer

---
 clang/unittests/Interpreter/InterpreterTest.cpp | 5 +
 1 file changed, 5 insertions(+)

diff --git a/clang/unittests/Interpreter/InterpreterTest.cpp 
b/clang/unittests/Interpreter/InterpreterTest.cpp
index e76c0677db5ead..25f6e4f900c882 100644
--- a/clang/unittests/Interpreter/InterpreterTest.cpp
+++ b/clang/unittests/Interpreter/InterpreterTest.cpp
@@ -340,6 +340,11 @@ TEST(InterpreterTest, Value) {
   EXPECT_EQ(V1.getKind(), Value::K_Int);
   EXPECT_FALSE(V1.isManuallyAlloc());
 
+  Value V1b;
+  llvm::cantFail(Interp->ParseAndExecute("char x = 42;"));
+  llvm::cantFail(Interp->ParseAndExecute("c", &V1b));
+  EXPECT_TRUE(V1b.getKind() == Value::K_Char_S);
+
   Value V2;
   llvm::cantFail(Interp->ParseAndExecute("double y = 3.14;"));
   llvm::cantFail(Interp->ParseAndExecute("y", &V2));

From fa3d6e1973c5d2fe13c85993cf851f18e41f0f32 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Thu, 21 Mar 2024 14:05:05 +0100
Subject: [PATCH 2/3] Add missing REPL_BUILTIN_TYPE

---
 clang/include/clang/Interpreter/Value.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/clang/include/clang/Interpreter/Value.h 
b/clang/include/clang/Interpreter/Value.h
index c380cd91550def..d70e8f8719026b 100644
--- a/clang/include/clang/Interpreter/Value.h
+++ b/clang/include/clang/Interpreter/Value.h
@@ -76,6 +76,7 @@ class QualType;
   X(bool, Bool)
\
   X(char, Char_S)  
\
   X(signed char, SChar)
\
+  X(unsigned char, Char_U) 
\
   X(unsigned char, UChar)  
\
   X(short, Short)  
\
   X(unsigned short, UShort)
\

From 3ade711f907758814d13852edae71cec746fcb7b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Thu, 21 Mar 2024 14:33:50 +0100
Subject: [PATCH 3/3] Accept signed or unsigned for unqualified char in
 reproducer

---
 clang/unittests/Interpreter/InterpreterTest.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/clang/unittests/Interpreter/InterpreterTest.cpp 
b/clang/unittests/Interpreter/InterpreterTest.cpp
index 25f6e4f900c882..4b5d73769e5da7 100644
--- a/clang/unittests/Interpreter/InterpreterTest.cpp
+++ b/clang/unittests/Interpreter/InterpreterTest.cpp
@@ -343,7 +343,8 @@ TEST(InterpreterTest, Value) {
   Value V1b;
   llvm::cantFail(Interp->ParseAndExecute("char x = 42;"));
   llvm::cantFail(Interp->ParseAndExecute("c", &V1b));
-  EXPECT_TRUE(V1b.getKind() == Value::K_Char_S);
+  EXPECT_TRUE(V1b.getKind() == Value::K_Char_S ||
+  V1b.getKind() == Value::K_Char_U);
 
   Value V2;
   llvm::cantFail(Interp->ParseAndExecute("double y = 3.14;"));

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Factor out CreateJITBuilder() and allow specialization in derived classes (PR #84461)

2024-03-21 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail updated 
https://github.com/llvm/llvm-project/pull/84461

From fae2f46d25650b8480f9d3135f33a0d6532f43ca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Thu, 7 Mar 2024 23:04:22 +0100
Subject: [PATCH] [clang-repl] Add CreateJITBuilder() for specialization in
 derived classes

The LLJITBuilder interface provides a very convenient way to configure the JIT.
---
 clang/include/clang/Interpreter/Interpreter.h |   9 ++
 clang/lib/Interpreter/IncrementalExecutor.cpp |  33 ++---
 clang/lib/Interpreter/IncrementalExecutor.h   |   9 +-
 clang/lib/Interpreter/Interpreter.cpp |  26 +++-
 .../Interpreter/InterpreterExtensionsTest.cpp | 117 +-
 5 files changed, 173 insertions(+), 21 deletions(-)

diff --git a/clang/include/clang/Interpreter/Interpreter.h 
b/clang/include/clang/Interpreter/Interpreter.h
index 1dcba1ef967980..33ce4bbf5bea10 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -29,7 +29,9 @@
 
 namespace llvm {
 namespace orc {
+class JITTargetMachineBuilder;
 class LLJIT;
+class LLJITBuilder;
 class ThreadSafeContext;
 } // namespace orc
 } // namespace llvm
@@ -127,6 +129,13 @@ class Interpreter {
   // custom runtime.
   virtual std::unique_ptr 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>
+  CreateJITBuilder(CompilerInstance &CI);
+
 public:
   virtual ~Interpreter();
 
diff --git a/clang/lib/Interpreter/IncrementalExecutor.cpp 
b/clang/lib/Interpreter/IncrementalExecutor.cpp
index 40bcef94797d43..6f036107c14a9c 100644
--- a/clang/lib/Interpreter/IncrementalExecutor.cpp
+++ b/clang/lib/Interpreter/IncrementalExecutor.cpp
@@ -20,6 +20,7 @@
 #include "llvm/ExecutionEngine/Orc/Debugging/DebuggerSupport.h"
 #include "llvm/ExecutionEngine/Orc/ExecutionUtils.h"
 #include "llvm/ExecutionEngine/Orc/IRCompileLayer.h"
+#include "llvm/ExecutionEngine/Orc/JITTargetMachineBuilder.h"
 #include "llvm/ExecutionEngine/Orc/LLJIT.h"
 #include "llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h"
 #include "llvm/ExecutionEngine/Orc/TargetProcess/JITLoaderGDB.h"
@@ -36,26 +37,28 @@ LLVM_ATTRIBUTE_USED void linkComponents() {
 
 namespace clang {
 
+llvm::Expected>
+IncrementalExecutor::createDefaultJITBuilder(
+llvm::orc::JITTargetMachineBuilder JTMB) {
+  auto JITBuilder = std::make_unique();
+  JITBuilder->setJITTargetMachineBuilder(std::move(JTMB));
+  JITBuilder->setPrePlatformSetup([](llvm::orc::LLJIT &J) {
+// Try to enable debugging of JIT'd code (only works with JITLink for
+// ELF and MachO).
+consumeError(llvm::orc::enableDebuggerSupport(J));
+return llvm::Error::success();
+  });
+  return std::move(JITBuilder);
+}
+
 IncrementalExecutor::IncrementalExecutor(llvm::orc::ThreadSafeContext &TSC,
- llvm::Error &Err,
- const clang::TargetInfo &TI)
+ llvm::orc::LLJITBuilder &JITBuilder,
+ llvm::Error &Err)
 : TSCtx(TSC) {
   using namespace llvm::orc;
   llvm::ErrorAsOutParameter EAO(&Err);
 
-  auto JTMB = JITTargetMachineBuilder(TI.getTriple());
-  JTMB.addFeatures(TI.getTargetOpts().Features);
-  LLJITBuilder Builder;
-  Builder.setJITTargetMachineBuilder(JTMB);
-  Builder.setPrePlatformSetup(
-  [](LLJIT &J) {
-// Try to enable debugging of JIT'd code (only works with JITLink for
-// ELF and MachO).
-consumeError(enableDebuggerSupport(J));
-return llvm::Error::success();
-  });
-
-  if (auto JitOrErr = Builder.create())
+  if (auto JitOrErr = JITBuilder.create())
 Jit = std::move(*JitOrErr);
   else {
 Err = JitOrErr.takeError();
diff --git a/clang/lib/Interpreter/IncrementalExecutor.h 
b/clang/lib/Interpreter/IncrementalExecutor.h
index dd0a210a061415..b4347209e14fe3 100644
--- a/clang/lib/Interpreter/IncrementalExecutor.h
+++ b/clang/lib/Interpreter/IncrementalExecutor.h
@@ -23,7 +23,9 @@
 namespace llvm {
 class Error;
 namespace orc {
+class JITTargetMachineBuilder;
 class LLJIT;
+class LLJITBuilder;
 class ThreadSafeContext;
 } // namespace orc
 } // namespace llvm
@@ -44,8 +46,8 @@ class IncrementalExecutor {
 public:
   enum SymbolNameKind { IRName, LinkerName };
 
-  IncrementalExecutor(llvm::orc::ThreadSafeContext &TSC, llvm::Error &Err,
-  const clang::TargetInfo &TI);
+  IncrementalExecutor(llvm::orc::ThreadSafeContext &TSC,
+  llvm::orc::LLJITBuilder &JITBuilder, llvm::Error &Err);
   ~IncrementalExecutor();
 
   llvm::Error addModule(PartialTranslationUnit &PTU);
@@ -56,6 +58,9 @@ class IncrementalExecutor {
   getSymbolAddre

[clang] [clang-repl] Factor out CreateJITBuilder() and allow specialization in derived classes (PR #84461)

2024-03-22 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail updated 
https://github.com/llvm/llvm-project/pull/84461

From fae2f46d25650b8480f9d3135f33a0d6532f43ca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Thu, 7 Mar 2024 23:04:22 +0100
Subject: [PATCH 1/2] [clang-repl] Add CreateJITBuilder() for specialization in
 derived classes

The LLJITBuilder interface provides a very convenient way to configure the JIT.
---
 clang/include/clang/Interpreter/Interpreter.h |   9 ++
 clang/lib/Interpreter/IncrementalExecutor.cpp |  33 ++---
 clang/lib/Interpreter/IncrementalExecutor.h   |   9 +-
 clang/lib/Interpreter/Interpreter.cpp |  26 +++-
 .../Interpreter/InterpreterExtensionsTest.cpp | 117 +-
 5 files changed, 173 insertions(+), 21 deletions(-)

diff --git a/clang/include/clang/Interpreter/Interpreter.h 
b/clang/include/clang/Interpreter/Interpreter.h
index 1dcba1ef967980..33ce4bbf5bea10 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -29,7 +29,9 @@
 
 namespace llvm {
 namespace orc {
+class JITTargetMachineBuilder;
 class LLJIT;
+class LLJITBuilder;
 class ThreadSafeContext;
 } // namespace orc
 } // namespace llvm
@@ -127,6 +129,13 @@ class Interpreter {
   // custom runtime.
   virtual std::unique_ptr 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>
+  CreateJITBuilder(CompilerInstance &CI);
+
 public:
   virtual ~Interpreter();
 
diff --git a/clang/lib/Interpreter/IncrementalExecutor.cpp 
b/clang/lib/Interpreter/IncrementalExecutor.cpp
index 40bcef94797d43..6f036107c14a9c 100644
--- a/clang/lib/Interpreter/IncrementalExecutor.cpp
+++ b/clang/lib/Interpreter/IncrementalExecutor.cpp
@@ -20,6 +20,7 @@
 #include "llvm/ExecutionEngine/Orc/Debugging/DebuggerSupport.h"
 #include "llvm/ExecutionEngine/Orc/ExecutionUtils.h"
 #include "llvm/ExecutionEngine/Orc/IRCompileLayer.h"
+#include "llvm/ExecutionEngine/Orc/JITTargetMachineBuilder.h"
 #include "llvm/ExecutionEngine/Orc/LLJIT.h"
 #include "llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h"
 #include "llvm/ExecutionEngine/Orc/TargetProcess/JITLoaderGDB.h"
@@ -36,26 +37,28 @@ LLVM_ATTRIBUTE_USED void linkComponents() {
 
 namespace clang {
 
+llvm::Expected>
+IncrementalExecutor::createDefaultJITBuilder(
+llvm::orc::JITTargetMachineBuilder JTMB) {
+  auto JITBuilder = std::make_unique();
+  JITBuilder->setJITTargetMachineBuilder(std::move(JTMB));
+  JITBuilder->setPrePlatformSetup([](llvm::orc::LLJIT &J) {
+// Try to enable debugging of JIT'd code (only works with JITLink for
+// ELF and MachO).
+consumeError(llvm::orc::enableDebuggerSupport(J));
+return llvm::Error::success();
+  });
+  return std::move(JITBuilder);
+}
+
 IncrementalExecutor::IncrementalExecutor(llvm::orc::ThreadSafeContext &TSC,
- llvm::Error &Err,
- const clang::TargetInfo &TI)
+ llvm::orc::LLJITBuilder &JITBuilder,
+ llvm::Error &Err)
 : TSCtx(TSC) {
   using namespace llvm::orc;
   llvm::ErrorAsOutParameter EAO(&Err);
 
-  auto JTMB = JITTargetMachineBuilder(TI.getTriple());
-  JTMB.addFeatures(TI.getTargetOpts().Features);
-  LLJITBuilder Builder;
-  Builder.setJITTargetMachineBuilder(JTMB);
-  Builder.setPrePlatformSetup(
-  [](LLJIT &J) {
-// Try to enable debugging of JIT'd code (only works with JITLink for
-// ELF and MachO).
-consumeError(enableDebuggerSupport(J));
-return llvm::Error::success();
-  });
-
-  if (auto JitOrErr = Builder.create())
+  if (auto JitOrErr = JITBuilder.create())
 Jit = std::move(*JitOrErr);
   else {
 Err = JitOrErr.takeError();
diff --git a/clang/lib/Interpreter/IncrementalExecutor.h 
b/clang/lib/Interpreter/IncrementalExecutor.h
index dd0a210a061415..b4347209e14fe3 100644
--- a/clang/lib/Interpreter/IncrementalExecutor.h
+++ b/clang/lib/Interpreter/IncrementalExecutor.h
@@ -23,7 +23,9 @@
 namespace llvm {
 class Error;
 namespace orc {
+class JITTargetMachineBuilder;
 class LLJIT;
+class LLJITBuilder;
 class ThreadSafeContext;
 } // namespace orc
 } // namespace llvm
@@ -44,8 +46,8 @@ class IncrementalExecutor {
 public:
   enum SymbolNameKind { IRName, LinkerName };
 
-  IncrementalExecutor(llvm::orc::ThreadSafeContext &TSC, llvm::Error &Err,
-  const clang::TargetInfo &TI);
+  IncrementalExecutor(llvm::orc::ThreadSafeContext &TSC,
+  llvm::orc::LLJITBuilder &JITBuilder, llvm::Error &Err);
   ~IncrementalExecutor();
 
   llvm::Error addModule(PartialTranslationUnit &PTU);
@@ -56,6 +58,9 @@ class IncrementalExecutor {
   getSymbolA

[clang] [clang-repl] Fix Value for platforms where unqualified char is unsigned (PR #86118)

2024-03-22 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail updated 
https://github.com/llvm/llvm-project/pull/86118

From 0b7f4bc8bf57219f5f1e5ffa06c986beb16b9546 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Thu, 21 Mar 2024 14:04:10 +0100
Subject: [PATCH 1/3] Add reproducer

---
 clang/unittests/Interpreter/InterpreterTest.cpp | 5 +
 1 file changed, 5 insertions(+)

diff --git a/clang/unittests/Interpreter/InterpreterTest.cpp 
b/clang/unittests/Interpreter/InterpreterTest.cpp
index e76c0677db5ead..25f6e4f900c882 100644
--- a/clang/unittests/Interpreter/InterpreterTest.cpp
+++ b/clang/unittests/Interpreter/InterpreterTest.cpp
@@ -340,6 +340,11 @@ TEST(InterpreterTest, Value) {
   EXPECT_EQ(V1.getKind(), Value::K_Int);
   EXPECT_FALSE(V1.isManuallyAlloc());
 
+  Value V1b;
+  llvm::cantFail(Interp->ParseAndExecute("char x = 42;"));
+  llvm::cantFail(Interp->ParseAndExecute("c", &V1b));
+  EXPECT_TRUE(V1b.getKind() == Value::K_Char_S);
+
   Value V2;
   llvm::cantFail(Interp->ParseAndExecute("double y = 3.14;"));
   llvm::cantFail(Interp->ParseAndExecute("y", &V2));

From eace13b5c95680feeab7792eab675e8dd56a56ee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Thu, 21 Mar 2024 14:05:05 +0100
Subject: [PATCH 2/3] Add missing REPL_BUILTIN_TYPE

---
 clang/include/clang/Interpreter/Value.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/clang/include/clang/Interpreter/Value.h 
b/clang/include/clang/Interpreter/Value.h
index c380cd91550def..d70e8f8719026b 100644
--- a/clang/include/clang/Interpreter/Value.h
+++ b/clang/include/clang/Interpreter/Value.h
@@ -76,6 +76,7 @@ class QualType;
   X(bool, Bool)
\
   X(char, Char_S)  
\
   X(signed char, SChar)
\
+  X(unsigned char, Char_U) 
\
   X(unsigned char, UChar)  
\
   X(short, Short)  
\
   X(unsigned short, UShort)
\

From 7db25a9f186818297d3ec0e7d3deff67e4549927 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Thu, 21 Mar 2024 14:33:50 +0100
Subject: [PATCH 3/3] Accept signed or unsigned for unqualified char in
 reproducer

---
 clang/unittests/Interpreter/InterpreterTest.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/clang/unittests/Interpreter/InterpreterTest.cpp 
b/clang/unittests/Interpreter/InterpreterTest.cpp
index 25f6e4f900c882..69bc2da242884e 100644
--- a/clang/unittests/Interpreter/InterpreterTest.cpp
+++ b/clang/unittests/Interpreter/InterpreterTest.cpp
@@ -341,9 +341,10 @@ TEST(InterpreterTest, Value) {
   EXPECT_FALSE(V1.isManuallyAlloc());
 
   Value V1b;
-  llvm::cantFail(Interp->ParseAndExecute("char x = 42;"));
+  llvm::cantFail(Interp->ParseAndExecute("char c = 42;"));
   llvm::cantFail(Interp->ParseAndExecute("c", &V1b));
-  EXPECT_TRUE(V1b.getKind() == Value::K_Char_S);
+  EXPECT_TRUE(V1b.getKind() == Value::K_Char_S ||
+  V1b.getKind() == Value::K_Char_U);
 
   Value V2;
   llvm::cantFail(Interp->ParseAndExecute("double y = 3.14;"));

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Fix Value for platforms where unqualified char is unsigned (PR #86118)

2024-03-22 Thread Stefan Gränitz via cfe-commits

weliveindetail wrote:

Let's give the pre-merge checks a 2nd chance..

https://github.com/llvm/llvm-project/pull/86118
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Fix Value for platforms where unqualified char is unsigned (PR #86118)

2024-03-25 Thread Stefan Gränitz via cfe-commits

weliveindetail wrote:

Windows buildbot failure is unrelated. This code is covered in unittest 
ClangReplInterpreterTests and it passed.

https://github.com/llvm/llvm-project/pull/86118
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Fix Value for platforms where unqualified char is unsigned (PR #86118)

2024-03-25 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail closed 
https://github.com/llvm/llvm-project/pull/86118
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Factor out CreateJITBuilder() and allow specialization in derived classes (PR #84461)

2024-03-25 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail closed 
https://github.com/llvm/llvm-project/pull/84461
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Factor out CreateJITBuilder() and allow specialization in derived classes (PR #84461)

2024-03-25 Thread Stefan Gränitz via cfe-commits

weliveindetail wrote:

Thanks for your note. Looks like the problem is that the ARM target is not 
registered. It's an uncommon requirement for a unitttest.. Will see how to 
check that at runtime. If you have an idea, let me know. Thanks

https://github.com/llvm/llvm-project/pull/84461
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Factor out CreateJITBuilder() and allow specialization in derived classes (PR #84461)

2024-03-25 Thread Stefan Gränitz via cfe-commits

weliveindetail wrote:

I will try this 
https://github.com/llvm/llvm-project/blob/release/18.x/llvm/unittests/DebugInfo/LogicalView/CodeViewReaderTest.cpp#L482
 and push a quick-fix.

https://github.com/llvm/llvm-project/pull/84461
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 13078cb - [clang-repl] Skip cross-JIT tests if specified target is not available (#84461)

2024-03-25 Thread Stefan Gränitz via cfe-commits

Author: Stefan Gränitz
Date: 2024-03-25T11:50:21+01:00
New Revision: 13078cbc3eeb0ae91c370ce0f604f7165b26e0c8

URL: 
https://github.com/llvm/llvm-project/commit/13078cbc3eeb0ae91c370ce0f604f7165b26e0c8
DIFF: 
https://github.com/llvm/llvm-project/commit/13078cbc3eeb0ae91c370ce0f604f7165b26e0c8.diff

LOG: [clang-repl] Skip cross-JIT tests if specified target is not available 
(#84461)

Added: 


Modified: 
clang/unittests/Interpreter/InterpreterExtensionsTest.cpp

Removed: 




diff  --git a/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp 
b/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp
index 8bc429d9ec2d7d..1ba865a79ed778 100644
--- a/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp
+++ b/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp
@@ -19,6 +19,7 @@
 
 #include "llvm/ExecutionEngine/Orc/LLJIT.h"
 #include "llvm/ExecutionEngine/Orc/Shared/ExecutorAddress.h"
+#include "llvm/MC/TargetRegistry.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/Threading.h"
@@ -44,14 +45,23 @@ static bool HostSupportsJit() {
   return false;
 }
 
+// Some tests require a arm-registered-target
+static bool IsARMTargetRegistered() {
+  llvm::Triple TT;
+  TT.setArch(llvm::Triple::arm);
+  TT.setVendor(llvm::Triple::UnknownVendor);
+  TT.setOS(llvm::Triple::UnknownOS);
+
+  std::string UnusedErr;
+  return llvm::TargetRegistry::lookupTarget(TT.str(), UnusedErr);
+}
+
 struct LLVMInitRAII {
   LLVMInitRAII() {
-llvm::InitializeNativeTarget();
-llvm::InitializeNativeTargetAsmPrinter();
-LLVMInitializeARMTarget();
-LLVMInitializeARMTargetInfo();
-LLVMInitializeARMTargetMC();
-LLVMInitializeARMAsmPrinter();
+llvm::InitializeAllTargets();
+llvm::InitializeAllTargetInfos();
+llvm::InitializeAllTargetMCs();
+llvm::InitializeAllAsmPrinters();
   }
   ~LLVMInitRAII() { llvm::llvm_shutdown(); }
 } LLVMInit;
@@ -190,6 +200,9 @@ TEST(InterpreterExtensionsTest, DISABLED_DefaultCrossJIT) {
 #else
 TEST(InterpreterExtensionsTest, DefaultCrossJIT) {
 #endif
+  if (!IsARMTargetRegistered())
+GTEST_SKIP();
+
   IncrementalCompilerBuilder CB;
   CB.SetTargetTriple("armv6-none-eabi");
   auto CI = cantFail(CB.CreateCpp());
@@ -204,6 +217,9 @@ TEST(InterpreterExtensionsTest, DISABLED_CustomCrossJIT) {
 #else
 TEST(InterpreterExtensionsTest, CustomCrossJIT) {
 #endif
+  if (!IsARMTargetRegistered())
+GTEST_SKIP();
+
   std::string TargetTriple = "armv6-none-eabi";
 
   IncrementalCompilerBuilder CB;



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Factor out CreateJITBuilder() and allow specialization in derived classes (PR #84461)

2024-03-25 Thread Stefan Gränitz via cfe-commits

weliveindetail wrote:

We do need the target and not just the symbols. It could be any non-native 
target, but implementing a selection mechanism isn't worth the effort. We just 
try ARM and (with my above patch) otherwise skip the test.

https://github.com/llvm/llvm-project/pull/84461
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Factor out CreateJITBuilder() and allow specialization in derived classes (PR #84461)

2024-03-25 Thread Stefan Gränitz via cfe-commits

weliveindetail wrote:

@amy-kwan Thanks for your note and sorry for the long delay. There were so many 
unrelated buildbot failures today, that I didn't catch this one. I think it was 
fixed meanwhile with 
https://github.com/llvm/llvm-project/commit/cb994d41c3afb2bd0b25a4c5b2ac48978bf1b23d
 thanks to @kparzysz!

https://github.com/llvm/llvm-project/pull/84461
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Fix assertion failure in CleanUpPTU() (PR #85378)

2024-03-26 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail closed 
https://github.com/llvm/llvm-project/pull/85378
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Expose markUserCodeStart() in extended Interpreter interface (PR #87064)

2024-03-29 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail created 
https://github.com/llvm/llvm-project/pull/87064

Adding code for built-in functionality during initialization is very common. 
Call this function afterwards to hide it from Undo. Any serious interpreter 
needs it.

From e924da63f1e926f7a7008f661730b1fd478818c6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Tue, 26 Mar 2024 15:31:04 +0100
Subject: [PATCH] [clang-repl] Expose markUserCodeStart() in extended
 Interpreter interface

---
 clang/include/clang/Interpreter/Interpreter.h |  7 --
 .../Interpreter/InterpreterExtensionsTest.cpp | 22 +++
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/clang/include/clang/Interpreter/Interpreter.h 
b/clang/include/clang/Interpreter/Interpreter.h
index 970e0245417b51..b21a95009918dc 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -44,7 +44,7 @@ class IncrementalParser;
 /// Create a pre-configured \c CompilerInstance for incremental processing.
 class IncrementalCompilerBuilder {
 public:
-  IncrementalCompilerBuilder() {}
+  IncrementalCompilerBuilder() = default;
 
   void SetCompilerArgs(const std::vector &Args) {
 UserArgs = Args;
@@ -122,6 +122,10 @@ class Interpreter {
   // JIT engine. In particular, it doesn't run cleanup or destructors.
   void ResetExecutor();
 
+  // Adding code for built-in functionality during initialization is very
+  // common. Call this function afterwards to hide it from Undo.
+  void markUserCodeStart();
+
   // 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
@@ -184,7 +188,6 @@ class Interpreter {
 
 private:
   size_t getEffectivePTUSize() const;
-  void markUserCodeStart();
 
   llvm::DenseMap Dtors;
 
diff --git a/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp 
b/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp
index 1ba865a79ed778..bb39946a31a1d3 100644
--- a/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp
+++ b/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp
@@ -66,10 +66,9 @@ struct LLVMInitRAII {
   ~LLVMInitRAII() { llvm::llvm_shutdown(); }
 } LLVMInit;
 
-class TestCreateResetExecutor : public Interpreter {
+class TestInterpreter : public Interpreter {
 public:
-  TestCreateResetExecutor(std::unique_ptr CI,
-  llvm::Error &Err)
+  TestInterpreter(std::unique_ptr CI, llvm::Error &Err)
   : Interpreter(std::move(CI), Err) {}
 
   llvm::Error testCreateJITBuilderError() {
@@ -83,6 +82,7 @@ class TestCreateResetExecutor : public Interpreter {
   }
 
   void resetExecutor() { Interpreter::ResetExecutor(); }
+  void markUserCodeStart() { Interpreter::markUserCodeStart(); }
 
 private:
   llvm::Expected>
@@ -95,6 +95,20 @@ class TestCreateResetExecutor : public Interpreter {
   std::unique_ptr JB;
 };
 
+TEST(InterpreterExtensionsTest, MarkUserCodeStart) {
+  clang::IncrementalCompilerBuilder CB;
+  llvm::Error ErrOut = llvm::Error::success();
+  TestInterpreter Interp(cantFail(CB.CreateCpp()), ErrOut);
+  cantFail(std::move(ErrOut));
+  llvm::Error Parse = Interp.Parse("int builtin = 42;").takeError();
+  EXPECT_THAT_ERROR(std::move(Parse), llvm::Succeeded());
+  // Hide above PTU from Undo
+  Interp.markUserCodeStart();
+  llvm::Error Undo = Interp.Undo(1);
+  EXPECT_THAT_ERROR(std::move(Undo), llvm::FailedWithMessage(
+ "Operation failed. Too many undos"));
+}
+
 #ifdef CLANG_INTERPRETER_PLATFORM_CANNOT_CREATE_LLJIT
 TEST(InterpreterExtensionsTest, DISABLED_ExecutorCreateReset) {
 #else
@@ -106,7 +120,7 @@ TEST(InterpreterExtensionsTest, ExecutorCreateReset) {
 
   clang::IncrementalCompilerBuilder CB;
   llvm::Error ErrOut = llvm::Error::success();
-  TestCreateResetExecutor Interp(cantFail(CB.CreateCpp()), ErrOut);
+  TestInterpreter Interp(cantFail(CB.CreateCpp()), ErrOut);
   cantFail(std::move(ErrOut));
   EXPECT_THAT_ERROR(Interp.testCreateJITBuilderError(),
 llvm::FailedWithMessage("TestError"));

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Minor cleanups in Value.cpp (NFC) (PR #87066)

2024-03-29 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail created 
https://github.com/llvm/llvm-project/pull/87066

None

From 1835e627203ece2e81787d6167ccc15b62c31f36 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Fri, 29 Mar 2024 13:37:36 +0100
Subject: [PATCH] [clang-repl] Minor cleanups in Value.cpp (NFC)

---
 clang/lib/Interpreter/Value.cpp | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/clang/lib/Interpreter/Value.cpp b/clang/lib/Interpreter/Value.cpp
index 1d6b2da087e9fb..eb2ce9c9fd3302 100644
--- a/clang/lib/Interpreter/Value.cpp
+++ b/clang/lib/Interpreter/Value.cpp
@@ -1,4 +1,4 @@
-//===--- Interpreter.h - Incremental Compiation and Execution---*- C++ -*-===//
+//=== Value.cpp - Definition of interpreter value 
-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -22,8 +22,6 @@
 #include 
 #include 
 
-using namespace clang;
-
 namespace {
 
 // This is internal buffer maintained by Value, used to hold temporaries.
@@ -61,7 +59,7 @@ class ValueStorage {
   void Release() {
 assert(RefCnt > 0 && "Can't release if reference count is already zero");
 if (--RefCnt == 0) {
-  // We hace a non-trivial dtor.
+  // We have a non-trivial dtor.
   if (Dtor && IsAlive()) {
 assert(Elements && "We at least should have 1 element in Value");
 size_t Stride = AllocSize / Elements;
@@ -97,6 +95,8 @@ class ValueStorage {
 };
 } // namespace
 
+namespace clang {
+
 static Value::Kind ConvertQualTypeToKind(const ASTContext &Ctx, QualType QT) {
   if (Ctx.hasSameType(QT, Ctx.VoidTy))
 return Value::K_Void;
@@ -265,3 +265,5 @@ void Value::print(llvm::raw_ostream &Out) const {
   assert(OpaqueType != nullptr && "Can't print default Value");
   Out << "Not implement yet.\n";
 }
+
+} // namespace clang

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Expose markUserCodeStart() in extended Interpreter interface (PR #87064)

2024-03-30 Thread Stefan Gränitz via cfe-commits

weliveindetail wrote:

Possible, but might add limitations. The init process can be complex. I could 
imagine a runtime to require actual execution of code (and not only parsing as 
we'd implement it right now I guess) or load a dynamic library. The patch here 
allows that, the constructor approach wouldn't.

https://github.com/llvm/llvm-project/pull/87064
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Minor cleanups in Value.cpp (NFC) (PR #87066)

2024-03-30 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail closed 
https://github.com/llvm/llvm-project/pull/87066
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [compiler-rt] [llvm] [clang-repl] [ORC] Add support for out-of-process execution on ELF (PR #79936)

2024-02-07 Thread Stefan Gränitz via cfe-commits

weliveindetail wrote:

Thanks for sharing your patch @jameshu15869!

For the moment it seems like a mix of many moving pieces. I think it would be 
good to review them isolation, at least ORC runtime support and ELFNix platform 
changes. All the RPC utilities could go into a separate cpp right? We may even 
think about lifting them to llvm::support at some point, because we already 
have copies of it 
[here](https://github.com/llvm/llvm-project/blob/release/18.x/llvm/examples/OrcV2Examples/LLJITWithRemoteDebugging/RemoteJITUtils.cpp)
 and 
[here](https://github.com/llvm/llvm-project/blob/release/18.x/llvm/tools/llvm-jitlink/llvm-jitlink.cpp#L723).

I didn't review in detail, but I think we need to initiate the connection to 
the executor much earlier. In particular, we must setup the [frontend compiler 
instance](https://github.com/llvm/llvm-project/blob/release/18.x/clang/tools/clang-repl/ClangRepl.cpp#L185)
 with the target triple we get from the executor here:
https://github.com/llvm/llvm-project/blob/release/18.x/llvm/include/llvm/ExecutionEngine/Orc/Shared/SimpleRemoteEPCUtils.h#L46

Apart from that, how scalable is the `Interpreter::createWithXY()` approach? I 
haven't been involved in the design discussions, but it seems much more 
convenient to use the `LLJITBuilder` interface for that. Why not pass it as an 
argument to the `IncrementalExecutor` ctor instead of `clang::TargetInfo`?

https://github.com/llvm/llvm-project/pull/79936
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Names declared in if conditions and for-init statements are local to the inner context (C++ 3.3.2p4) (PR #84150)

2024-03-06 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail created 
https://github.com/llvm/llvm-project/pull/84150

A `TopLevelStmtDecl` must be a `DeclContext` itself to avoid variable 
definitions to be added to the outer `TranslationUnitDecl` context. 
Additionally, `ActOnForStmt()` requires a `CompoundScope` when processing a 
`NullStmt` body.

From 174261f83c5c19327c751cdc7f96ef81563a8779 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Wed, 28 Feb 2024 15:45:53 +0100
Subject: [PATCH 1/7] Add test cases for variable definitions inside for-init
 and condition statements

Interpreter/execute-stmts.cpp fails with:
```
i = 2
i = 5
i = 5
i = 5
j = 4
error: Parsing failed.
error: Parsing failed.

error: 'expected-error' diagnostics seen but not expected:
  Line 1: redefinition of 'i'
  Line 1: redefinition of 'i'
error: 'expected-note' diagnostics seen but not expected:
  Line 1: previous definition is here
  Line 1: previous definition is here
```
---
 clang/test/Interpreter/execute-stmts.cpp | 8 
 1 file changed, 8 insertions(+)

diff --git a/clang/test/Interpreter/execute-stmts.cpp 
b/clang/test/Interpreter/execute-stmts.cpp
index 2d4c17e0c91e66..34a2a35c0027d8 100644
--- a/clang/test/Interpreter/execute-stmts.cpp
+++ b/clang/test/Interpreter/execute-stmts.cpp
@@ -9,6 +9,8 @@
 //CODEGEN-CHECK-COUNT-2: define internal void @__stmts__
 //CODEGEN-CHECK-NOT: define internal void @__stmts__
 
+// New tests fail right now
+// XFAIL: *
 
 extern "C" int printf(const char*,...);
 
@@ -41,3 +43,9 @@ for (; i > 4; --i) { printf("i = %d\n", i); };
 
 int j = i; printf("j = %d\n", j);
 // CHECK-NEXT: j = 4
+
+if (int i = j) printf("i = %d\n", i);
+// CHECK-NEXT: i = 4
+
+for (int i = j; i > 3; --i) printf("i = %d\n", i);
+// CHECK-NEXT: i = 4

From 2787073dea787fe081986818ed70d42f332dab8e Mon Sep 17 00:00:00 2001
From: Vassil Vassilev 
Date: Mon, 4 Mar 2024 17:51:43 +
Subject: [PATCH 2/7] Try to fix llvm/llvm-project#83028

---
 clang/include/clang/AST/Decl.h   | 16 ++--
 clang/include/clang/AST/DeclBase.h   |  1 +
 clang/include/clang/Basic/DeclNodes.td   |  2 +-
 clang/include/clang/Sema/Sema.h  |  3 ++-
 clang/lib/AST/Decl.cpp   | 11 ---
 clang/lib/AST/DeclBase.cpp   |  1 +
 clang/lib/Parse/ParseDecl.cpp| 24 
 clang/lib/Sema/SemaDecl.cpp  | 13 ++---
 clang/test/Interpreter/execute-stmts.cpp | 10 ++
 9 files changed, 59 insertions(+), 22 deletions(-)

diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 61117cc5ce71f9..bca9d42c7e0586 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -4419,7 +4419,7 @@ class FileScopeAsmDecl : public Decl {
 ///
 /// \note This is used in libInterpreter, clang -cc1 -fincremental-extensions
 /// and in tools such as clang-repl.
-class TopLevelStmtDecl : public Decl {
+class TopLevelStmtDecl : public Decl, public DeclContext {
   friend class ASTDeclReader;
   friend class ASTDeclWriter;
 
@@ -4427,7 +4427,7 @@ class TopLevelStmtDecl : public Decl {
   bool IsSemiMissing = false;
 
   TopLevelStmtDecl(DeclContext *DC, SourceLocation L, Stmt *S)
-  : Decl(TopLevelStmt, DC, L), Statement(S) {}
+: Decl(TopLevelStmt, DC, L), DeclContext(TopLevelStmt), Statement(S) {}
 
   virtual void anchor();
 
@@ -4438,15 +4438,19 @@ class TopLevelStmtDecl : public Decl {
   SourceRange getSourceRange() const override LLVM_READONLY;
   Stmt *getStmt() { return Statement; }
   const Stmt *getStmt() const { return Statement; }
-  void setStmt(Stmt *S) {
-assert(IsSemiMissing && "Operation supported for printing values only!");
-Statement = S;
-  }
+  void setStmt(Stmt *S);
   bool isSemiMissing() const { return IsSemiMissing; }
   void setSemiMissing(bool Missing = true) { IsSemiMissing = Missing; }
 
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
   static bool classofKind(Kind K) { return K == TopLevelStmt; }
+
+  static DeclContext *castToDeclContext(const TopLevelStmtDecl *D) {
+return static_cast(const_cast(D));
+  }
+  static TopLevelStmtDecl *castFromDeclContext(const DeclContext *DC) {
+return static_cast(const_cast(DC));
+  }
 };
 
 /// Represents a block literal declaration, which is like an
diff --git a/clang/include/clang/AST/DeclBase.h 
b/clang/include/clang/AST/DeclBase.h
index 9a4736019d1b1b..76810a86a78a46 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -2120,6 +2120,7 @@ class DeclContext {
 case Decl::Block:
 case Decl::Captured:
 case Decl::ObjCMethod:
+case Decl::TopLevelStmt:
   return true;
 default:
   return getDeclKind() >= Decl::firstFunction &&
diff --git a/clang/include/clang/Basic/DeclNodes.td 
b/clang/include/clang/Basic/DeclNodes.td
index 8b1f415dd5fe2c..48396e85c5adac 100644
--- a/clang/include/clang/Basic/DeclNodes.td
+++ b/clang/include/clang/

[clang] [clang-repl] Names declared in if conditions and for-init statements are local to the inner context (C++ 3.3.2p4) (PR #84150)

2024-03-06 Thread Stefan Gränitz via cfe-commits

weliveindetail wrote:

This fixes https://github.com/llvm/llvm-project/issues/83028 which provides 
context on investigation.

https://github.com/llvm/llvm-project/pull/84150
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Names declared in if conditions and for-init statements are local to the inner context (C++ 3.3.2p4) (PR #84150)

2024-03-06 Thread Stefan Gränitz via cfe-commits


@@ -5676,24 +5676,32 @@ Parser::DeclGroupPtrTy Parser::ParseTopLevelStmtDecl() {
   // Parse a top-level-stmt.
   Parser::StmtVector Stmts;
   ParsedStmtContext SubStmtCtx = ParsedStmtContext();
-  Actions.PushFunctionScope();
+  ParseScope FnScope(this, Scope::FnScope | Scope::DeclScope |

weliveindetail wrote:

I tested without it the `ParseScope` and got a segfault, so I assume we can not 
drop it (without looking into the details). I kept both, `FnScope` and 
`CompoundStmtScope`, because in `ActOnStartTopLevelStmtDecl()` we push them on 
the Sema side as well.

https://github.com/llvm/llvm-project/pull/84150
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Names declared in if conditions and for-init statements are local to the inner context (C++ 3.3.2p4) (PR #84150)

2024-03-06 Thread Stefan Gränitz via cfe-commits


@@ -20446,12 +20446,22 @@ Decl *Sema::ActOnFileScopeAsmDecl(Expr *expr,
   return New;
 }
 
-Decl *Sema::ActOnTopLevelStmtDecl(Stmt *Statement) {
-  auto *New = TopLevelStmtDecl::Create(Context, Statement);
-  Context.getTranslationUnitDecl()->addDecl(New);
+TopLevelStmtDecl *Sema::ActOnStartTopLevelStmtDecl(Scope *S) {
+  auto *New = TopLevelStmtDecl::Create(Context, /*Statement=*/nullptr);
+  CurContext->addDecl(New);

weliveindetail wrote:

I think we have to add the `TopLevelStmtDecl` to the decl list of the outer 
context. This happens in other similar cases as well, `ActOnBlockStart()` for 
example:
https://github.com/llvm/llvm-project/blob/release/18.x/clang/lib/Sema/SemaExpr.cpp#L16954

> Why does the next line not add it to the CurContext?

Not sure about the why, but it doesn't go through `DeclContext::addDecl()`:
https://github.com/llvm/llvm-project/blob/release/18.x/clang/lib/AST/DeclBase.cpp#L1674

FYI: the individual fixup commits contain short notes [like 
this](https://github.com/llvm/llvm-project/pull/84150/commits/39d0f4ab17e9640acbca2ce5e7bf153d14846ab9)

https://github.com/llvm/llvm-project/pull/84150
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Names declared in if conditions and for-init statements are local to the inner context (C++ 3.3.2p4) (PR #84150)

2024-03-06 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail edited 
https://github.com/llvm/llvm-project/pull/84150
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Names declared in if conditions and for-init statements are local to the inner context (C++ 3.3.2p4) (PR #84150)

2024-03-06 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail updated 
https://github.com/llvm/llvm-project/pull/84150

From 27ff0ddc0d736f4959abe61c9fd43b9c48ffb6a6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Wed, 28 Feb 2024 15:45:53 +0100
Subject: [PATCH 1/7] Add test cases for variable definitions inside for-init
 and condition statements

Interpreter/execute-stmts.cpp fails with:
```
i = 2
i = 5
i = 5
i = 5
j = 4
error: Parsing failed.
error: Parsing failed.

error: 'expected-error' diagnostics seen but not expected:
  Line 1: redefinition of 'i'
  Line 1: redefinition of 'i'
error: 'expected-note' diagnostics seen but not expected:
  Line 1: previous definition is here
  Line 1: previous definition is here
```
---
 clang/test/Interpreter/execute-stmts.cpp | 8 
 1 file changed, 8 insertions(+)

diff --git a/clang/test/Interpreter/execute-stmts.cpp 
b/clang/test/Interpreter/execute-stmts.cpp
index 2d4c17e0c91e66..34a2a35c0027d8 100644
--- a/clang/test/Interpreter/execute-stmts.cpp
+++ b/clang/test/Interpreter/execute-stmts.cpp
@@ -9,6 +9,8 @@
 //CODEGEN-CHECK-COUNT-2: define internal void @__stmts__
 //CODEGEN-CHECK-NOT: define internal void @__stmts__
 
+// New tests fail right now
+// XFAIL: *
 
 extern "C" int printf(const char*,...);
 
@@ -41,3 +43,9 @@ for (; i > 4; --i) { printf("i = %d\n", i); };
 
 int j = i; printf("j = %d\n", j);
 // CHECK-NEXT: j = 4
+
+if (int i = j) printf("i = %d\n", i);
+// CHECK-NEXT: i = 4
+
+for (int i = j; i > 3; --i) printf("i = %d\n", i);
+// CHECK-NEXT: i = 4

From b97605ae98b10ef9cbb1544f997c6a7fed917bf3 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev 
Date: Mon, 4 Mar 2024 17:51:43 +
Subject: [PATCH 2/7] Try to fix llvm/llvm-project#83028

---
 clang/include/clang/AST/Decl.h   | 16 ++--
 clang/include/clang/AST/DeclBase.h   |  1 +
 clang/include/clang/Basic/DeclNodes.td   |  2 +-
 clang/include/clang/Sema/Sema.h  |  3 ++-
 clang/lib/AST/Decl.cpp   | 11 ---
 clang/lib/AST/DeclBase.cpp   |  1 +
 clang/lib/Parse/ParseDecl.cpp| 24 
 clang/lib/Sema/SemaDecl.cpp  | 13 ++---
 clang/test/Interpreter/execute-stmts.cpp | 10 ++
 9 files changed, 59 insertions(+), 22 deletions(-)

diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 61117cc5ce71f9..bca9d42c7e0586 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -4419,7 +4419,7 @@ class FileScopeAsmDecl : public Decl {
 ///
 /// \note This is used in libInterpreter, clang -cc1 -fincremental-extensions
 /// and in tools such as clang-repl.
-class TopLevelStmtDecl : public Decl {
+class TopLevelStmtDecl : public Decl, public DeclContext {
   friend class ASTDeclReader;
   friend class ASTDeclWriter;
 
@@ -4427,7 +4427,7 @@ class TopLevelStmtDecl : public Decl {
   bool IsSemiMissing = false;
 
   TopLevelStmtDecl(DeclContext *DC, SourceLocation L, Stmt *S)
-  : Decl(TopLevelStmt, DC, L), Statement(S) {}
+: Decl(TopLevelStmt, DC, L), DeclContext(TopLevelStmt), Statement(S) {}
 
   virtual void anchor();
 
@@ -4438,15 +4438,19 @@ class TopLevelStmtDecl : public Decl {
   SourceRange getSourceRange() const override LLVM_READONLY;
   Stmt *getStmt() { return Statement; }
   const Stmt *getStmt() const { return Statement; }
-  void setStmt(Stmt *S) {
-assert(IsSemiMissing && "Operation supported for printing values only!");
-Statement = S;
-  }
+  void setStmt(Stmt *S);
   bool isSemiMissing() const { return IsSemiMissing; }
   void setSemiMissing(bool Missing = true) { IsSemiMissing = Missing; }
 
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
   static bool classofKind(Kind K) { return K == TopLevelStmt; }
+
+  static DeclContext *castToDeclContext(const TopLevelStmtDecl *D) {
+return static_cast(const_cast(D));
+  }
+  static TopLevelStmtDecl *castFromDeclContext(const DeclContext *DC) {
+return static_cast(const_cast(DC));
+  }
 };
 
 /// Represents a block literal declaration, which is like an
diff --git a/clang/include/clang/AST/DeclBase.h 
b/clang/include/clang/AST/DeclBase.h
index 9a4736019d1b1b..76810a86a78a46 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -2120,6 +2120,7 @@ class DeclContext {
 case Decl::Block:
 case Decl::Captured:
 case Decl::ObjCMethod:
+case Decl::TopLevelStmt:
   return true;
 default:
   return getDeclKind() >= Decl::firstFunction &&
diff --git a/clang/include/clang/Basic/DeclNodes.td 
b/clang/include/clang/Basic/DeclNodes.td
index 8b1f415dd5fe2c..48396e85c5adac 100644
--- a/clang/include/clang/Basic/DeclNodes.td
+++ b/clang/include/clang/Basic/DeclNodes.td
@@ -95,7 +95,7 @@ def LinkageSpec : DeclNode, DeclContext;
 def Export : DeclNode, DeclContext;
 def ObjCPropertyImpl : DeclNode;
 def FileScopeAsm : DeclNode;
-def TopLevelStmt : DeclNode;
+def TopLevelStmt : DeclNo

[clang] [clang-repl] Names declared in if conditions and for-init statements are local to the inner context (C++ 3.3.2p4) (PR #84150)

2024-03-06 Thread Stefan Gränitz via cfe-commits

weliveindetail wrote:

Thanks! Added `while` with empty body as well as `switch`. Refined test output 
a bit, so that it now prints:
```
i = 5 (global scope)
i = 1 (while condition)
i = 2 (if condition)
i = 3 (switch condition)
i = 4 (for-init)
```

https://github.com/llvm/llvm-project/pull/84150
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Pass triple to IncrementalCompilerBuilder as explicit argument (PR #84174)

2024-03-06 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail created 
https://github.com/llvm/llvm-project/pull/84174

With out-of-process execution the target triple can be different from the one 
on the host. We need an interface to configure it.

From 767fccd9b2f5badee46aaf8ab9cca5315c538720 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Wed, 6 Mar 2024 15:14:52 +0100
Subject: [PATCH] [clang-repl] Pass triple to IncrementalCompilerBuilder as
 explicit argument

With out-of-process execution the target triple can be different from the one 
on the host. We need an interface to configure it.
---
 clang/include/clang/Interpreter/Interpreter.h | 13 ++-
 clang/lib/Interpreter/Interpreter.cpp | 22 +--
 clang/tools/clang-repl/ClangRepl.cpp  | 12 ++
 3 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/clang/include/clang/Interpreter/Interpreter.h 
b/clang/include/clang/Interpreter/Interpreter.h
index 292fa566ae7037..8d111aa086867a 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -49,7 +49,7 @@ class IncrementalCompilerBuilder {
   }
 
   // General C++
-  llvm::Expected> CreateCpp();
+  llvm::Expected> CreateCpp(llvm::Triple TT);
 
   // Offload options
   void SetOffloadArch(llvm::StringRef Arch) { OffloadArch = Arch; };
@@ -57,14 +57,17 @@ class IncrementalCompilerBuilder {
   // CUDA specific
   void SetCudaSDK(llvm::StringRef path) { CudaSDKPath = path; };
 
-  llvm::Expected> CreateCudaHost();
-  llvm::Expected> CreateCudaDevice();
+  llvm::Expected>
+  CreateCudaHost(llvm::Triple TT);
+  llvm::Expected>
+  CreateCudaDevice(llvm::Triple TT);
 
 private:
   static llvm::Expected>
-  create(std::vector &ClangArgv);
+  create(llvm::Triple TT, std::vector &ClangArgv);
 
-  llvm::Expected> createCuda(bool device);
+  llvm::Expected> createCuda(llvm::Triple TT,
+   bool device);
 
   std::vector UserArgs;
 
diff --git a/clang/lib/Interpreter/Interpreter.cpp 
b/clang/lib/Interpreter/Interpreter.cpp
index 9f97a3c6b0be9e..429c1b65f6665d 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -132,7 +132,8 @@ CreateCI(const llvm::opt::ArgStringList &Argv) {
 } // anonymous namespace
 
 llvm::Expected>
-IncrementalCompilerBuilder::create(std::vector &ClangArgv) {
+IncrementalCompilerBuilder::create(llvm::Triple TT,
+   std::vector &ClangArgv) {
 
   // If we don't know ClangArgv0 or the address of main() at this point, try
   // to guess it anyway (it's possible on some platforms).
@@ -162,8 +163,7 @@ IncrementalCompilerBuilder::create(std::vector &ClangArgv) {
   TextDiagnosticBuffer *DiagsBuffer = new TextDiagnosticBuffer;
   DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagsBuffer);
 
-  driver::Driver Driver(/*MainBinaryName=*/ClangArgv[0],
-llvm::sys::getProcessTriple(), Diags);
+  driver::Driver Driver(/*MainBinaryName=*/ClangArgv[0], TT.str(), Diags);
   Driver.setCheckInputsExist(false); // the input comes from mem buffers
   llvm::ArrayRef RF = llvm::ArrayRef(ClangArgv);
   std::unique_ptr 
Compilation(Driver.BuildCompilation(RF));
@@ -179,17 +179,17 @@ IncrementalCompilerBuilder::create(std::vector &ClangArgv) {
 }
 
 llvm::Expected>
-IncrementalCompilerBuilder::CreateCpp() {
+IncrementalCompilerBuilder::CreateCpp(llvm::Triple TT) {
   std::vector Argv;
   Argv.reserve(5 + 1 + UserArgs.size());
   Argv.push_back("-xc++");
   Argv.insert(Argv.end(), UserArgs.begin(), UserArgs.end());
 
-  return IncrementalCompilerBuilder::create(Argv);
+  return IncrementalCompilerBuilder::create(TT, Argv);
 }
 
 llvm::Expected>
-IncrementalCompilerBuilder::createCuda(bool device) {
+IncrementalCompilerBuilder::createCuda(llvm::Triple TT, bool device) {
   std::vector Argv;
   Argv.reserve(5 + 4 + UserArgs.size());
 
@@ -213,17 +213,17 @@ IncrementalCompilerBuilder::createCuda(bool device) {
 
   Argv.insert(Argv.end(), UserArgs.begin(), UserArgs.end());
 
-  return IncrementalCompilerBuilder::create(Argv);
+  return IncrementalCompilerBuilder::create(TT, Argv);
 }
 
 llvm::Expected>
-IncrementalCompilerBuilder::CreateCudaDevice() {
-  return IncrementalCompilerBuilder::createCuda(true);
+IncrementalCompilerBuilder::CreateCudaDevice(llvm::Triple TT) {
+  return IncrementalCompilerBuilder::createCuda(TT, true);
 }
 
 llvm::Expected>
-IncrementalCompilerBuilder::CreateCudaHost() {
-  return IncrementalCompilerBuilder::createCuda(false);
+IncrementalCompilerBuilder::CreateCudaHost(llvm::Triple TT) {
+  return IncrementalCompilerBuilder::createCuda(TT, false);
 }
 
 Interpreter::Interpreter(std::unique_ptr CI,
diff --git a/clang/tools/clang-repl/ClangRepl.cpp 
b/clang/tools/clang-repl/ClangRepl.cpp
index 5bad8145324d06..96337ada043a6f 100644
--- a/clang/tools/clang-repl/ClangRepl.cpp
+++ b/clang/tools/clang-repl/ClangRepl.cpp
@@ -24,6 +24,7 @@
 #includ

[clang] [clang-repl] Pass triple to IncrementalCompilerBuilder as explicit argument (PR #84174)

2024-03-06 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail updated 
https://github.com/llvm/llvm-project/pull/84174

From 0d7f6a8e72caeada251c661f5804d9766345e052 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Wed, 6 Mar 2024 16:46:56 +0100
Subject: [PATCH] [clang-repl] Pass triple to IncrementalCompilerBuilder as
 explicit argument

With out-of-process execution the target triple can be different from the one 
on the host. We need an interface to configure it.
---
 clang/include/clang/Interpreter/Interpreter.h |  5 ++-
 clang/lib/Interpreter/Interpreter.cpp | 12 ---
 clang/unittests/Interpreter/CMakeLists.txt|  1 +
 .../IncrementalCompilerBuilderTest.cpp| 35 +++
 4 files changed, 47 insertions(+), 6 deletions(-)
 create mode 100644 
clang/unittests/Interpreter/IncrementalCompilerBuilderTest.cpp

diff --git a/clang/include/clang/Interpreter/Interpreter.h 
b/clang/include/clang/Interpreter/Interpreter.h
index 292fa566ae7037..c8f932e95c4798 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -48,6 +48,8 @@ class IncrementalCompilerBuilder {
 UserArgs = Args;
   }
 
+  void SetTargetTriple(std::string TT) { TargetTriple = TT; }
+
   // General C++
   llvm::Expected> CreateCpp();
 
@@ -62,11 +64,12 @@ class IncrementalCompilerBuilder {
 
 private:
   static llvm::Expected>
-  create(std::vector &ClangArgv);
+  create(std::string TT, std::vector &ClangArgv);
 
   llvm::Expected> createCuda(bool device);
 
   std::vector UserArgs;
+  std::optional TargetTriple;
 
   llvm::StringRef OffloadArch;
   llvm::StringRef CudaSDKPath;
diff --git a/clang/lib/Interpreter/Interpreter.cpp 
b/clang/lib/Interpreter/Interpreter.cpp
index 9f97a3c6b0be9e..37696b28976428 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -132,7 +132,8 @@ CreateCI(const llvm::opt::ArgStringList &Argv) {
 } // anonymous namespace
 
 llvm::Expected>
-IncrementalCompilerBuilder::create(std::vector &ClangArgv) {
+IncrementalCompilerBuilder::create(std::string TT,
+   std::vector &ClangArgv) {
 
   // If we don't know ClangArgv0 or the address of main() at this point, try
   // to guess it anyway (it's possible on some platforms).
@@ -162,8 +163,7 @@ IncrementalCompilerBuilder::create(std::vector &ClangArgv) {
   TextDiagnosticBuffer *DiagsBuffer = new TextDiagnosticBuffer;
   DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagsBuffer);
 
-  driver::Driver Driver(/*MainBinaryName=*/ClangArgv[0],
-llvm::sys::getProcessTriple(), Diags);
+  driver::Driver Driver(/*MainBinaryName=*/ClangArgv[0], TT, Diags);
   Driver.setCheckInputsExist(false); // the input comes from mem buffers
   llvm::ArrayRef RF = llvm::ArrayRef(ClangArgv);
   std::unique_ptr 
Compilation(Driver.BuildCompilation(RF));
@@ -185,7 +185,8 @@ IncrementalCompilerBuilder::CreateCpp() {
   Argv.push_back("-xc++");
   Argv.insert(Argv.end(), UserArgs.begin(), UserArgs.end());
 
-  return IncrementalCompilerBuilder::create(Argv);
+  std::string TT = TargetTriple ? *TargetTriple : 
llvm::sys::getProcessTriple();
+  return IncrementalCompilerBuilder::create(TT, Argv);
 }
 
 llvm::Expected>
@@ -213,7 +214,8 @@ IncrementalCompilerBuilder::createCuda(bool device) {
 
   Argv.insert(Argv.end(), UserArgs.begin(), UserArgs.end());
 
-  return IncrementalCompilerBuilder::create(Argv);
+  std::string TT = TargetTriple ? *TargetTriple : 
llvm::sys::getProcessTriple();
+  return IncrementalCompilerBuilder::create(TT, Argv);
 }
 
 llvm::Expected>
diff --git a/clang/unittests/Interpreter/CMakeLists.txt 
b/clang/unittests/Interpreter/CMakeLists.txt
index 712641afb976dd..0ddedb283e07d1 100644
--- a/clang/unittests/Interpreter/CMakeLists.txt
+++ b/clang/unittests/Interpreter/CMakeLists.txt
@@ -7,6 +7,7 @@ set(LLVM_LINK_COMPONENTS
   )
 
 add_clang_unittest(ClangReplInterpreterTests
+  IncrementalCompilerBuilderTest.cpp
   IncrementalProcessingTest.cpp
   InterpreterTest.cpp
   CodeCompletionTest.cpp
diff --git a/clang/unittests/Interpreter/IncrementalCompilerBuilderTest.cpp 
b/clang/unittests/Interpreter/IncrementalCompilerBuilderTest.cpp
new file mode 100644
index 00..1cc0223465c8fa
--- /dev/null
+++ b/clang/unittests/Interpreter/IncrementalCompilerBuilderTest.cpp
@@ -0,0 +1,35 @@
+//=== unittests/Interpreter/IncrementalCompilerBuilderTest.cpp 
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Basic/TargetOptions.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Interpreter/Interpreter.h"
+#include "llvm/Support/Error.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace clang;
+
+namesp

[clang] [clang-repl] Refactor locking of runtime PTU stack (NFC) (PR #84176)

2024-03-06 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail updated 
https://github.com/llvm/llvm-project/pull/84176

From 143ed8ccf592be46181fb3dcd814b4afa1b39833 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Wed, 6 Mar 2024 12:37:50 +0100
Subject: [PATCH 1/2] [clang-repl] Refactor locking of runtime PTU stack (NFC)

The previous implementation seemed hacky, because it required the reader to be 
familiar with the internal workings of the PTU stack. The concept itself is a 
pragmatic solution and not very surprising. Keeping it behind a finalization 
call seems reasonable.
---
 clang/include/clang/Interpreter/Interpreter.h |  1 +
 clang/lib/Interpreter/Interpreter.cpp | 12 
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/clang/include/clang/Interpreter/Interpreter.h 
b/clang/include/clang/Interpreter/Interpreter.h
index 292fa566ae7037..ce46aefe08a475 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -139,6 +139,7 @@ class Interpreter {
 
 private:
   size_t getEffectivePTUSize() const;
+  void finalizeInitPTUStack();
 
   bool FindRuntimeInterface();
 
diff --git a/clang/lib/Interpreter/Interpreter.cpp 
b/clang/lib/Interpreter/Interpreter.cpp
index 9f97a3c6b0be9e..e06ab45fc6fa0d 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -278,15 +278,14 @@ Interpreter::create(std::unique_ptr CI) 
{
   if (Err)
 return std::move(Err);
 
+  // Runtimes contain essential definitions that are irrevocable. Lock their
+  // stack of initial PTUs to make them unavailable for undo.
   auto PTU = Interp->Parse(Runtimes);
   if (!PTU)
 return PTU.takeError();
+  Interp->finalizeInitPTUStack();
 
   Interp->ValuePrintingInfo.resize(4);
-  // FIXME: This is a ugly hack. Undo command checks its availability by 
looking
-  // at the size of the PTU list. However we have parsed something in the
-  // beginning of the REPL so we have to mark them as 'Irrevocable'.
-  Interp->InitPTUSize = Interp->IncrParser->getPTUs().size();
   return std::move(Interp);
 }
 
@@ -343,6 +342,11 @@ const ASTContext &Interpreter::getASTContext() const {
   return getCompilerInstance()->getASTContext();
 }
 
+void Interpreter::finalizeInitPTUStack() {
+  assert(!InitPTUSize && "We only do this once");
+  InitPTUSize = IncrParser->getPTUs().size();
+}
+
 size_t Interpreter::getEffectivePTUSize() const {
   std::list &PTUs = IncrParser->getPTUs();
   assert(PTUs.size() >= InitPTUSize && "empty PTU list?");

From 6aa4f901c0ba0b946b60b9a4f03ce1a6c47b1924 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Wed, 6 Mar 2024 17:28:40 +0100
Subject: [PATCH 2/2] Make interface accessible and add unittest

---
 clang/include/clang/Interpreter/Interpreter.h  |  5 ++---
 .../unittests/Interpreter/InterpreterTest.cpp  | 18 ++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/clang/include/clang/Interpreter/Interpreter.h 
b/clang/include/clang/Interpreter/Interpreter.h
index ce46aefe08a475..402d3a79d3ed83 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -81,8 +81,6 @@ class Interpreter {
   // An optional parser for CUDA offloading
   std::unique_ptr DeviceParser;
 
-  Interpreter(std::unique_ptr CI, llvm::Error &Err);
-
   llvm::Error CreateExecutor();
   unsigned InitPTUSize = 0;
 
@@ -92,6 +90,7 @@ class Interpreter {
   Value LastValue;
 
 public:
+  Interpreter(std::unique_ptr CI, llvm::Error &Err);
   ~Interpreter();
   static llvm::Expected>
   create(std::unique_ptr CI);
@@ -137,10 +136,10 @@ class Interpreter {
 
   Expr *SynthesizeExpr(Expr *E);
 
-private:
   size_t getEffectivePTUSize() const;
   void finalizeInitPTUStack();
 
+private:
   bool FindRuntimeInterface();
 
   llvm::DenseMap Dtors;
diff --git a/clang/unittests/Interpreter/InterpreterTest.cpp 
b/clang/unittests/Interpreter/InterpreterTest.cpp
index e76c0677db5ead..c8fdf969589cf7 100644
--- a/clang/unittests/Interpreter/InterpreterTest.cpp
+++ b/clang/unittests/Interpreter/InterpreterTest.cpp
@@ -136,6 +136,24 @@ TEST(InterpreterTest, DeclsAndStatements) {
   EXPECT_TRUE(!!R2);
 }
 
+TEST(InterpreterTest, PTUStack) {
+  clang::IncrementalCompilerBuilder CB;
+  auto CI = cantFail(CB.CreateCpp());
+
+  llvm::Error Err = llvm::Error::success();
+  auto Interp = std::make_unique(std::move(CI), Err);
+  cantFail(std::move(Err));
+
+  auto NumBuiltinPTUs = Interp->getEffectivePTUSize();
+  cantFail(Interp->Parse("void firstRuntimePTU() {}"));
+  EXPECT_EQ(NumBuiltinPTUs + 1, Interp->getEffectivePTUSize());
+  cantFail(Interp->Parse("void secondRuntimePTU() {}"));
+  EXPECT_EQ(NumBuiltinPTUs + 2, Interp->getEffectivePTUSize());
+
+  Interp->finalizeInitPTUStack();
+  EXPECT_EQ(0ul, Interp->getEffectivePTUSize());
+}
+
 TEST(InterpreterTest, UndoCommand) {
   Args ExtraArgs = {"-Xclang", "-diagnostic-log-file", "-Xclang", "-"};
 

___

[clang] [clang-repl] Refactor locking of runtime PTU stack (NFC) (PR #84176)

2024-03-06 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail edited 
https://github.com/llvm/llvm-project/pull/84176
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Refactor locking of runtime PTU stack (NFC) (PR #84176)

2024-03-06 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail commented:

Yes sure! It requires access to the interface, but IMHO that makes a lot of 
sense anyway.

https://github.com/llvm/llvm-project/pull/84176
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Refactor locking of runtime PTU stack (NFC) (PR #84176)

2024-03-06 Thread Stefan Gränitz via cfe-commits


@@ -137,9 +136,10 @@ class Interpreter {
 
   Expr *SynthesizeExpr(Expr *E);
 
-private:
   size_t getEffectivePTUSize() const;
+  void finalizeInitPTUStack();

weliveindetail wrote:

We could also consider renaming this to `getNumUndoSteps()` and 
`setUndoBarrier()` respectively (or some thing like that).

https://github.com/llvm/llvm-project/pull/84176
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Names declared in if conditions and for-init statements are local to the inner context (C++ 3.3.2p4) (PR #84150)

2024-03-06 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail updated 
https://github.com/llvm/llvm-project/pull/84150

From 27ff0ddc0d736f4959abe61c9fd43b9c48ffb6a6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Wed, 28 Feb 2024 15:45:53 +0100
Subject: [PATCH 1/9] Add test cases for variable definitions inside for-init
 and condition statements

Interpreter/execute-stmts.cpp fails with:
```
i = 2
i = 5
i = 5
i = 5
j = 4
error: Parsing failed.
error: Parsing failed.

error: 'expected-error' diagnostics seen but not expected:
  Line 1: redefinition of 'i'
  Line 1: redefinition of 'i'
error: 'expected-note' diagnostics seen but not expected:
  Line 1: previous definition is here
  Line 1: previous definition is here
```
---
 clang/test/Interpreter/execute-stmts.cpp | 8 
 1 file changed, 8 insertions(+)

diff --git a/clang/test/Interpreter/execute-stmts.cpp 
b/clang/test/Interpreter/execute-stmts.cpp
index 2d4c17e0c91e66..34a2a35c0027d8 100644
--- a/clang/test/Interpreter/execute-stmts.cpp
+++ b/clang/test/Interpreter/execute-stmts.cpp
@@ -9,6 +9,8 @@
 //CODEGEN-CHECK-COUNT-2: define internal void @__stmts__
 //CODEGEN-CHECK-NOT: define internal void @__stmts__
 
+// New tests fail right now
+// XFAIL: *
 
 extern "C" int printf(const char*,...);
 
@@ -41,3 +43,9 @@ for (; i > 4; --i) { printf("i = %d\n", i); };
 
 int j = i; printf("j = %d\n", j);
 // CHECK-NEXT: j = 4
+
+if (int i = j) printf("i = %d\n", i);
+// CHECK-NEXT: i = 4
+
+for (int i = j; i > 3; --i) printf("i = %d\n", i);
+// CHECK-NEXT: i = 4

From b97605ae98b10ef9cbb1544f997c6a7fed917bf3 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev 
Date: Mon, 4 Mar 2024 17:51:43 +
Subject: [PATCH 2/9] Try to fix llvm/llvm-project#83028

---
 clang/include/clang/AST/Decl.h   | 16 ++--
 clang/include/clang/AST/DeclBase.h   |  1 +
 clang/include/clang/Basic/DeclNodes.td   |  2 +-
 clang/include/clang/Sema/Sema.h  |  3 ++-
 clang/lib/AST/Decl.cpp   | 11 ---
 clang/lib/AST/DeclBase.cpp   |  1 +
 clang/lib/Parse/ParseDecl.cpp| 24 
 clang/lib/Sema/SemaDecl.cpp  | 13 ++---
 clang/test/Interpreter/execute-stmts.cpp | 10 ++
 9 files changed, 59 insertions(+), 22 deletions(-)

diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 61117cc5ce71f9..bca9d42c7e0586 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -4419,7 +4419,7 @@ class FileScopeAsmDecl : public Decl {
 ///
 /// \note This is used in libInterpreter, clang -cc1 -fincremental-extensions
 /// and in tools such as clang-repl.
-class TopLevelStmtDecl : public Decl {
+class TopLevelStmtDecl : public Decl, public DeclContext {
   friend class ASTDeclReader;
   friend class ASTDeclWriter;
 
@@ -4427,7 +4427,7 @@ class TopLevelStmtDecl : public Decl {
   bool IsSemiMissing = false;
 
   TopLevelStmtDecl(DeclContext *DC, SourceLocation L, Stmt *S)
-  : Decl(TopLevelStmt, DC, L), Statement(S) {}
+: Decl(TopLevelStmt, DC, L), DeclContext(TopLevelStmt), Statement(S) {}
 
   virtual void anchor();
 
@@ -4438,15 +4438,19 @@ class TopLevelStmtDecl : public Decl {
   SourceRange getSourceRange() const override LLVM_READONLY;
   Stmt *getStmt() { return Statement; }
   const Stmt *getStmt() const { return Statement; }
-  void setStmt(Stmt *S) {
-assert(IsSemiMissing && "Operation supported for printing values only!");
-Statement = S;
-  }
+  void setStmt(Stmt *S);
   bool isSemiMissing() const { return IsSemiMissing; }
   void setSemiMissing(bool Missing = true) { IsSemiMissing = Missing; }
 
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
   static bool classofKind(Kind K) { return K == TopLevelStmt; }
+
+  static DeclContext *castToDeclContext(const TopLevelStmtDecl *D) {
+return static_cast(const_cast(D));
+  }
+  static TopLevelStmtDecl *castFromDeclContext(const DeclContext *DC) {
+return static_cast(const_cast(DC));
+  }
 };
 
 /// Represents a block literal declaration, which is like an
diff --git a/clang/include/clang/AST/DeclBase.h 
b/clang/include/clang/AST/DeclBase.h
index 9a4736019d1b1b..76810a86a78a46 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -2120,6 +2120,7 @@ class DeclContext {
 case Decl::Block:
 case Decl::Captured:
 case Decl::ObjCMethod:
+case Decl::TopLevelStmt:
   return true;
 default:
   return getDeclKind() >= Decl::firstFunction &&
diff --git a/clang/include/clang/Basic/DeclNodes.td 
b/clang/include/clang/Basic/DeclNodes.td
index 8b1f415dd5fe2c..48396e85c5adac 100644
--- a/clang/include/clang/Basic/DeclNodes.td
+++ b/clang/include/clang/Basic/DeclNodes.td
@@ -95,7 +95,7 @@ def LinkageSpec : DeclNode, DeclContext;
 def Export : DeclNode, DeclContext;
 def ObjCPropertyImpl : DeclNode;
 def FileScopeAsm : DeclNode;
-def TopLevelStmt : DeclNode;
+def TopLevelStmt : DeclNo

[clang] [clang-repl] Names declared in if conditions and for-init statements are local to the inner context (C++ 3.3.2p4) (PR #84150)

2024-03-06 Thread Stefan Gränitz via cfe-commits


@@ -41,3 +40,23 @@ for (; i > 4; --i) { printf("i = %d\n", i); };
 
 int j = i; printf("j = %d\n", j);
 // CHECK-NEXT: j = 4
+
+{++i; printf("i = %d (global scope)\n", i);}
+// CHECK-NEXT: i = 5
+
+while (int i = 1) { printf("i = %d (while condition)\n", i--); break; }
+// CHECK-NEXT: i = 1
+
+if (int i = 2) printf("i = %d (if condition)\n", i);
+// CHECK-NEXT: i = 2
+
+switch (int i = 3) { default: printf("i = %d (switch condition)\n", i); }
+// CHECK-NEXT: i = 3
+
+for (int i = 4; i > 3; --i) printf("i = %d (for-init)\n", i);

weliveindetail wrote:

Yes good point, that makes a lot of sense indeed!

https://github.com/llvm/llvm-project/pull/84150
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Refactor locking of runtime PTU stack (NFC) (PR #84176)

2024-03-06 Thread Stefan Gränitz via cfe-commits


@@ -137,9 +136,10 @@ class Interpreter {
 
   Expr *SynthesizeExpr(Expr *E);
 
-private:

weliveindetail wrote:

Ok, let me play with it a little more.

https://github.com/llvm/llvm-project/pull/84176
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Refactor locking of runtime PTU stack (NFC) (PR #84176)

2024-03-06 Thread Stefan Gränitz via cfe-commits


@@ -137,9 +136,10 @@ class Interpreter {
 
   Expr *SynthesizeExpr(Expr *E);
 
-private:
   size_t getEffectivePTUSize() const;
+  void finalizeInitPTUStack();

weliveindetail wrote:

Hm, why restore point? Do you mean sth like `markMaxRestorePoint` as in "the 
last point we can revert to"? And would you keep `getEffectivePTUSize` for the 
getter then? It seemed reasonable to me to make a relation between the two. I 
still think restoration is a misleading term, because we can only recover the 
compiler state precisely. Execution state is not restorable in the same way.

What else does it affect apart from Undo? If it's only Undo, then I'd be 
inclined to have it in the name. If we explicitly don't want a relation between 
then two, it could also just be `finalize`.

https://github.com/llvm/llvm-project/pull/84176
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Pass triple to IncrementalCompilerBuilder as explicit argument (PR #84174)

2024-03-06 Thread Stefan Gränitz via cfe-commits

weliveindetail wrote:

Thanks for the quick feedback and review!

https://github.com/llvm/llvm-project/pull/84174
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Pass triple to IncrementalCompilerBuilder as explicit argument (PR #84174)

2024-03-06 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail closed 
https://github.com/llvm/llvm-project/pull/84174
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Refactor locking of runtime PTU stack (NFC) (PR #84176)

2024-03-06 Thread Stefan Gränitz via cfe-commits


@@ -137,9 +136,10 @@ class Interpreter {
 
   Expr *SynthesizeExpr(Expr *E);
 
-private:

weliveindetail wrote:

> We can befriend the test to the interpreter.

I don't think we can do this:
```
// Defines FRIEND_TEST.
#include 
```

> make this `protected:` and have a derived class on the test side to access 
> this

The derived class must mimic `clang::Interpreter::create()`, but we can't 
because the ctor is private. There are a few more options:
(1) Allow to derive properly from Interpreter. (Pragmatic choice. Effort seems 
reasonable.)
(2) Build another factory interface around it, like "InterpreterBuilder". (Too 
much effort.)
(3) Integrate it into `IncrementalCompilerBuilder`. (Not sure, but maybe 
interesting.)
(4) Add a special purpose `create` function. (Quick and hacky)

This will not remain the only case, where we want to access the extended class 
surface from tests. What do you think?

https://github.com/llvm/llvm-project/pull/84176
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Pass triple to IncrementalCompilerBuilder as explicit argument (PR #84174)

2024-03-06 Thread Stefan Gränitz via cfe-commits

weliveindetail wrote:

Interesting, thanks for reporting! It's this code:
```
  llvm::MemoryBuffer *MB = llvm::MemoryBuffer::getMemBuffer("").release();
  Clang->getPreprocessorOpts().addRemappedFile("<<< inputs >>>", MB);
```

Apparently, it is related to releasing the MemBuffer and passing the raw 
pointer to `addRemappedFile()`, but I don't see why this fails here and not in 
any of the existing tests. Let me revert and investigate tomorrow.

https://github.com/llvm/llvm-project/pull/84174
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Revert "[clang-repl] Pass triple to IncrementalCompilerBuilder as explicit argument" (PR #84261)

2024-03-06 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail created 
https://github.com/llvm/llvm-project/pull/84261

Reverts llvm/llvm-project#84174 due too sanitizer memory leak detection

From efe6097aad69aba7d2421880305198bf09226db6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Thu, 7 Mar 2024 01:00:03 +0100
Subject: [PATCH] =?UTF-8?q?Revert=20"[clang-repl]=20Expose=20setter=20for?=
 =?UTF-8?q?=20triple=20in=20IncrementalCompilerBuilder=20(=E2=80=A6"?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This reverts commit 6494f9bb8ac1e6f7526b72ee07f71527b8e66066.
---
 clang/include/clang/Interpreter/Interpreter.h |  5 +--
 clang/lib/Interpreter/Interpreter.cpp | 12 +++
 clang/unittests/Interpreter/CMakeLists.txt|  1 -
 .../IncrementalCompilerBuilderTest.cpp| 35 ---
 4 files changed, 6 insertions(+), 47 deletions(-)
 delete mode 100644 
clang/unittests/Interpreter/IncrementalCompilerBuilderTest.cpp

diff --git a/clang/include/clang/Interpreter/Interpreter.h 
b/clang/include/clang/Interpreter/Interpreter.h
index c8f932e95c4798..292fa566ae7037 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -48,8 +48,6 @@ class IncrementalCompilerBuilder {
 UserArgs = Args;
   }
 
-  void SetTargetTriple(std::string TT) { TargetTriple = TT; }
-
   // General C++
   llvm::Expected> CreateCpp();
 
@@ -64,12 +62,11 @@ class IncrementalCompilerBuilder {
 
 private:
   static llvm::Expected>
-  create(std::string TT, std::vector &ClangArgv);
+  create(std::vector &ClangArgv);
 
   llvm::Expected> createCuda(bool device);
 
   std::vector UserArgs;
-  std::optional TargetTriple;
 
   llvm::StringRef OffloadArch;
   llvm::StringRef CudaSDKPath;
diff --git a/clang/lib/Interpreter/Interpreter.cpp 
b/clang/lib/Interpreter/Interpreter.cpp
index 37696b28976428..9f97a3c6b0be9e 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -132,8 +132,7 @@ CreateCI(const llvm::opt::ArgStringList &Argv) {
 } // anonymous namespace
 
 llvm::Expected>
-IncrementalCompilerBuilder::create(std::string TT,
-   std::vector &ClangArgv) {
+IncrementalCompilerBuilder::create(std::vector &ClangArgv) {
 
   // If we don't know ClangArgv0 or the address of main() at this point, try
   // to guess it anyway (it's possible on some platforms).
@@ -163,7 +162,8 @@ IncrementalCompilerBuilder::create(std::string TT,
   TextDiagnosticBuffer *DiagsBuffer = new TextDiagnosticBuffer;
   DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagsBuffer);
 
-  driver::Driver Driver(/*MainBinaryName=*/ClangArgv[0], TT, Diags);
+  driver::Driver Driver(/*MainBinaryName=*/ClangArgv[0],
+llvm::sys::getProcessTriple(), Diags);
   Driver.setCheckInputsExist(false); // the input comes from mem buffers
   llvm::ArrayRef RF = llvm::ArrayRef(ClangArgv);
   std::unique_ptr 
Compilation(Driver.BuildCompilation(RF));
@@ -185,8 +185,7 @@ IncrementalCompilerBuilder::CreateCpp() {
   Argv.push_back("-xc++");
   Argv.insert(Argv.end(), UserArgs.begin(), UserArgs.end());
 
-  std::string TT = TargetTriple ? *TargetTriple : 
llvm::sys::getProcessTriple();
-  return IncrementalCompilerBuilder::create(TT, Argv);
+  return IncrementalCompilerBuilder::create(Argv);
 }
 
 llvm::Expected>
@@ -214,8 +213,7 @@ IncrementalCompilerBuilder::createCuda(bool device) {
 
   Argv.insert(Argv.end(), UserArgs.begin(), UserArgs.end());
 
-  std::string TT = TargetTriple ? *TargetTriple : 
llvm::sys::getProcessTriple();
-  return IncrementalCompilerBuilder::create(TT, Argv);
+  return IncrementalCompilerBuilder::create(Argv);
 }
 
 llvm::Expected>
diff --git a/clang/unittests/Interpreter/CMakeLists.txt 
b/clang/unittests/Interpreter/CMakeLists.txt
index 0ddedb283e07d1..712641afb976dd 100644
--- a/clang/unittests/Interpreter/CMakeLists.txt
+++ b/clang/unittests/Interpreter/CMakeLists.txt
@@ -7,7 +7,6 @@ set(LLVM_LINK_COMPONENTS
   )
 
 add_clang_unittest(ClangReplInterpreterTests
-  IncrementalCompilerBuilderTest.cpp
   IncrementalProcessingTest.cpp
   InterpreterTest.cpp
   CodeCompletionTest.cpp
diff --git a/clang/unittests/Interpreter/IncrementalCompilerBuilderTest.cpp 
b/clang/unittests/Interpreter/IncrementalCompilerBuilderTest.cpp
deleted file mode 100644
index 1cc0223465c8fa..00
--- a/clang/unittests/Interpreter/IncrementalCompilerBuilderTest.cpp
+++ /dev/null
@@ -1,35 +0,0 @@
-//=== unittests/Interpreter/IncrementalCompilerBuilderTest.cpp 
===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "clang/Basic/TargetOptions.h"
-#include "clang/Frontend/CompilerInstance.h"
-#include "clang/In

[clang] Revert "[clang-repl] Pass triple to IncrementalCompilerBuilder as explicit argument" (PR #84261)

2024-03-06 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail closed 
https://github.com/llvm/llvm-project/pull/84261
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Names declared in if conditions and for-init statements are local to the inner context (C++ 3.3.2p4) (PR #84150)

2024-03-07 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail closed 
https://github.com/llvm/llvm-project/pull/84150
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 2a4a852 - Reland [clang-repl] Expose setter for triple in IncrementalCompilerBuilder (#84174)

2024-03-07 Thread Stefan Gränitz via cfe-commits

Author: Stefan Gränitz
Date: 2024-03-07T23:15:02+01:00
New Revision: 2a4a852a67eab2f8d0533c23719b1bd08d6edea9

URL: 
https://github.com/llvm/llvm-project/commit/2a4a852a67eab2f8d0533c23719b1bd08d6edea9
DIFF: 
https://github.com/llvm/llvm-project/commit/2a4a852a67eab2f8d0533c23719b1bd08d6edea9.diff

LOG: Reland [clang-repl] Expose setter for triple in IncrementalCompilerBuilder 
(#84174)

With out-of-process execution the target triple can be different from
the one on the host. We need an interface to configure it.

Relanding this with cleanup-fixes in the unittest.

Added: 
clang/unittests/Interpreter/IncrementalCompilerBuilderTest.cpp

Modified: 
clang/include/clang/Interpreter/Interpreter.h
clang/lib/Interpreter/Interpreter.cpp
clang/unittests/Interpreter/CMakeLists.txt

Removed: 




diff  --git a/clang/include/clang/Interpreter/Interpreter.h 
b/clang/include/clang/Interpreter/Interpreter.h
index 292fa566ae7037..c8f932e95c4798 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -48,6 +48,8 @@ class IncrementalCompilerBuilder {
 UserArgs = Args;
   }
 
+  void SetTargetTriple(std::string TT) { TargetTriple = TT; }
+
   // General C++
   llvm::Expected> CreateCpp();
 
@@ -62,11 +64,12 @@ class IncrementalCompilerBuilder {
 
 private:
   static llvm::Expected>
-  create(std::vector &ClangArgv);
+  create(std::string TT, std::vector &ClangArgv);
 
   llvm::Expected> createCuda(bool device);
 
   std::vector UserArgs;
+  std::optional TargetTriple;
 
   llvm::StringRef OffloadArch;
   llvm::StringRef CudaSDKPath;

diff  --git a/clang/lib/Interpreter/Interpreter.cpp 
b/clang/lib/Interpreter/Interpreter.cpp
index 9f97a3c6b0be9e..37696b28976428 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -132,7 +132,8 @@ CreateCI(const llvm::opt::ArgStringList &Argv) {
 } // anonymous namespace
 
 llvm::Expected>
-IncrementalCompilerBuilder::create(std::vector &ClangArgv) {
+IncrementalCompilerBuilder::create(std::string TT,
+   std::vector &ClangArgv) {
 
   // If we don't know ClangArgv0 or the address of main() at this point, try
   // to guess it anyway (it's possible on some platforms).
@@ -162,8 +163,7 @@ IncrementalCompilerBuilder::create(std::vector &ClangArgv) {
   TextDiagnosticBuffer *DiagsBuffer = new TextDiagnosticBuffer;
   DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagsBuffer);
 
-  driver::Driver Driver(/*MainBinaryName=*/ClangArgv[0],
-llvm::sys::getProcessTriple(), Diags);
+  driver::Driver Driver(/*MainBinaryName=*/ClangArgv[0], TT, Diags);
   Driver.setCheckInputsExist(false); // the input comes from mem buffers
   llvm::ArrayRef RF = llvm::ArrayRef(ClangArgv);
   std::unique_ptr 
Compilation(Driver.BuildCompilation(RF));
@@ -185,7 +185,8 @@ IncrementalCompilerBuilder::CreateCpp() {
   Argv.push_back("-xc++");
   Argv.insert(Argv.end(), UserArgs.begin(), UserArgs.end());
 
-  return IncrementalCompilerBuilder::create(Argv);
+  std::string TT = TargetTriple ? *TargetTriple : 
llvm::sys::getProcessTriple();
+  return IncrementalCompilerBuilder::create(TT, Argv);
 }
 
 llvm::Expected>
@@ -213,7 +214,8 @@ IncrementalCompilerBuilder::createCuda(bool device) {
 
   Argv.insert(Argv.end(), UserArgs.begin(), UserArgs.end());
 
-  return IncrementalCompilerBuilder::create(Argv);
+  std::string TT = TargetTriple ? *TargetTriple : 
llvm::sys::getProcessTriple();
+  return IncrementalCompilerBuilder::create(TT, Argv);
 }
 
 llvm::Expected>

diff  --git a/clang/unittests/Interpreter/CMakeLists.txt 
b/clang/unittests/Interpreter/CMakeLists.txt
index 712641afb976dd..0ddedb283e07d1 100644
--- a/clang/unittests/Interpreter/CMakeLists.txt
+++ b/clang/unittests/Interpreter/CMakeLists.txt
@@ -7,6 +7,7 @@ set(LLVM_LINK_COMPONENTS
   )
 
 add_clang_unittest(ClangReplInterpreterTests
+  IncrementalCompilerBuilderTest.cpp
   IncrementalProcessingTest.cpp
   InterpreterTest.cpp
   CodeCompletionTest.cpp

diff  --git a/clang/unittests/Interpreter/IncrementalCompilerBuilderTest.cpp 
b/clang/unittests/Interpreter/IncrementalCompilerBuilderTest.cpp
new file mode 100644
index 00..f729566f7efde6
--- /dev/null
+++ b/clang/unittests/Interpreter/IncrementalCompilerBuilderTest.cpp
@@ -0,0 +1,47 @@
+//=== unittests/Interpreter/IncrementalCompilerBuilderTest.cpp 
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Basic/TargetOptions.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Interpreter/Interpreter.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "llvm/

[clang] [clang-repl] Refactor locking of runtime PTU stack (NFC) (PR #84176)

2024-03-07 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail updated 
https://github.com/llvm/llvm-project/pull/84176

From ebf00ec8396eabf96c413e861b72ff1c88424685 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Wed, 6 Mar 2024 12:37:50 +0100
Subject: [PATCH] [clang-repl] Refactor locking of runtime PTU stack (NFC)

The previous implementation seemed hacky, because it required the reader to be 
familiar with the internal workings of the PTU stack. The concept itself is a 
pragmatic solution and not very surprising. Keeping it behind a finalization 
call seems reasonable.
---
 clang/include/clang/Interpreter/Interpreter.h |  1 +
 clang/lib/Interpreter/Interpreter.cpp | 12 
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/clang/include/clang/Interpreter/Interpreter.h 
b/clang/include/clang/Interpreter/Interpreter.h
index c8f932e95c4798..988ab86ccf3c8b 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -142,6 +142,7 @@ class Interpreter {
 
 private:
   size_t getEffectivePTUSize() const;
+  void markUserCodeStart();
 
   bool FindRuntimeInterface();
 
diff --git a/clang/lib/Interpreter/Interpreter.cpp 
b/clang/lib/Interpreter/Interpreter.cpp
index 37696b28976428..6410aadd1f5abe 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -280,15 +280,14 @@ Interpreter::create(std::unique_ptr CI) 
{
   if (Err)
 return std::move(Err);
 
+  // Add runtime code and set a marker to hide it from user code. Undo will not
+  // go through that.
   auto PTU = Interp->Parse(Runtimes);
   if (!PTU)
 return PTU.takeError();
+  Interp->markUserCodeStart();
 
   Interp->ValuePrintingInfo.resize(4);
-  // FIXME: This is a ugly hack. Undo command checks its availability by 
looking
-  // at the size of the PTU list. However we have parsed something in the
-  // beginning of the REPL so we have to mark them as 'Irrevocable'.
-  Interp->InitPTUSize = Interp->IncrParser->getPTUs().size();
   return std::move(Interp);
 }
 
@@ -345,6 +344,11 @@ const ASTContext &Interpreter::getASTContext() const {
   return getCompilerInstance()->getASTContext();
 }
 
+void Interpreter::markUserCodeStart() {
+  assert(!InitPTUSize && "We only do this once");
+  InitPTUSize = IncrParser->getPTUs().size();
+}
+
 size_t Interpreter::getEffectivePTUSize() const {
   std::list &PTUs = IncrParser->getPTUs();
   assert(PTUs.size() >= InitPTUSize && "empty PTU list?");

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Refactor locking of runtime PTU stack (NFC) (PR #84176)

2024-03-07 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail edited 
https://github.com/llvm/llvm-project/pull/84176
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Refactor locking of runtime PTU stack (NFC) (PR #84176)

2024-03-07 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail commented:

Rebased and reduced to refactoring NFC

https://github.com/llvm/llvm-project/pull/84176
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Refactor locking of runtime PTU stack (NFC) (PR #84176)

2024-03-07 Thread Stefan Gränitz via cfe-commits


@@ -137,9 +136,10 @@ class Interpreter {
 
   Expr *SynthesizeExpr(Expr *E);
 
-private:
   size_t getEffectivePTUSize() const;
+  void finalizeInitPTUStack();

weliveindetail wrote:

Yes, sounds goo. Done

https://github.com/llvm/llvm-project/pull/84176
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Refactor locking of runtime PTU stack (NFC) (PR #84176)

2024-03-07 Thread Stefan Gränitz via cfe-commits


@@ -136,6 +136,24 @@ TEST(InterpreterTest, DeclsAndStatements) {
   EXPECT_TRUE(!!R2);
 }
 
+TEST(InterpreterTest, PTUStack) {
+  clang::IncrementalCompilerBuilder CB;
+  auto CI = cantFail(CB.CreateCpp());
+
+  llvm::Error Err = llvm::Error::success();
+  auto Interp = std::make_unique(std::move(CI), Err);
+  cantFail(std::move(Err));
+
+  auto NumBuiltinPTUs = Interp->getEffectivePTUSize();
+  cantFail(Interp->Parse("void firstRuntimePTU() {}"));
+  EXPECT_EQ(NumBuiltinPTUs + 1, Interp->getEffectivePTUSize());

weliveindetail wrote:

Yes, that's right. In fact, we could just not expose it right now and instead 
keep this a NFC. Then we don't need a new test either.

https://github.com/llvm/llvm-project/pull/84176
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Expose RuntimeInterfaceBuilder to allow customization (PR #83126)

2024-03-07 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail updated 
https://github.com/llvm/llvm-project/pull/83126

From 8ba5253b20d1aef0a542506a667f6b66b84ac5b9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Fri, 8 Mar 2024 00:08:56 +0100
Subject: [PATCH] [clang-repl] Expose RuntimeInterfaceBuilder for
 customizations

---
 clang/include/clang/Interpreter/Interpreter.h |  35 ++-
 clang/lib/Interpreter/Interpreter.cpp | 247 ++
 clang/unittests/Interpreter/CMakeLists.txt|   1 +
 .../Interpreter/InterpreterExtensionsTest.cpp |  79 ++
 4 files changed, 253 insertions(+), 109 deletions(-)
 create mode 100644 clang/unittests/Interpreter/InterpreterExtensionsTest.cpp

diff --git a/clang/include/clang/Interpreter/Interpreter.h 
b/clang/include/clang/Interpreter/Interpreter.h
index c8f932e95c4798..d972d960dcb7cd 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -18,6 +18,7 @@
 #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"
@@ -75,17 +76,26 @@ 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);
+  virtual TransformExprFunction *getPrintValueTransformer() = 0;
+};
+
 /// Provides top-level interfaces for incremental compilation and execution.
 class Interpreter {
   std::unique_ptr TSCtx;
   std::unique_ptr IncrParser;
   std::unique_ptr IncrExecutor;
+  std::unique_ptr RuntimeIB;
 
   // An optional parser for CUDA offloading
   std::unique_ptr DeviceParser;
 
-  Interpreter(std::unique_ptr CI, llvm::Error &Err);
-
   llvm::Error CreateExecutor();
   unsigned InitPTUSize = 0;
 
@@ -94,8 +104,25 @@ 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;
+
+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 CI, llvm::Error &Err);
+
+  // 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 FindRuntimeInterface();
+
 public:
-  ~Interpreter();
+  virtual ~Interpreter();
+
   static llvm::Expected>
   create(std::unique_ptr CI);
   static llvm::Expected>
@@ -143,8 +170,6 @@ class Interpreter {
 private:
   size_t getEffectivePTUSize() const;
 
-  bool FindRuntimeInterface();
-
   llvm::DenseMap Dtors;
 
   llvm::SmallVector ValuePrintingInfo;
diff --git a/clang/lib/Interpreter/Interpreter.cpp 
b/clang/lib/Interpreter/Interpreter.cpp
index 37696b28976428..3485da8196683a 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -507,9 +507,13 @@ static constexpr llvm::StringRef MagicRuntimeInterface[] = 
{
 "__clang_Interpreter_SetValueWithAlloc",
 "__clang_Interpreter_SetValueCopyArr", "__ci_newtag"};
 
-bool Interpreter::FindRuntimeInterface() {
+static std::unique_ptr
+createInProcessRuntimeInterfaceBuilder(Interpreter &Interp, ASTContext &Ctx,
+   Sema &S);
+
+std::unique_ptr Interpreter::FindRuntimeInterface() {
   if (llvm::all_of(ValuePrintingInfo, [](Expr *E) { return E != nullptr; }))
-return true;
+return nullptr;
 
   Sema &S = getCompilerInstance()->getSema();
   ASTContext &Ctx = S.getASTContext();
@@ -528,120 +532,34 @@ bool Interpreter::FindRuntimeInterface() {
 
   if (!LookupInterface(ValuePrintingInfo[NoAlloc],
MagicRuntimeInterface[NoAlloc]))
-return false;
+return nullptr;
   if (!LookupInterface(ValuePrintingInfo[WithAlloc],
MagicRuntimeInterface[WithAlloc]))
-return false;
+return nullptr;
   if (!LookupInterface(ValuePrintingInfo[CopyArray],
MagicRuntimeInterface[CopyArray]))
-return false;
+return nullptr;
   if (!LookupInterface(ValuePrintingInfo[NewTag],
MagicRuntimeInterface[NewTag]))
-return false;
-  return true;
+return nullptr;
+
+  return createInProcessRuntimeInterfaceBuilder(*this, Ctx, S);
 }

[clang] [clang-repl] Expose RuntimeInterfaceBuilder to allow customization (PR #83126)

2024-03-07 Thread Stefan Gränitz via cfe-commits

weliveindetail wrote:

Squashed, rebased, polished and tested

https://github.com/llvm/llvm-project/pull/83126
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Expose CreateExecutor() and ResetExecutor() in extended Interpreter interface (PR #84460)

2024-03-08 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail created 
https://github.com/llvm/llvm-project/pull/84460

IncrementalExecutor is an implementation detail of the Interpreter. In order to 
test extended features properly, we must be able to setup and tear down the 
executor manually.

From 10f8d3eb040a5838d84d8a84fe636c7c2a7d196b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Fri, 8 Mar 2024 00:08:56 +0100
Subject: [PATCH 1/2] [clang-repl] Expose RuntimeInterfaceBuilder for
 customizations

---
 clang/include/clang/Interpreter/Interpreter.h |  35 ++-
 clang/lib/Interpreter/Interpreter.cpp | 247 ++
 clang/unittests/Interpreter/CMakeLists.txt|   1 +
 .../Interpreter/InterpreterExtensionsTest.cpp |  79 ++
 4 files changed, 253 insertions(+), 109 deletions(-)
 create mode 100644 clang/unittests/Interpreter/InterpreterExtensionsTest.cpp

diff --git a/clang/include/clang/Interpreter/Interpreter.h 
b/clang/include/clang/Interpreter/Interpreter.h
index c8f932e95c4798..d972d960dcb7cd 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -18,6 +18,7 @@
 #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"
@@ -75,17 +76,26 @@ 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);
+  virtual TransformExprFunction *getPrintValueTransformer() = 0;
+};
+
 /// Provides top-level interfaces for incremental compilation and execution.
 class Interpreter {
   std::unique_ptr TSCtx;
   std::unique_ptr IncrParser;
   std::unique_ptr IncrExecutor;
+  std::unique_ptr RuntimeIB;
 
   // An optional parser for CUDA offloading
   std::unique_ptr DeviceParser;
 
-  Interpreter(std::unique_ptr CI, llvm::Error &Err);
-
   llvm::Error CreateExecutor();
   unsigned InitPTUSize = 0;
 
@@ -94,8 +104,25 @@ 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;
+
+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 CI, llvm::Error &Err);
+
+  // 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 FindRuntimeInterface();
+
 public:
-  ~Interpreter();
+  virtual ~Interpreter();
+
   static llvm::Expected>
   create(std::unique_ptr CI);
   static llvm::Expected>
@@ -143,8 +170,6 @@ class Interpreter {
 private:
   size_t getEffectivePTUSize() const;
 
-  bool FindRuntimeInterface();
-
   llvm::DenseMap Dtors;
 
   llvm::SmallVector ValuePrintingInfo;
diff --git a/clang/lib/Interpreter/Interpreter.cpp 
b/clang/lib/Interpreter/Interpreter.cpp
index 37696b28976428..3485da8196683a 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -507,9 +507,13 @@ static constexpr llvm::StringRef MagicRuntimeInterface[] = 
{
 "__clang_Interpreter_SetValueWithAlloc",
 "__clang_Interpreter_SetValueCopyArr", "__ci_newtag"};
 
-bool Interpreter::FindRuntimeInterface() {
+static std::unique_ptr
+createInProcessRuntimeInterfaceBuilder(Interpreter &Interp, ASTContext &Ctx,
+   Sema &S);
+
+std::unique_ptr Interpreter::FindRuntimeInterface() {
   if (llvm::all_of(ValuePrintingInfo, [](Expr *E) { return E != nullptr; }))
-return true;
+return nullptr;
 
   Sema &S = getCompilerInstance()->getSema();
   ASTContext &Ctx = S.getASTContext();
@@ -528,120 +532,34 @@ bool Interpreter::FindRuntimeInterface() {
 
   if (!LookupInterface(ValuePrintingInfo[NoAlloc],
MagicRuntimeInterface[NoAlloc]))
-return false;
+return nullptr;
   if (!LookupInterface(ValuePrintingInfo[WithAlloc],
MagicRuntimeInterface[WithAlloc]))
-return false;
+return nullptr;
   if (!LookupInterface(ValuePrintingInfo[CopyArray],
MagicRuntimeInterface[CopyArray]))
-return false;
+return nullptr;
   if (!LookupInterface(ValuePrintingInfo[NewTag],

[clang] [clang-repl] Factor out CreateJITBuilder() and allow specialization in derived classes (PR #84461)

2024-03-08 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail created 
https://github.com/llvm/llvm-project/pull/84461

The LLJITBuilder interface provides a very convenient way to configure the 
ORCv2 JIT engine. IncrementalExecutor already used it internally to construct 
the JIT, but didn't provide external access. This patch lifts control of the 
creation process to the Interpreter and allows injection of a custom instance 
through the extended interface. The Interpreter's default behavior remains 
unchanged and the IncrementalExecutor remains an implementation detail.

From f87c7ccf59ad436087ada4be479102f7317d50ad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Fri, 8 Mar 2024 00:08:56 +0100
Subject: [PATCH 1/3] [clang-repl] Expose RuntimeInterfaceBuilder for
 customizations

---
 clang/include/clang/Interpreter/Interpreter.h |  35 ++-
 clang/lib/Interpreter/Interpreter.cpp | 247 ++
 clang/unittests/Interpreter/CMakeLists.txt|   1 +
 .../Interpreter/InterpreterExtensionsTest.cpp |  79 ++
 4 files changed, 253 insertions(+), 109 deletions(-)
 create mode 100644 clang/unittests/Interpreter/InterpreterExtensionsTest.cpp

diff --git a/clang/include/clang/Interpreter/Interpreter.h 
b/clang/include/clang/Interpreter/Interpreter.h
index c8f932e95c4798..d972d960dcb7cd 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -18,6 +18,7 @@
 #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"
@@ -75,17 +76,26 @@ 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);
+  virtual TransformExprFunction *getPrintValueTransformer() = 0;
+};
+
 /// Provides top-level interfaces for incremental compilation and execution.
 class Interpreter {
   std::unique_ptr TSCtx;
   std::unique_ptr IncrParser;
   std::unique_ptr IncrExecutor;
+  std::unique_ptr RuntimeIB;
 
   // An optional parser for CUDA offloading
   std::unique_ptr DeviceParser;
 
-  Interpreter(std::unique_ptr CI, llvm::Error &Err);
-
   llvm::Error CreateExecutor();
   unsigned InitPTUSize = 0;
 
@@ -94,8 +104,25 @@ 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;
+
+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 CI, llvm::Error &Err);
+
+  // 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 FindRuntimeInterface();
+
 public:
-  ~Interpreter();
+  virtual ~Interpreter();
+
   static llvm::Expected>
   create(std::unique_ptr CI);
   static llvm::Expected>
@@ -143,8 +170,6 @@ class Interpreter {
 private:
   size_t getEffectivePTUSize() const;
 
-  bool FindRuntimeInterface();
-
   llvm::DenseMap Dtors;
 
   llvm::SmallVector ValuePrintingInfo;
diff --git a/clang/lib/Interpreter/Interpreter.cpp 
b/clang/lib/Interpreter/Interpreter.cpp
index 37696b28976428..3485da8196683a 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -507,9 +507,13 @@ static constexpr llvm::StringRef MagicRuntimeInterface[] = 
{
 "__clang_Interpreter_SetValueWithAlloc",
 "__clang_Interpreter_SetValueCopyArr", "__ci_newtag"};
 
-bool Interpreter::FindRuntimeInterface() {
+static std::unique_ptr
+createInProcessRuntimeInterfaceBuilder(Interpreter &Interp, ASTContext &Ctx,
+   Sema &S);
+
+std::unique_ptr Interpreter::FindRuntimeInterface() {
   if (llvm::all_of(ValuePrintingInfo, [](Expr *E) { return E != nullptr; }))
-return true;
+return nullptr;
 
   Sema &S = getCompilerInstance()->getSema();
   ASTContext &Ctx = S.getASTContext();
@@ -528,120 +532,34 @@ bool Interpreter::FindRuntimeInterface() {
 
   if (!LookupInterface(ValuePrintingInfo[NoAlloc],
MagicRuntimeInterface[NoAlloc]))
-return false;
+return nullptr;
   if (!LookupInterface(ValuePrintingInfo[WithAlloc],

[clang] [clang-repl] Expose CreateExecutor() and ResetExecutor() in extended Interpreter interface (PR #84460)

2024-03-08 Thread Stefan Gränitz via cfe-commits

weliveindetail wrote:

This PR was factored out for isolated review and to reduce the size of the 
follow-up patch in https://github.com/llvm/llvm-project/pull/84461. It depends 
on the previous change in https://github.com/llvm/llvm-project/pull/83126. 
Combined patch size will shrink significantly, once it landed (tonight 
hopefully). Until then please refer to the single commit 
https://github.com/llvm/llvm-project/pull/84460/commits/023cab5a10e381533c9dc2a7c15823bb9bb9e54e

https://github.com/llvm/llvm-project/pull/84460
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Factor out CreateJITBuilder() and allow specialization in derived classes (PR #84461)

2024-03-08 Thread Stefan Gränitz via cfe-commits


@@ -373,21 +373,32 @@ Interpreter::Parse(llvm::StringRef Code) {
 static llvm::Expected
 createJITTargetMachineBuilder(const std::string &TT) {
   if (TT == llvm::sys::getProcessTriple())
+// This fails immediately if the target backend is not registered
 return llvm::orc::JITTargetMachineBuilder::detectHost();
-  // FIXME: This can fail as well if the target is not registered! We just 
don't
-  // catch it yet.
+
+  // If the target backend is not registered, LLJITBuilder::create() will fail
   return llvm::orc::JITTargetMachineBuilder(llvm::Triple(TT));
 }
 
+llvm::Expected>
+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() {
-  const clang::TargetInfo &TI =
-  getCompilerInstance()->getASTContext().getTargetInfo();
   if (IncrExecutor)
 return llvm::make_error("Operation failed. "
"Execution engine exists",
std::error_code());
+  llvm::Expected> JB =
+  CreateJITBuilder(*getCompilerInstance());

weliveindetail wrote:

We keep creating a new LLJITBuilder for each new IncrementalExecutor. This is 
questionable, but I didn't want to clutter the patch unnecessarily. I am happy 
to change that in a follow-up PR.

https://github.com/llvm/llvm-project/pull/84461
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Factor out CreateJITBuilder() and allow specialization in derived classes (PR #84461)

2024-03-08 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail commented:

This PR depends on two predecessor patches, but I wanted to share it for review 
already. For the moment please refer to the single commit in 
https://github.com/llvm/llvm-project/pull/84461/commits/cc46034e5288ba54dfc8a0ea7d54792cbb99227d
 for review.

https://github.com/llvm/llvm-project/pull/84461
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Factor out CreateJITBuilder() and allow specialization in derived classes (PR #84461)

2024-03-08 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail edited 
https://github.com/llvm/llvm-project/pull/84461
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Factor out CreateJITBuilder() and allow specialization in derived classes (PR #84461)

2024-03-08 Thread Stefan Gränitz via cfe-commits


@@ -102,4 +124,97 @@ TEST(InterpreterExtensionsTest, FindRuntimeInterface) {
   EXPECT_EQ(1U, Interp.RuntimeIBPtr->TransformerQueries);
 }
 
+class CustomJBInterpreter : public Interpreter {
+  using CustomJITBuilderCreatorFunction =
+  
std::function>()>;
+  CustomJITBuilderCreatorFunction JBCreator = nullptr;
+
+public:
+  CustomJBInterpreter(std::unique_ptr CI, llvm::Error 
&ErrOut)
+  : Interpreter(std::move(CI), ErrOut) {}
+
+  ~CustomJBInterpreter() override {
+// Skip cleanUp() because it would trigger LLJIT default dtors
+Interpreter::ResetExecutor();
+  }
+
+  void setCustomJITBuilderCreator(CustomJITBuilderCreatorFunction Fn) {
+JBCreator = std::move(Fn);
+  }
+
+  llvm::Expected>
+  CreateJITBuilder(CompilerInstance &CI) override {
+if (JBCreator)
+  return JBCreator();
+return Interpreter::CreateJITBuilder(CI);
+  }
+
+  llvm::Error CreateExecutor() { return Interpreter::CreateExecutor(); }
+};
+
+static void initArmTarget() {
+  static llvm::once_flag F;
+  llvm::call_once(F, [] {
+LLVMInitializeARMTarget();
+LLVMInitializeARMTargetInfo();
+LLVMInitializeARMTargetMC();
+LLVMInitializeARMAsmPrinter();
+  });
+}
+
+llvm::llvm_shutdown_obj Shutdown;
+
+TEST(InterpreterExtensionsTest, DefaultCrossJIT) {
+  IncrementalCompilerBuilder CB;
+  CB.SetTargetTriple("armv6-none-eabi");
+  auto CI = cantFail(CB.CreateCpp());
+  llvm::Error ErrOut = llvm::Error::success();
+  CustomJBInterpreter Interp(std::move(CI), ErrOut);
+  cantFail(std::move(ErrOut));
+
+  initArmTarget();
+  cantFail(Interp.CreateExecutor());
+}
+
+TEST(InterpreterExtensionsTest, CustomCrossJIT) {
+  std::string TargetTriple = "armv6-none-eabi";
+
+  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> Objs;
+  Interp.setCustomJITBuilderCreator([&]() {
+initArmTarget();
+auto JTMB = JITTargetMachineBuilder(llvm::Triple(TargetTriple));
+JTMB.setCPU("cortex-m0plus");
+auto JB = std::make_unique();
+JB->setJITTargetMachineBuilder(JTMB);
+JB->setPlatformSetUp(setUpInactivePlatform);
+JB->setNotifyCreatedCallback([&](LLJIT &J) {
+  ObjectLayer &ObjLayer = J.getObjLinkingLayer();
+  auto *JITLinkObjLayer = llvm::dyn_cast(&ObjLayer);

weliveindetail wrote:

We know that the ARM target uses JITLink, but we could also check explicitly. 
It seems nice to check that we produce some actual code. Maybe we want checks 
on the object code in the future? This might be a good template for such tests.

https://github.com/llvm/llvm-project/pull/84461
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Factor out CreateJITBuilder() and allow specialization in derived classes (PR #84461)

2024-03-08 Thread Stefan Gränitz via cfe-commits


@@ -102,4 +124,97 @@ TEST(InterpreterExtensionsTest, FindRuntimeInterface) {
   EXPECT_EQ(1U, Interp.RuntimeIBPtr->TransformerQueries);
 }
 
+class CustomJBInterpreter : public Interpreter {
+  using CustomJITBuilderCreatorFunction =
+  
std::function>()>;
+  CustomJITBuilderCreatorFunction JBCreator = nullptr;
+
+public:
+  CustomJBInterpreter(std::unique_ptr CI, llvm::Error 
&ErrOut)
+  : Interpreter(std::move(CI), ErrOut) {}
+
+  ~CustomJBInterpreter() override {
+// Skip cleanUp() because it would trigger LLJIT default dtors
+Interpreter::ResetExecutor();
+  }
+
+  void setCustomJITBuilderCreator(CustomJITBuilderCreatorFunction Fn) {
+JBCreator = std::move(Fn);
+  }
+
+  llvm::Expected>
+  CreateJITBuilder(CompilerInstance &CI) override {
+if (JBCreator)
+  return JBCreator();
+return Interpreter::CreateJITBuilder(CI);
+  }
+
+  llvm::Error CreateExecutor() { return Interpreter::CreateExecutor(); }
+};
+
+static void initArmTarget() {
+  static llvm::once_flag F;
+  llvm::call_once(F, [] {
+LLVMInitializeARMTarget();
+LLVMInitializeARMTargetInfo();
+LLVMInitializeARMTargetMC();
+LLVMInitializeARMAsmPrinter();
+  });
+}
+
+llvm::llvm_shutdown_obj Shutdown;
+
+TEST(InterpreterExtensionsTest, DefaultCrossJIT) {
+  IncrementalCompilerBuilder CB;
+  CB.SetTargetTriple("armv6-none-eabi");
+  auto CI = cantFail(CB.CreateCpp());
+  llvm::Error ErrOut = llvm::Error::success();
+  CustomJBInterpreter Interp(std::move(CI), ErrOut);
+  cantFail(std::move(ErrOut));
+
+  initArmTarget();
+  cantFail(Interp.CreateExecutor());
+}
+
+TEST(InterpreterExtensionsTest, CustomCrossJIT) {
+  std::string TargetTriple = "armv6-none-eabi";
+
+  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> Objs;
+  Interp.setCustomJITBuilderCreator([&]() {
+initArmTarget();
+auto JTMB = JITTargetMachineBuilder(llvm::Triple(TargetTriple));
+JTMB.setCPU("cortex-m0plus");
+auto JB = std::make_unique();
+JB->setJITTargetMachineBuilder(JTMB);
+JB->setPlatformSetUp(setUpInactivePlatform);
+JB->setNotifyCreatedCallback([&](LLJIT &J) {
+  ObjectLayer &ObjLayer = J.getObjLinkingLayer();
+  auto *JITLinkObjLayer = llvm::dyn_cast(&ObjLayer);
+  JITLinkObjLayer->setReturnObjectBuffer(
+  [&Objs](std::unique_ptr MB) {
+Objs.push_back(std::move(MB));
+  });
+  JIT = &J;
+  return llvm::Error::success();
+});
+return JB;
+  });
+
+  EXPECT_EQ(0U, Objs.size());
+  cantFail(Interp.CreateExecutor());
+  cantFail(Interp.ParseAndExecute("int a = 1;"));

weliveindetail wrote:

This works cross-platform, because we don't actually execute code. 
InactivePlatform suppresses execution of the respective initializers.

https://github.com/llvm/llvm-project/pull/84461
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Expose CreateExecutor() and ResetExecutor() in extended Interpreter interface (PR #84460)

2024-03-08 Thread Stefan Gränitz via cfe-commits

weliveindetail wrote:

> Do you mind incorporating this patch so that we avoid churn?

Sure. Essentially, this drops lazy creation of the executor and makes it 
dependent on the frontend action explicitly. Fine for me. We can still expose 
explicit setup/tear-down.

https://github.com/llvm/llvm-project/pull/84460
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Expose CreateExecutor() and ResetExecutor() in extended Interpreter interface (PR #84460)

2024-03-10 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail updated 
https://github.com/llvm/llvm-project/pull/84460

From 10f8d3eb040a5838d84d8a84fe636c7c2a7d196b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Fri, 8 Mar 2024 00:08:56 +0100
Subject: [PATCH 1/3] [clang-repl] Expose RuntimeInterfaceBuilder for
 customizations

---
 clang/include/clang/Interpreter/Interpreter.h |  35 ++-
 clang/lib/Interpreter/Interpreter.cpp | 247 ++
 clang/unittests/Interpreter/CMakeLists.txt|   1 +
 .../Interpreter/InterpreterExtensionsTest.cpp |  79 ++
 4 files changed, 253 insertions(+), 109 deletions(-)
 create mode 100644 clang/unittests/Interpreter/InterpreterExtensionsTest.cpp

diff --git a/clang/include/clang/Interpreter/Interpreter.h 
b/clang/include/clang/Interpreter/Interpreter.h
index c8f932e95c4798..d972d960dcb7cd 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -18,6 +18,7 @@
 #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"
@@ -75,17 +76,26 @@ 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);
+  virtual TransformExprFunction *getPrintValueTransformer() = 0;
+};
+
 /// Provides top-level interfaces for incremental compilation and execution.
 class Interpreter {
   std::unique_ptr TSCtx;
   std::unique_ptr IncrParser;
   std::unique_ptr IncrExecutor;
+  std::unique_ptr RuntimeIB;
 
   // An optional parser for CUDA offloading
   std::unique_ptr DeviceParser;
 
-  Interpreter(std::unique_ptr CI, llvm::Error &Err);
-
   llvm::Error CreateExecutor();
   unsigned InitPTUSize = 0;
 
@@ -94,8 +104,25 @@ 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;
+
+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 CI, llvm::Error &Err);
+
+  // 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 FindRuntimeInterface();
+
 public:
-  ~Interpreter();
+  virtual ~Interpreter();
+
   static llvm::Expected>
   create(std::unique_ptr CI);
   static llvm::Expected>
@@ -143,8 +170,6 @@ class Interpreter {
 private:
   size_t getEffectivePTUSize() const;
 
-  bool FindRuntimeInterface();
-
   llvm::DenseMap Dtors;
 
   llvm::SmallVector ValuePrintingInfo;
diff --git a/clang/lib/Interpreter/Interpreter.cpp 
b/clang/lib/Interpreter/Interpreter.cpp
index 37696b28976428..3485da8196683a 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -507,9 +507,13 @@ static constexpr llvm::StringRef MagicRuntimeInterface[] = 
{
 "__clang_Interpreter_SetValueWithAlloc",
 "__clang_Interpreter_SetValueCopyArr", "__ci_newtag"};
 
-bool Interpreter::FindRuntimeInterface() {
+static std::unique_ptr
+createInProcessRuntimeInterfaceBuilder(Interpreter &Interp, ASTContext &Ctx,
+   Sema &S);
+
+std::unique_ptr Interpreter::FindRuntimeInterface() {
   if (llvm::all_of(ValuePrintingInfo, [](Expr *E) { return E != nullptr; }))
-return true;
+return nullptr;
 
   Sema &S = getCompilerInstance()->getSema();
   ASTContext &Ctx = S.getASTContext();
@@ -528,120 +532,34 @@ bool Interpreter::FindRuntimeInterface() {
 
   if (!LookupInterface(ValuePrintingInfo[NoAlloc],
MagicRuntimeInterface[NoAlloc]))
-return false;
+return nullptr;
   if (!LookupInterface(ValuePrintingInfo[WithAlloc],
MagicRuntimeInterface[WithAlloc]))
-return false;
+return nullptr;
   if (!LookupInterface(ValuePrintingInfo[CopyArray],
MagicRuntimeInterface[CopyArray]))
-return false;
+return nullptr;
   if (!LookupInterface(ValuePrintingInfo[NewTag],
MagicRuntimeInterface[NewTag]))
-return false;
-  return true;
+return nullptr;
+
+  return createInProcessRuntimeInterfaceBuilder(*this, Ctx, S);

[clang] [clang-repl] Expose RuntimeInterfaceBuilder to allow customization (PR #83126)

2024-03-11 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail closed 
https://github.com/llvm/llvm-project/pull/83126
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Refactor locking of runtime PTU stack (NFC) (PR #84176)

2024-03-11 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail closed 
https://github.com/llvm/llvm-project/pull/84176
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Clang repl implicit executor create (PR #84758)

2024-03-11 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail created 
https://github.com/llvm/llvm-project/pull/84758

Until now the IncrExecutor is created lazily on the first execution request. In 
order to process the PTUs that come from initialization, we have to do it 
upfront implicitly.

From 2f7b523472543a92a97701880f8cd3c928061f53 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Sun, 10 Mar 2024 18:17:48 +0100
Subject: [PATCH 1/2] [clang-repl] Set up executor implicitly to account for
 init PTUs

---
 clang/lib/Interpreter/Interpreter.cpp | 30 +++
 clang/test/Interpreter/execute.cpp|  4 ++--
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/clang/lib/Interpreter/Interpreter.cpp 
b/clang/lib/Interpreter/Interpreter.cpp
index e293fefb524963..5e1161ca472b36 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -229,12 +229,30 @@ IncrementalCompilerBuilder::CreateCudaHost() {
 }
 
 Interpreter::Interpreter(std::unique_ptr CI,
- llvm::Error &Err) {
-  llvm::ErrorAsOutParameter EAO(&Err);
+ llvm::Error &ErrOut) {
+  llvm::ErrorAsOutParameter EAO(&ErrOut);
   auto LLVMCtx = std::make_unique();
   TSCtx = std::make_unique(std::move(LLVMCtx));
-  IncrParser = std::make_unique(*this, std::move(CI),
-   *TSCtx->getContext(), Err);
+  IncrParser = std::make_unique(
+  *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() {
@@ -375,6 +393,10 @@ Interpreter::Parse(llvm::StringRef Code) {
 llvm::Error Interpreter::CreateExecutor() {
   const clang::TargetInfo &TI =
   getCompilerInstance()->getASTContext().getTargetInfo();
+  if (!IncrParser->getCodeGen())
+return llvm::make_error("Operation failed. "
+   "No code generator available",
+   std::error_code());
   llvm::Error Err = llvm::Error::success();
   auto Executor = std::make_unique(*TSCtx, Err, TI);
   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
Date: Mon, 11 Mar 2024 14:10:58 +0100
Subject: [PATCH 2/2] [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)

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Set up executor implicitly to account for init PTUs (PR #84758)

2024-03-11 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail edited 
https://github.com/llvm/llvm-project/pull/84758
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Expose CreateExecutor() and ResetExecutor() in extended Interpreter interface (PR #84460)

2024-03-11 Thread Stefan Gränitz via cfe-commits

weliveindetail wrote:

> > Do you mind incorporating this patch so that we avoid churn?
> 
> Sure. Essentially, this drops lazy creation of the executor and makes it 
> dependent on the frontend action explicitly. Fine for me. We can still expose 
> explicit setup/tear-down.

@vgvassilev I moved it into the separate review 
https://github.com/llvm/llvm-project/pull/84758, because it has unexpected 
side-effects.

https://github.com/llvm/llvm-project/pull/84460
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Set up executor implicitly to account for init PTUs (PR #84758)

2024-03-11 Thread Stefan Gränitz via cfe-commits


@@ -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);

weliveindetail wrote:

With `-O2` this test fails unexpectedly now. Minimal repro:
```
clang-repl> struct A { int a; A(int a) : a(a) {} virtual ~A() {} };
clang-repl> A *a1 = new A(1);
clang-repl: llvm/include/llvm/ADT/SmallVector.h:308: const_reference 
llvm::SmallVectorTemplateCommon::operator[](size_type) 
const [T = llvm::PointerAlignElem]: Assertion `idx < size()' failed.
```

The following still works and thus I assume it's related to 
https://github.com/llvm/llvm-project/commit/c861d32d7c2791bdc058d9d9fbaecc1c2f07b8c7:
```
clang-repl> struct A { int a; A(int a) : a(a) {} virtual ~A() {} }; A *a1 = new 
A(1);
```

@hahnjo Maybe we now process init code that clashes with your above fix. Do you 
think that's possible? Any idea how to handle it? If possible, I'd like to 
submit the patch as-is with the test `XFAIL`ed for later investigation.

https://github.com/llvm/llvm-project/pull/84758
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Set up executor implicitly to account for init PTUs (PR #84758)

2024-03-12 Thread Stefan Gränitz via cfe-commits


@@ -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);

weliveindetail wrote:

> Do we even have initial PTUs in the default case?

Well, this test passes, if I comment out the for loop in the ctor that executes 
initial PTUs:
https://github.com/llvm/llvm-project/pull/84758/files#diff-b8484f1fc5b057f146ed5d9b6e2cd47c3f6f5ae879c7a0eee44f0a272581a88cR250-R254

> Also the minimal reproducer shows a more general version where the virtual 
> destructor is actually defined inline 
> (https://github.com/llvm/llvm-project/commit/c861d32d7c2791bdc058d9d9fbaecc1c2f07b8c7
>  addresses the case where it is out-of-line, which is special due to key 
> virtual functions).

Oh that's a good note, I had not considered the difference yet and actually 
they have different backtraces. Eventually, they both reach the same VTablePtr 
code though.

> I could not see the stack trace on osx. Can you paste it here?

Here is a diff (inline left, out-of-line right):
https://github.com/llvm/llvm-project/assets/7307454/41b64f92-83ea-4f82-b983-0b05e32ace42";>

> So if that breaks entirely (which is critical for us), I'm personally not ok 
> with just `XFAIL`ing it to land another change...

What breaks here is the parser and this patch doesn't even touch it. Not sure I 
am missing something, but it seems that it triggers a bug that always existed 
and just didn't show up so far.

https://github.com/llvm/llvm-project/pull/84758
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Set up executor implicitly to account for init PTUs (PR #84758)

2024-03-12 Thread Stefan Gränitz via cfe-commits


@@ -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);

weliveindetail wrote:

Let me share some more observations.

We **execute** the initial PTU and not only parse it. It shouldn't make a 
difference for the parser right? Still, I wasn't sure, especially since we 
unusally **only parse** runtime PTUs. So I made some experiments:
```
Init  Runtime  Dtor-def  |  New Delete  Automatic
(1) Execute   Execute   Out-of-line  |  fails   -   fails
 Inline  |  fails   -   once*

(2) Execute   Parse Out-of-line  |  fails   -   fails
 Inline  |  once*   fails   once

(3) Parse Any   Out-of-line  |  works   works   works
 Inline  |  works   works   works
```

Without `-O2` everything works.
Automatic = `A a1(1);`
`once` = Works exactly once (including shutdown)
`once*` = Same, but only if dtor has side-effects e.g. printf call

(1) is the behavior with this patch. (3) was the status-quo before this patch.

https://github.com/llvm/llvm-project/pull/84758
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Expose CreateExecutor() and ResetExecutor() in extended Interpreter interface (PR #84460)

2024-03-12 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail updated 
https://github.com/llvm/llvm-project/pull/84460

From a8c9cf8901d3fc24f3a2c2161b4a8796a7b28989 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Thu, 7 Mar 2024 23:04:22 +0100
Subject: [PATCH] [clang-repl] Expose CreateExecutor() and ResetExecutor() in
 extended Interpreter interface

---
 clang/include/clang/Interpreter/Interpreter.h |  9 ++-
 clang/lib/Interpreter/Interpreter.cpp |  6 +
 clang/unittests/Interpreter/CMakeLists.txt|  1 +
 .../Interpreter/InterpreterExtensionsTest.cpp | 24 +++
 4 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/clang/include/clang/Interpreter/Interpreter.h 
b/clang/include/clang/Interpreter/Interpreter.h
index 469ce1fd75bf84..1dcba1ef967980 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -96,7 +96,6 @@ class Interpreter {
   // An optional parser for CUDA offloading
   std::unique_ptr DeviceParser;
 
-  llvm::Error CreateExecutor();
   unsigned InitPTUSize = 0;
 
   // This member holds the last result of the value printing. It's a class
@@ -114,6 +113,14 @@ class Interpreter {
   // That's useful for testing and out-of-tree clients.
   Interpreter(std::unique_ptr CI, llvm::Error &Err);
 
+  // Create the internal IncrementalExecutor, or re-create it after calling
+  // ResetExecutor().
+  llvm::Error CreateExecutor();
+
+  // Delete the internal IncrementalExecutor. This causes a hard shutdown of 
the
+  // 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
diff --git a/clang/lib/Interpreter/Interpreter.cpp 
b/clang/lib/Interpreter/Interpreter.cpp
index e293fefb524963..7fa52f2f15fc49 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -375,6 +375,10 @@ Interpreter::Parse(llvm::StringRef Code) {
 llvm::Error Interpreter::CreateExecutor() {
   const clang::TargetInfo &TI =
   getCompilerInstance()->getASTContext().getTargetInfo();
+  if (IncrExecutor)
+return llvm::make_error("Operation failed. "
+   "Execution engine exists",
+   std::error_code());
   llvm::Error Err = llvm::Error::success();
   auto Executor = std::make_unique(*TSCtx, Err, TI);
   if (!Err)
@@ -383,6 +387,8 @@ llvm::Error Interpreter::CreateExecutor() {
   return Err;
 }
 
+void Interpreter::ResetExecutor() { IncrExecutor.reset(); }
+
 llvm::Error Interpreter::Execute(PartialTranslationUnit &T) {
   assert(T.TheModule);
   if (!IncrExecutor) {
diff --git a/clang/unittests/Interpreter/CMakeLists.txt 
b/clang/unittests/Interpreter/CMakeLists.txt
index 046d96ad0ec644..498070b43d922e 100644
--- a/clang/unittests/Interpreter/CMakeLists.txt
+++ b/clang/unittests/Interpreter/CMakeLists.txt
@@ -4,6 +4,7 @@ set(LLVM_LINK_COMPONENTS
   OrcJIT
   Support
   TargetParser
+  TestingSupport
   )
 
 add_clang_unittest(ClangReplInterpreterTests
diff --git a/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp 
b/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp
index 4e9f2dba210a37..f1c3d65ab0a95d 100644
--- a/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp
+++ b/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp
@@ -27,6 +27,30 @@
 using namespace clang;
 namespace {
 
+class TestCreateResetExecutor : public Interpreter {
+public:
+  TestCreateResetExecutor(std::unique_ptr CI,
+  llvm::Error &Err)
+  : Interpreter(std::move(CI), Err) {}
+
+  llvm::Error testCreateExecutor() { return Interpreter::CreateExecutor(); }
+
+  void resetExecutor() { Interpreter::ResetExecutor(); }
+};
+
+TEST(InterpreterExtensionsTest, ExecutorCreateReset) {
+  clang::IncrementalCompilerBuilder CB;
+  llvm::Error ErrOut = llvm::Error::success();
+  TestCreateResetExecutor Interp(cantFail(CB.CreateCpp()), ErrOut);
+  cantFail(std::move(ErrOut));
+  cantFail(Interp.testCreateExecutor());
+  Interp.resetExecutor();
+  cantFail(Interp.testCreateExecutor());
+  EXPECT_THAT_ERROR(Interp.testCreateExecutor(),
+llvm::FailedWithMessage("Operation failed. "
+"Execution engine exists"));
+}
+
 class RecordRuntimeIBMetrics : public Interpreter {
   struct NoopRuntimeInterfaceBuilder : public RuntimeInterfaceBuilder {
 NoopRuntimeInterfaceBuilder(Sema &S) : S(S) {}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Expose CreateExecutor() and ResetExecutor() in extended Interpreter interface (PR #84460)

2024-03-12 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail closed 
https://github.com/llvm/llvm-project/pull/84460
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Factor out CreateJITBuilder() and allow specialization in derived classes (PR #84461)

2024-03-12 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail updated 
https://github.com/llvm/llvm-project/pull/84461

From 88271c39b30b84041b4b7fb8b8f34c211d8190d5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Thu, 7 Mar 2024 23:04:22 +0100
Subject: [PATCH] [clang-repl] Add CreateJITBuilder() for specialization in
 derived classes

The LLJITBuilder interface provides a very convenient way to configure the JIT.
---
 clang/include/clang/Interpreter/Interpreter.h |   9 ++
 clang/lib/Interpreter/IncrementalExecutor.cpp |  33 ++---
 clang/lib/Interpreter/IncrementalExecutor.h   |   9 +-
 clang/lib/Interpreter/Interpreter.cpp |  26 +++-
 .../Interpreter/InterpreterExtensionsTest.cpp | 119 +-
 5 files changed, 175 insertions(+), 21 deletions(-)

diff --git a/clang/include/clang/Interpreter/Interpreter.h 
b/clang/include/clang/Interpreter/Interpreter.h
index 1dcba1ef967980..33ce4bbf5bea10 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -29,7 +29,9 @@
 
 namespace llvm {
 namespace orc {
+class JITTargetMachineBuilder;
 class LLJIT;
+class LLJITBuilder;
 class ThreadSafeContext;
 } // namespace orc
 } // namespace llvm
@@ -127,6 +129,13 @@ class Interpreter {
   // custom runtime.
   virtual std::unique_ptr 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>
+  CreateJITBuilder(CompilerInstance &CI);
+
 public:
   virtual ~Interpreter();
 
diff --git a/clang/lib/Interpreter/IncrementalExecutor.cpp 
b/clang/lib/Interpreter/IncrementalExecutor.cpp
index 40bcef94797d43..6f036107c14a9c 100644
--- a/clang/lib/Interpreter/IncrementalExecutor.cpp
+++ b/clang/lib/Interpreter/IncrementalExecutor.cpp
@@ -20,6 +20,7 @@
 #include "llvm/ExecutionEngine/Orc/Debugging/DebuggerSupport.h"
 #include "llvm/ExecutionEngine/Orc/ExecutionUtils.h"
 #include "llvm/ExecutionEngine/Orc/IRCompileLayer.h"
+#include "llvm/ExecutionEngine/Orc/JITTargetMachineBuilder.h"
 #include "llvm/ExecutionEngine/Orc/LLJIT.h"
 #include "llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h"
 #include "llvm/ExecutionEngine/Orc/TargetProcess/JITLoaderGDB.h"
@@ -36,26 +37,28 @@ LLVM_ATTRIBUTE_USED void linkComponents() {
 
 namespace clang {
 
+llvm::Expected>
+IncrementalExecutor::createDefaultJITBuilder(
+llvm::orc::JITTargetMachineBuilder JTMB) {
+  auto JITBuilder = std::make_unique();
+  JITBuilder->setJITTargetMachineBuilder(std::move(JTMB));
+  JITBuilder->setPrePlatformSetup([](llvm::orc::LLJIT &J) {
+// Try to enable debugging of JIT'd code (only works with JITLink for
+// ELF and MachO).
+consumeError(llvm::orc::enableDebuggerSupport(J));
+return llvm::Error::success();
+  });
+  return std::move(JITBuilder);
+}
+
 IncrementalExecutor::IncrementalExecutor(llvm::orc::ThreadSafeContext &TSC,
- llvm::Error &Err,
- const clang::TargetInfo &TI)
+ llvm::orc::LLJITBuilder &JITBuilder,
+ llvm::Error &Err)
 : TSCtx(TSC) {
   using namespace llvm::orc;
   llvm::ErrorAsOutParameter EAO(&Err);
 
-  auto JTMB = JITTargetMachineBuilder(TI.getTriple());
-  JTMB.addFeatures(TI.getTargetOpts().Features);
-  LLJITBuilder Builder;
-  Builder.setJITTargetMachineBuilder(JTMB);
-  Builder.setPrePlatformSetup(
-  [](LLJIT &J) {
-// Try to enable debugging of JIT'd code (only works with JITLink for
-// ELF and MachO).
-consumeError(enableDebuggerSupport(J));
-return llvm::Error::success();
-  });
-
-  if (auto JitOrErr = Builder.create())
+  if (auto JitOrErr = JITBuilder.create())
 Jit = std::move(*JitOrErr);
   else {
 Err = JitOrErr.takeError();
diff --git a/clang/lib/Interpreter/IncrementalExecutor.h 
b/clang/lib/Interpreter/IncrementalExecutor.h
index dd0a210a061415..b4347209e14fe3 100644
--- a/clang/lib/Interpreter/IncrementalExecutor.h
+++ b/clang/lib/Interpreter/IncrementalExecutor.h
@@ -23,7 +23,9 @@
 namespace llvm {
 class Error;
 namespace orc {
+class JITTargetMachineBuilder;
 class LLJIT;
+class LLJITBuilder;
 class ThreadSafeContext;
 } // namespace orc
 } // namespace llvm
@@ -44,8 +46,8 @@ class IncrementalExecutor {
 public:
   enum SymbolNameKind { IRName, LinkerName };
 
-  IncrementalExecutor(llvm::orc::ThreadSafeContext &TSC, llvm::Error &Err,
-  const clang::TargetInfo &TI);
+  IncrementalExecutor(llvm::orc::ThreadSafeContext &TSC,
+  llvm::orc::LLJITBuilder &JITBuilder, llvm::Error &Err);
   ~IncrementalExecutor();
 
   llvm::Error addModule(PartialTranslationUnit &PTU);
@@ -56,6 +58,9 @@ class IncrementalExecutor {
   getSymbolAddre

[clang] [clang-repl] Factor out CreateJITBuilder() and allow specialization in derived classes (PR #84461)

2024-03-12 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail edited 
https://github.com/llvm/llvm-project/pull/84461
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Expose CreateExecutor() and ResetExecutor() in extended Interpreter interface (PR #84460)

2024-03-12 Thread Stefan Gränitz via cfe-commits

weliveindetail wrote:

Build bot https://lab.llvm.org/buildbot/#/builders/196/builds/46799 detected an 
issue with the test -- I will try and push a fix quickly:
```
tools/clang/unittests/Interpreter/CMakeFiles/ClangReplInterpreterTests.dir/InterpreterExtensionsTest.cpp.o:InterpreterExtensionsTest.cpp:function
 (anonymous 
namespace)::InterpreterExtensionsTest_ExecutorCreateReset_Test::TestBody(): 
error: undefined reference to 'llvm::detail::TakeError(llvm::Error)'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
```

https://github.com/llvm/llvm-project/pull/84460
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] ffd31c5 - Fix build after #84460: link LLVMTestingSupport explicitly in clang unittest

2024-03-12 Thread Stefan Gränitz via cfe-commits

Author: Stefan Gränitz
Date: 2024-03-12T14:47:23+01:00
New Revision: ffd31c5e92da9da37cf57ca653e22db38b5af9a3

URL: 
https://github.com/llvm/llvm-project/commit/ffd31c5e92da9da37cf57ca653e22db38b5af9a3
DIFF: 
https://github.com/llvm/llvm-project/commit/ffd31c5e92da9da37cf57ca653e22db38b5af9a3.diff

LOG: Fix build after #84460: link LLVMTestingSupport explicitly in clang 
unittest

This is supposed to fix the DYLIB-enabled build, i.e. 
https://lab.llvm.org/buildbot/#/builders/196

Added: 


Modified: 
clang/unittests/Interpreter/CMakeLists.txt

Removed: 




diff  --git a/clang/unittests/Interpreter/CMakeLists.txt 
b/clang/unittests/Interpreter/CMakeLists.txt
index 498070b43d922e..b56e1e21015db9 100644
--- a/clang/unittests/Interpreter/CMakeLists.txt
+++ b/clang/unittests/Interpreter/CMakeLists.txt
@@ -4,7 +4,6 @@ set(LLVM_LINK_COMPONENTS
   OrcJIT
   Support
   TargetParser
-  TestingSupport
   )
 
 add_clang_unittest(ClangReplInterpreterTests
@@ -20,6 +19,7 @@ target_link_libraries(ClangReplInterpreterTests PUBLIC
   clangInterpreter
   clangFrontend
   clangSema
+  LLVMTestingSupport
   )
 
 # Exceptions on Windows are not yet supported.



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Expose CreateExecutor() and ResetExecutor() in extended Interpreter interface (PR #84460)

2024-03-12 Thread Stefan Gränitz via cfe-commits

weliveindetail wrote:

Yes, I think I can fix it quickly

https://github.com/llvm/llvm-project/pull/84460
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] d73c2d5 - Fix unittest after #84460: only applicable if the platform supports JIT

2024-03-12 Thread Stefan Gränitz via cfe-commits

Author: Stefan Gränitz
Date: 2024-03-12T23:14:23+01:00
New Revision: d73c2d5df21735805a1f46a85790db64c0615e1c

URL: 
https://github.com/llvm/llvm-project/commit/d73c2d5df21735805a1f46a85790db64c0615e1c
DIFF: 
https://github.com/llvm/llvm-project/commit/d73c2d5df21735805a1f46a85790db64c0615e1c.diff

LOG: Fix unittest after #84460: only applicable if the platform supports JIT

Added: 


Modified: 
clang/unittests/Interpreter/InterpreterExtensionsTest.cpp

Removed: 




diff  --git a/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp 
b/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp
index f1c3d65ab0a95d..77fd1b4e19816c 100644
--- a/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp
+++ b/clang/unittests/Interpreter/InterpreterExtensionsTest.cpp
@@ -17,7 +17,9 @@
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Sema.h"
 
+#include "llvm/ExecutionEngine/Orc/LLJIT.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/TargetSelect.h"
 #include "llvm/Testing/Support/Error.h"
 
 #include "gmock/gmock.h"
@@ -27,6 +29,22 @@
 using namespace clang;
 namespace {
 
+static bool HostSupportsJit() {
+  auto J = llvm::orc::LLJITBuilder().create();
+  if (J)
+return true;
+  LLVMConsumeError(llvm::wrap(J.takeError()));
+  return false;
+}
+
+struct LLVMInitRAII {
+  LLVMInitRAII() {
+llvm::InitializeNativeTarget();
+llvm::InitializeNativeTargetAsmPrinter();
+  }
+  ~LLVMInitRAII() { llvm::llvm_shutdown(); }
+} LLVMInit;
+
 class TestCreateResetExecutor : public Interpreter {
 public:
   TestCreateResetExecutor(std::unique_ptr CI,
@@ -39,6 +57,10 @@ class TestCreateResetExecutor : public Interpreter {
 };
 
 TEST(InterpreterExtensionsTest, ExecutorCreateReset) {
+  // 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);



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Expose CreateExecutor() and ResetExecutor() in extended Interpreter interface (PR #84460)

2024-03-12 Thread Stefan Gränitz via cfe-commits

weliveindetail wrote:

This should fix both: initializing the native target (your reported case) AND 
skipping the test on platforms that don't support JIT (other build bots 
complained).

https://github.com/llvm/llvm-project/pull/84460
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Expose RuntimeInterfaceBuilder to allow customization (PR #83126)

2024-02-27 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail created 
https://github.com/llvm/llvm-project/pull/83126

RuntimeInterfaceBuilder wires up JITed expressions with the hardcoded 
Interpreter runtime. It's used only for value printing right now, but it is not 
limited to that. The default implementation focuses on an evaluation process 
where the Interpreter has direct access to the memory of JITed expressions, 
because they share the same memory space (in-process or shared memory).

We need a different approach to support out-of-process evaluation or variations 
of the runtime. It seems reasonable to expose a minimal interface for it. The 
new RuntimeInterfaceBuilder is an abstract base class in the public header. For 
that, the TypeVisitor had to become a component (instead of inheriting from 
it). FindRuntimeInterface() was adjusted to return an instance of the 
RuntimeInterfaceBuilder and it can be overridden from derived classes.

From 3a6fdd006f00b0530e7fe6ead1fcd2765c1bfe07 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Mon, 26 Feb 2024 19:12:41 +0100
Subject: [PATCH 1/3] [clang-repl] Split out TypeVisitor from
 RuntimeInterfaceBuilder

---
 clang/lib/Interpreter/Interpreter.cpp | 171 ++
 1 file changed, 92 insertions(+), 79 deletions(-)

diff --git a/clang/lib/Interpreter/Interpreter.cpp 
b/clang/lib/Interpreter/Interpreter.cpp
index 9f97a3c6b0be9e..7afe55bbb266a3 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -541,21 +541,104 @@ bool Interpreter::FindRuntimeInterface() {
 
 namespace {
 
-class RuntimeInterfaceBuilder
-: public TypeVisitor {
-  clang::Interpreter &Interp;
+class InterfaceKindVisitor
+: public TypeVisitor {
+  friend class RuntimeInterfaceBuilder;
+
   ASTContext &Ctx;
   Sema &S;
   Expr *E;
   llvm::SmallVector Args;
 
+public:
+  InterfaceKindVisitor(ASTContext &Ctx, Sema &S, Expr *E)
+  : Ctx(Ctx), S(S), E(E) {}
+
+  Interpreter::InterfaceKind VisitRecordType(const RecordType *Ty) {
+return Interpreter::InterfaceKind::WithAlloc;
+  }
+
+  Interpreter::InterfaceKind
+  VisitMemberPointerType(const MemberPointerType *Ty) {
+return Interpreter::InterfaceKind::WithAlloc;
+  }
+
+  Interpreter::InterfaceKind
+  VisitConstantArrayType(const ConstantArrayType *Ty) {
+return Interpreter::InterfaceKind::CopyArray;
+  }
+
+  Interpreter::InterfaceKind
+  VisitFunctionProtoType(const FunctionProtoType *Ty) {
+HandlePtrType(Ty);
+return Interpreter::InterfaceKind::NoAlloc;
+  }
+
+  Interpreter::InterfaceKind VisitPointerType(const PointerType *Ty) {
+HandlePtrType(Ty);
+return Interpreter::InterfaceKind::NoAlloc;
+  }
+
+  Interpreter::InterfaceKind VisitReferenceType(const ReferenceType *Ty) {
+ExprResult AddrOfE = S.CreateBuiltinUnaryOp(SourceLocation(), UO_AddrOf, 
E);
+assert(!AddrOfE.isInvalid() && "Can not create unary expression");
+Args.push_back(AddrOfE.get());
+return Interpreter::InterfaceKind::NoAlloc;
+  }
+
+  Interpreter::InterfaceKind VisitBuiltinType(const BuiltinType *Ty) {
+if (Ty->isNullPtrType())
+  Args.push_back(E);
+else if (Ty->isFloatingType())
+  Args.push_back(E);
+else if (Ty->isIntegralOrEnumerationType())
+  HandleIntegralOrEnumType(Ty);
+else if (Ty->isVoidType()) {
+  // Do we need to still run `E`?
+}
+
+return Interpreter::InterfaceKind::NoAlloc;
+  }
+
+  Interpreter::InterfaceKind VisitEnumType(const EnumType *Ty) {
+HandleIntegralOrEnumType(Ty);
+return Interpreter::InterfaceKind::NoAlloc;
+  }
+
+private:
+  // Force cast these types to uint64 to reduce the number of overloads of
+  // `__clang_Interpreter_SetValueNoAlloc`.
+  void HandleIntegralOrEnumType(const Type *Ty) {
+TypeSourceInfo *TSI = Ctx.getTrivialTypeSourceInfo(Ctx.UnsignedLongLongTy);
+ExprResult CastedExpr =
+S.BuildCStyleCastExpr(SourceLocation(), TSI, SourceLocation(), E);
+assert(!CastedExpr.isInvalid() && "Cannot create cstyle cast expr");
+Args.push_back(CastedExpr.get());
+  }
+
+  void HandlePtrType(const Type *Ty) {
+TypeSourceInfo *TSI = Ctx.getTrivialTypeSourceInfo(Ctx.VoidPtrTy);
+ExprResult CastedExpr =
+S.BuildCStyleCastExpr(SourceLocation(), TSI, SourceLocation(), E);
+assert(!CastedExpr.isInvalid() && "Can not create cstyle cast expression");
+Args.push_back(CastedExpr.get());
+  }
+};
+
+class RuntimeInterfaceBuilder {
+  clang::Interpreter &Interp;
+  ASTContext &Ctx;
+  Sema &S;
+  Expr *E;
+  InterfaceKindVisitor Visitor;
+
 public:
   RuntimeInterfaceBuilder(clang::Interpreter &In, ASTContext &C, Sema &SemaRef,
   Expr *VE, ArrayRef FixedArgs)
-  : Interp(In), Ctx(C), S(SemaRef), E(VE) {
+  : Interp(In), Ctx(C), S(SemaRef), E(VE), Visitor(C, SemaRef, VE) {
 // The Interpreter* parameter and the out parameter `OutVal`.
 for (Expr *E : FixedArgs)
-  Args.push_back(E);
+  Visitor.Args.push

[clang] [clang-repl] Expose RuntimeInterfaceBuilder to allow customization (PR #83126)

2024-02-27 Thread Stefan Gränitz via cfe-commits

weliveindetail wrote:

This is an early proposal. I'd like to collect feedback before polishing. Let's 
focus on the conceptual questions. I tried to keep the patch minimal, so that 
it's easier to review. Each of the three commits depends on the previous ones. 
They should work and be reviewed in isolation (I guess). What do you think?

https://github.com/llvm/llvm-project/pull/83126
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Expose RuntimeInterfaceBuilder to allow customization (PR #83126)

2024-02-27 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail commented:



This is an early proposal. I'd like to collect feedback before polishing. Let's 
focus on the conceptual questions. I tried to keep the patch minimal, so that 
it's easier to review. Each of the three commits depends on the previous ones. 
They should work and be reviewed in isolation (I guess). What do you think?

https://github.com/llvm/llvm-project/pull/83126
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Expose RuntimeInterfaceBuilder to allow customization (PR #83126)

2024-02-27 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail edited 
https://github.com/llvm/llvm-project/pull/83126
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Expose RuntimeInterfaceBuilder to allow customization (PR #83126)

2024-02-27 Thread Stefan Gränitz via cfe-commits


@@ -741,9 +775,8 @@ Expr *Interpreter::SynthesizeExpr(Expr *E) {
   auto *OutValue = CStyleCastPtrExpr(S, Ctx.VoidPtrTy, (uintptr_t)&LastValue);
 
   // Build `__clang_Interpreter_SetValue*` call.
-  RuntimeInterfaceBuilder Builder(*this, Ctx, S, E, {ThisInterp, OutValue});
+  ExprResult Result = Builder->getCall(E, {ThisInterp, OutValue});

weliveindetail wrote:

FindRuntimeInterface() and getCall() are virtual functions now, which 
introduces a tiny runtime overhead for each synthesized expression. It should 
be negligible compared to the effort for parsing and compilation, but if it's 
an issue, I am happy to change it to a call to function pointer.

https://github.com/llvm/llvm-project/pull/83126
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Expose RuntimeInterfaceBuilder to allow customization (PR #83126)

2024-02-28 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail updated 
https://github.com/llvm/llvm-project/pull/83126

From 3a6fdd006f00b0530e7fe6ead1fcd2765c1bfe07 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Mon, 26 Feb 2024 19:12:41 +0100
Subject: [PATCH 1/4] [clang-repl] Split out TypeVisitor from
 RuntimeInterfaceBuilder

---
 clang/lib/Interpreter/Interpreter.cpp | 171 ++
 1 file changed, 92 insertions(+), 79 deletions(-)

diff --git a/clang/lib/Interpreter/Interpreter.cpp 
b/clang/lib/Interpreter/Interpreter.cpp
index 9f97a3c6b0be9e..7afe55bbb266a3 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -541,21 +541,104 @@ bool Interpreter::FindRuntimeInterface() {
 
 namespace {
 
-class RuntimeInterfaceBuilder
-: public TypeVisitor {
-  clang::Interpreter &Interp;
+class InterfaceKindVisitor
+: public TypeVisitor {
+  friend class RuntimeInterfaceBuilder;
+
   ASTContext &Ctx;
   Sema &S;
   Expr *E;
   llvm::SmallVector Args;
 
+public:
+  InterfaceKindVisitor(ASTContext &Ctx, Sema &S, Expr *E)
+  : Ctx(Ctx), S(S), E(E) {}
+
+  Interpreter::InterfaceKind VisitRecordType(const RecordType *Ty) {
+return Interpreter::InterfaceKind::WithAlloc;
+  }
+
+  Interpreter::InterfaceKind
+  VisitMemberPointerType(const MemberPointerType *Ty) {
+return Interpreter::InterfaceKind::WithAlloc;
+  }
+
+  Interpreter::InterfaceKind
+  VisitConstantArrayType(const ConstantArrayType *Ty) {
+return Interpreter::InterfaceKind::CopyArray;
+  }
+
+  Interpreter::InterfaceKind
+  VisitFunctionProtoType(const FunctionProtoType *Ty) {
+HandlePtrType(Ty);
+return Interpreter::InterfaceKind::NoAlloc;
+  }
+
+  Interpreter::InterfaceKind VisitPointerType(const PointerType *Ty) {
+HandlePtrType(Ty);
+return Interpreter::InterfaceKind::NoAlloc;
+  }
+
+  Interpreter::InterfaceKind VisitReferenceType(const ReferenceType *Ty) {
+ExprResult AddrOfE = S.CreateBuiltinUnaryOp(SourceLocation(), UO_AddrOf, 
E);
+assert(!AddrOfE.isInvalid() && "Can not create unary expression");
+Args.push_back(AddrOfE.get());
+return Interpreter::InterfaceKind::NoAlloc;
+  }
+
+  Interpreter::InterfaceKind VisitBuiltinType(const BuiltinType *Ty) {
+if (Ty->isNullPtrType())
+  Args.push_back(E);
+else if (Ty->isFloatingType())
+  Args.push_back(E);
+else if (Ty->isIntegralOrEnumerationType())
+  HandleIntegralOrEnumType(Ty);
+else if (Ty->isVoidType()) {
+  // Do we need to still run `E`?
+}
+
+return Interpreter::InterfaceKind::NoAlloc;
+  }
+
+  Interpreter::InterfaceKind VisitEnumType(const EnumType *Ty) {
+HandleIntegralOrEnumType(Ty);
+return Interpreter::InterfaceKind::NoAlloc;
+  }
+
+private:
+  // Force cast these types to uint64 to reduce the number of overloads of
+  // `__clang_Interpreter_SetValueNoAlloc`.
+  void HandleIntegralOrEnumType(const Type *Ty) {
+TypeSourceInfo *TSI = Ctx.getTrivialTypeSourceInfo(Ctx.UnsignedLongLongTy);
+ExprResult CastedExpr =
+S.BuildCStyleCastExpr(SourceLocation(), TSI, SourceLocation(), E);
+assert(!CastedExpr.isInvalid() && "Cannot create cstyle cast expr");
+Args.push_back(CastedExpr.get());
+  }
+
+  void HandlePtrType(const Type *Ty) {
+TypeSourceInfo *TSI = Ctx.getTrivialTypeSourceInfo(Ctx.VoidPtrTy);
+ExprResult CastedExpr =
+S.BuildCStyleCastExpr(SourceLocation(), TSI, SourceLocation(), E);
+assert(!CastedExpr.isInvalid() && "Can not create cstyle cast expression");
+Args.push_back(CastedExpr.get());
+  }
+};
+
+class RuntimeInterfaceBuilder {
+  clang::Interpreter &Interp;
+  ASTContext &Ctx;
+  Sema &S;
+  Expr *E;
+  InterfaceKindVisitor Visitor;
+
 public:
   RuntimeInterfaceBuilder(clang::Interpreter &In, ASTContext &C, Sema &SemaRef,
   Expr *VE, ArrayRef FixedArgs)
-  : Interp(In), Ctx(C), S(SemaRef), E(VE) {
+  : Interp(In), Ctx(C), S(SemaRef), E(VE), Visitor(C, SemaRef, VE) {
 // The Interpreter* parameter and the out parameter `OutVal`.
 for (Expr *E : FixedArgs)
-  Args.push_back(E);
+  Visitor.Args.push_back(E);
 
 // Get rid of ExprWithCleanups.
 if (auto *EWC = llvm::dyn_cast_if_present(E))
@@ -575,11 +658,11 @@ class RuntimeInterfaceBuilder
 Expr *TypeArg =
 CStyleCastPtrExpr(S, Ctx.VoidPtrTy, (uintptr_t)Ty.getAsOpaquePtr());
 // The QualType parameter `OpaqueType`, represented as `void*`.
-Args.push_back(TypeArg);
+Visitor.Args.push_back(TypeArg);
 
 // We push the last parameter based on the type of the Expr. Note we need
 // special care for rvalue struct.
-Interpreter::InterfaceKind Kind = Visit(&*DesugaredTy);
+Interpreter::InterfaceKind Kind = Visitor.Visit(&*DesugaredTy);
 switch (Kind) {
 case Interpreter::InterfaceKind::WithAlloc:
 case Interpreter::InterfaceKind::CopyArray: {
@@ -587,7 +670,7 @@ class RuntimeInterfaceBuilder
   ExprResult AllocCall = S

[clang] [clang-repl] Expose RuntimeInterfaceBuilder to allow customization (PR #83126)

2024-02-28 Thread Stefan Gränitz via cfe-commits

weliveindetail wrote:

Alright. The above change makes it as cheap as it gets with an abstraction: 
virtual function calls only on init and one function pointer call for each 
processed statement.

> I was wondering if we can extend that functionality via a callback mechanism 

How would that look like? I assume the runtime interface will have other 
functions than `getCall()` in the future right? Then we need to keep it as an 
entity. What is the difference between a callback and the virtual function call 
from the first patch?

We can change it to provide function pointers instead like in my last patch. 
It's less idiomatic but a bit cheaper. Of course we could use `std::function` 
callbacks and capturing lambdas instead, but I am afraid that would be even 
more expensive than the virtual function proposal.

> We also have some pending work (which I was planning on working when time 
> permits) in that area [D146809](https://reviews.llvm.org/D146809). I was 
> wondering if that'd interfere with your work...

Your patch doesn't actually touch the `RuntimeInterfaceBuilder`, so this should 
be no problem.


https://github.com/llvm/llvm-project/pull/83126
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-repl] Expose RuntimeInterfaceBuilder to allow customization (PR #83126)

2024-02-28 Thread Stefan Gränitz via cfe-commits

https://github.com/weliveindetail updated 
https://github.com/llvm/llvm-project/pull/83126

From 3a6fdd006f00b0530e7fe6ead1fcd2765c1bfe07 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= 
Date: Mon, 26 Feb 2024 19:12:41 +0100
Subject: [PATCH 1/5] [clang-repl] Split out TypeVisitor from
 RuntimeInterfaceBuilder

---
 clang/lib/Interpreter/Interpreter.cpp | 171 ++
 1 file changed, 92 insertions(+), 79 deletions(-)

diff --git a/clang/lib/Interpreter/Interpreter.cpp 
b/clang/lib/Interpreter/Interpreter.cpp
index 9f97a3c6b0be9e..7afe55bbb266a3 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -541,21 +541,104 @@ bool Interpreter::FindRuntimeInterface() {
 
 namespace {
 
-class RuntimeInterfaceBuilder
-: public TypeVisitor {
-  clang::Interpreter &Interp;
+class InterfaceKindVisitor
+: public TypeVisitor {
+  friend class RuntimeInterfaceBuilder;
+
   ASTContext &Ctx;
   Sema &S;
   Expr *E;
   llvm::SmallVector Args;
 
+public:
+  InterfaceKindVisitor(ASTContext &Ctx, Sema &S, Expr *E)
+  : Ctx(Ctx), S(S), E(E) {}
+
+  Interpreter::InterfaceKind VisitRecordType(const RecordType *Ty) {
+return Interpreter::InterfaceKind::WithAlloc;
+  }
+
+  Interpreter::InterfaceKind
+  VisitMemberPointerType(const MemberPointerType *Ty) {
+return Interpreter::InterfaceKind::WithAlloc;
+  }
+
+  Interpreter::InterfaceKind
+  VisitConstantArrayType(const ConstantArrayType *Ty) {
+return Interpreter::InterfaceKind::CopyArray;
+  }
+
+  Interpreter::InterfaceKind
+  VisitFunctionProtoType(const FunctionProtoType *Ty) {
+HandlePtrType(Ty);
+return Interpreter::InterfaceKind::NoAlloc;
+  }
+
+  Interpreter::InterfaceKind VisitPointerType(const PointerType *Ty) {
+HandlePtrType(Ty);
+return Interpreter::InterfaceKind::NoAlloc;
+  }
+
+  Interpreter::InterfaceKind VisitReferenceType(const ReferenceType *Ty) {
+ExprResult AddrOfE = S.CreateBuiltinUnaryOp(SourceLocation(), UO_AddrOf, 
E);
+assert(!AddrOfE.isInvalid() && "Can not create unary expression");
+Args.push_back(AddrOfE.get());
+return Interpreter::InterfaceKind::NoAlloc;
+  }
+
+  Interpreter::InterfaceKind VisitBuiltinType(const BuiltinType *Ty) {
+if (Ty->isNullPtrType())
+  Args.push_back(E);
+else if (Ty->isFloatingType())
+  Args.push_back(E);
+else if (Ty->isIntegralOrEnumerationType())
+  HandleIntegralOrEnumType(Ty);
+else if (Ty->isVoidType()) {
+  // Do we need to still run `E`?
+}
+
+return Interpreter::InterfaceKind::NoAlloc;
+  }
+
+  Interpreter::InterfaceKind VisitEnumType(const EnumType *Ty) {
+HandleIntegralOrEnumType(Ty);
+return Interpreter::InterfaceKind::NoAlloc;
+  }
+
+private:
+  // Force cast these types to uint64 to reduce the number of overloads of
+  // `__clang_Interpreter_SetValueNoAlloc`.
+  void HandleIntegralOrEnumType(const Type *Ty) {
+TypeSourceInfo *TSI = Ctx.getTrivialTypeSourceInfo(Ctx.UnsignedLongLongTy);
+ExprResult CastedExpr =
+S.BuildCStyleCastExpr(SourceLocation(), TSI, SourceLocation(), E);
+assert(!CastedExpr.isInvalid() && "Cannot create cstyle cast expr");
+Args.push_back(CastedExpr.get());
+  }
+
+  void HandlePtrType(const Type *Ty) {
+TypeSourceInfo *TSI = Ctx.getTrivialTypeSourceInfo(Ctx.VoidPtrTy);
+ExprResult CastedExpr =
+S.BuildCStyleCastExpr(SourceLocation(), TSI, SourceLocation(), E);
+assert(!CastedExpr.isInvalid() && "Can not create cstyle cast expression");
+Args.push_back(CastedExpr.get());
+  }
+};
+
+class RuntimeInterfaceBuilder {
+  clang::Interpreter &Interp;
+  ASTContext &Ctx;
+  Sema &S;
+  Expr *E;
+  InterfaceKindVisitor Visitor;
+
 public:
   RuntimeInterfaceBuilder(clang::Interpreter &In, ASTContext &C, Sema &SemaRef,
   Expr *VE, ArrayRef FixedArgs)
-  : Interp(In), Ctx(C), S(SemaRef), E(VE) {
+  : Interp(In), Ctx(C), S(SemaRef), E(VE), Visitor(C, SemaRef, VE) {
 // The Interpreter* parameter and the out parameter `OutVal`.
 for (Expr *E : FixedArgs)
-  Args.push_back(E);
+  Visitor.Args.push_back(E);
 
 // Get rid of ExprWithCleanups.
 if (auto *EWC = llvm::dyn_cast_if_present(E))
@@ -575,11 +658,11 @@ class RuntimeInterfaceBuilder
 Expr *TypeArg =
 CStyleCastPtrExpr(S, Ctx.VoidPtrTy, (uintptr_t)Ty.getAsOpaquePtr());
 // The QualType parameter `OpaqueType`, represented as `void*`.
-Args.push_back(TypeArg);
+Visitor.Args.push_back(TypeArg);
 
 // We push the last parameter based on the type of the Expr. Note we need
 // special care for rvalue struct.
-Interpreter::InterfaceKind Kind = Visit(&*DesugaredTy);
+Interpreter::InterfaceKind Kind = Visitor.Visit(&*DesugaredTy);
 switch (Kind) {
 case Interpreter::InterfaceKind::WithAlloc:
 case Interpreter::InterfaceKind::CopyArray: {
@@ -587,7 +670,7 @@ class RuntimeInterfaceBuilder
   ExprResult AllocCall = S

  1   2   >