[clang] [clang-repl] Enable native CPU detection by default (PR #77491)
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)
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)
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)
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
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)
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)
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)
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)
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)
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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
@@ -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)
@@ -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)
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)
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)
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)
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)
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)
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)
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)
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)
@@ -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)
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)
@@ -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)
@@ -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)
@@ -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)
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)
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)
@@ -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)
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)
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)
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)
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)
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)
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)
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)
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)
@@ -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)
@@ -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)
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)
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)
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)
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)
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)
@@ -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)
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)
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)
@@ -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)
@@ -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)
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)
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)
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)
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)
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)
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)
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)
@@ -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)
@@ -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)
@@ -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)
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)
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)
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)
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)
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
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)
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
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)
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)
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)
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)
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)
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)
@@ -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)
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)
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)
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