[PATCH] D55128: [CMake] Store path to vendor-specific headers in clang-headers target property

sgraenitz created this revision. sgraenitz added reviewers: aprantl, JDevlieghere, davide, friss, dexonsmith. Herald added a subscriber: mgorny. LLDB.framework wants a copy these headers. With this change LLDB can easily glob for the list of files: get_target_property(clang_include_dir

[PATCH] D55891: [compiler-rt] [xray] [tests] Detect and handle missing LLVMTestingSupport gracefully

sgraenitz added a comment. In case compiler-rt abandons `llvm-config` in favor of `find_package(LLVM)` one day, we could drop the `COMPILER_RT_HAS_LLVMTESTINGSUPPORT` logic here and use imported target properties instead. It seems a cleaner solution to me and avoids issues like D57521

[PATCH] D57521: [CMake] External compiler-rt-configure requires LLVMTestingSupport when including tests

sgraenitz added a comment. In D57521#1379837 , @beanz wrote: > @phosek, does LLVM’s runtime directory do this correctly? > > At some point we should try and deprecate and remove this option in favor of > the LLVM directory because the LLVM approach is

[PATCH] D57521: [CMake] External compiler-rt-configure requires LLVMTestingSupport when including tests

sgraenitz created this revision. sgraenitz added reviewers: ab, beanz. Herald added subscribers: erik.pilkington, mgorny, dberris. Apparently `LLVMTestingSupport` must be built before `llvm-config` can be asked for it. Symptom with `LLVM_INCLUDE_TESTS=ON` is: $

[PATCH] D57521: [CMake] External compiler-rt-configure requires LLVMTestingSupport when including tests

sgraenitz updated this revision to Diff 184531. sgraenitz added a comment. Polishing Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57521/new/ https://reviews.llvm.org/D57521 Files: runtime/CMakeLists.txt Index: runtime/CMakeLists.txt

[PATCH] D57521: [CMake] External compiler-rt-configure requires LLVMTestingSupport when including tests

sgraenitz added a comment. FYI: Seems to be happening since D55891 Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57521/new/ https://reviews.llvm.org/D57521 ___ cfe-commits mailing

[PATCH] D61438: [ASTImporter] Use llvm::Expected and Error in the importer API

sgraenitz added inline comments. Comment at: lldb/source/Symbol/ClangASTImporter.cpp:68 + return *ret_or_error; +} else { + Log *log = aprantl wrote: > The `else` is redundant. Here it's necessary for the scope of `ret_or_error`. That's a bit

[PATCH] D61438: [ASTImporter] Use llvm::Expected and Error in the importer API

sgraenitz added inline comments. Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp:1980 + } + return *type_or_error; } >>! In D61438#1490102, @jingham wrote: > [...] include the contents of that error n the log message? e.g: ``` if

[PATCH] D104898: [clang-repl] Allow passing in code as positional arguments.

sgraenitz accepted this revision. sgraenitz added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/test/Interpreter/execute.cpp:1 // RUN: cat %s | clang-repl | FileCheck %s +// RUN: clang-repl "int i = 10;" 'extern "C" int printf(const

[PATCH] D104918: [clang-repl] Implement partial translation units and error recovery.

sgraenitz added a comment. A few minor notes Comment at: clang/include/clang/Interpreter/Interpreter.h:63 +if (auto Err = ErrOrPTU.takeError()) return Err; +if (ErrOrPTU->TheModule) `ErrorOr` has different semantics and is still in use, so the

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

sgraenitz accepted this revision. sgraenitz added a comment. Thanks. From my side this looks good now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96033/new/ https://reviews.llvm.org/D96033 ___ cfe-commits mailing list

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

sgraenitz added a comment. I think this makes good progress. I found two details in the test code that need attention. The stdout issue might be done now or in a follow-up patch some time soon. Otherwise, this seems to get ready to land. @teemperor What about your notes. Are there any open

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

sgraenitz added a subscriber: anarazel. sgraenitz added a comment. Hi Vassil, thanks for upstreaming this! I think it goes into a good direction. The last time I looked at the Cling sources downstream, it was based on LLVM release 5.0. The IncrementalJIT class was based on what we call OrcV1

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

sgraenitz added a comment. @v.g.vassilev Great to see this getting upstreamed! @teemperor Thanks for adding, I will take a look in the next days. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96033/new/ https://reviews.llvm.org/D96033 ___

[PATCH] D110260: [ORC] Minor renaming and typo fixes (NFC)

sgraenitz added a comment. Thanks @xgupta for your note! The parameter passed to `EPCIndirectionUtils::Create()` in the example was referencing a moved-from value. This caused the segfault. Unfortunately, the examples don't have good test coverage so far. Repository: rG LLVM Github

[PATCH] D110260: [ORC] Minor renaming and typo fixes (NFC)

sgraenitz added a comment. FYI Included one more typo fix in ExecutorAddress.h Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110260/new/ https://reviews.llvm.org/D110260 ___ cfe-commits mailing list

[PATCH] D110260: [ORC] Minor renaming and typo fixes (NFC)

sgraenitz added a comment. In D110260#3018412 , @xgupta wrote: > Yeah we need testcases, now it says > > JIT session error: Symbols not found: [ return1 ] > JIT session error: Failed to materialize symbols: { (main, { foo_body }) } > Unable to lazily

[PATCH] D110260: [ORC] Minor renaming and typo fixes (NFC)

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG767b328e506e: [ORC] Minor renaming and typo fixes (NFC) (authored by sgraenitz). Changed prior to commit:

[PATCH] D110260: [ORC] Minor renaming and typo fixes (NFC)

sgraenitz created this revision. sgraenitz added a reviewer: lhames. Herald added subscribers: hiraditya, mgorny. sgraenitz requested review of this revision. Herald added projects: clang, LLVM. Herald added a subscriber: cfe-commits. One typo, one unsused include and some leftovers from the

[PATCH] D110484: [clang-repl] Allow loading of plugins in clang-repl.

sgraenitz added a comment. Two minor comments. Otherwise LGTM. Comment at: clang/include/clang/Frontend/CompilerInstance.h:223 + void LoadRequestedPlugins(); + /// } This could have a minimal doxygen comment now that it's part of the public interface.

[PATCH] D124762: [WinEHPrepare] Avoid truncation of EH funclets with GNUstep ObjC runtime

sgraenitz updated this revision to Diff 434024. sgraenitz added a comment. Clang frontend: check that 'funclet' bundle operands are emitted for Pre-ISel intrinsics Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124762/new/

[PATCH] D124762: [WinEHPrepare] Avoid truncation of EH funclets with GNUstep ObjC runtime

sgraenitz marked an inline comment as done. sgraenitz added a comment. My follow-up got delayed, because I hit another bug, which appears to be a regression in release 14. This is why I wrote the tests for release/13.x and I still have to port them back to mainline, so this is *not yet ready to

[PATCH] D124762: [WinEHPrepare] Avoid truncation of EH funclets with GNUstep ObjC runtime

sgraenitz updated this revision to Diff 434020. sgraenitz added a comment. Fix unchecked nullptr compiler crash and assertion Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124762/new/ https://reviews.llvm.org/D124762 Files:

[PATCH] D124762: [WinEHPrepare] Avoid truncation of EH funclets with GNUstep ObjC runtime

sgraenitz updated this revision to Diff 434023. sgraenitz added a comment. Herald added subscribers: jsji, pengfei. LLVM CodeGen: check that presence of bundle operands avoids truncation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124762/new/

[PATCH] D124762: [WinEHPrepare] Avoid truncation of EH funclets with GNUstep ObjC runtime

sgraenitz added a comment. For illustration, truncations look like this: F23303573: Screenshot 2022-06-02 at 15.35.19.png F23303572: Screenshot 2022-06-02 at 19.06.41.png Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D124762: [WinEHPrepare] Avoid truncation of EH funclets with GNUstep ObjC runtime

sgraenitz updated this revision to Diff 437179. sgraenitz added a comment. Fix accidental functional change that failed Clang test CodeGenCXX/microsoft-abi-eh-inlineasm.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124762/new/

[PATCH] D124762: [WinEHPrepare] Avoid truncation of EH funclets with GNUstep ObjC runtime

sgraenitz updated this revision to Diff 437180. sgraenitz added a comment. Drop GNUstep assertion from getBundlesForFunclet(). Indeed a similar issue was observed before. See: D44640 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D124762: [WinEHPrepare] Avoid truncation of EH funclets with GNUstep ObjC runtime

sgraenitz updated this revision to Diff 436730. sgraenitz added a comment. Update LLVM CodeGen test for mainline opaque pointers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124762/new/ https://reviews.llvm.org/D124762 Files:

[PATCH] D128190: [WinEH] Apply funclet operand bundles to nounwind intrinsics that lower to function calls

sgraenitz added a comment. Running `check-llvm` and `check-clang` locally found no more failing tests. The issue is not limited to pre-ISel intrinsics (anymore) and I have to re-phrase a few comments. Apart from that, is there any more feedback? Thanks Repository: rG LLVM Github Monorepo

[PATCH] D128190: [WinEH] Apply funclet operand bundles to nounwind intrinsics that lower to function calls

sgraenitz created this revision. sgraenitz added reviewers: rnk, theraven, DHowett-MSFT. Herald added subscribers: jsji, pengfei, hiraditya. Herald added a project: All. sgraenitz requested review of this revision. Herald added projects: clang, LLVM. Herald added a subscriber: cfe-commits.

[PATCH] D128190: [WinEH] Apply funclet operand bundles to nounwind intrinsics that lower to function calls

sgraenitz updated this revision to Diff 438425. sgraenitz added a comment. Add missing `objc_sync_exit` to fix failing test Transforms/PreISelIntrinsicLowering/objc-arc.ll Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128190/new/

[PATCH] D124762: [WinEHPrepare] Avoid truncation of EH funclets with GNUstep ObjC runtime

sgraenitz abandoned this revision. sgraenitz added a comment. Close in favor of the D128190 , which covers both cases at once: emit (was D124762 ) and inline (was D127962 ) Repository: rG

[PATCH] D128190: [WinEH] Apply funclet operand bundles to nounwind intrinsics that lower to function calls

sgraenitz added a comment. Hi @theraven thanks for reviewing! Comment at: llvm/lib/IR/IntrinsicInst.cpp:61 + case Intrinsic::objc_sync_enter: + case Intrinsic::objc_sync_exit: +return true; theraven wrote: > I'm curious why memcpy and friends don't need

[PATCH] D128190: [WinEH] Apply funclet operand bundles to nounwind intrinsics that lower to function calls

sgraenitz updated this revision to Diff 447667. sgraenitz marked 2 inline comments as done. sgraenitz added a comment. Rebase and polish comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128190/new/ https://reviews.llvm.org/D128190 Files:

[PATCH] D128190: [WinEH] Apply funclet operand bundles to nounwind intrinsics that lower to function calls

sgraenitz added a comment. Pre-merge checks passed on Windows. Debian failures are unrelated. Locally on Ubuntu and macOS all tests pass. Getting ready to land this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128190/new/

[PATCH] D128190: [WinEH] Apply funclet operand bundles to nounwind intrinsics that lower to function calls

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1e308204838b: [WinEH] Apply funclet operand bundles to nounwind intrinsics that lower to… (authored by sgraenitz). Repository: rG LLVM Github

[PATCH] D124762: [WinEHPrepare] Avoid truncation of EH funclets with GNUstep ObjC runtime

sgraenitz created this revision. sgraenitz added reviewers: rnk, theraven, DHowett-MSFT. Herald added a subscriber: hiraditya. Herald added a project: All. sgraenitz requested review of this revision. Herald added projects: clang, LLVM. Herald added a subscriber: cfe-commits. Unwinding ObjC code

[PATCH] D124762: [WinEHPrepare] Avoid truncation of EH funclets with GNUstep ObjC runtime

sgraenitz added a comment. I guess testing must be split in two: - Clang wants to make sure the "funclet" bundle operand gets emitted for ObjC ARC runtime calls on Windows. Maybe that fits into clang/test/CodeGenObjC/gnu-exceptions.m

[PATCH] D124762: [WinEHPrepare] Avoid truncation of EH funclets with GNUstep ObjC runtime

sgraenitz added a comment. Thanks for your help! I will work on testing in calendar week 21. Comment at: llvm/lib/CodeGen/WinEHPrepare.cpp:966 -if (FuncletBundleOperand == FuncletPad) +if (!FuncletPad || FuncletBundleOperand == FuncletPad)

[PATCH] D134441: [ObjC][ARC] Fix target register for call expanded from CALL_RVMARKER on Windows

This revision was automatically updated to reflect the committed changes. Closed by commit rGed8409dfa0a9: [ObjC][ARC] Fix target register for call expanded from CALL_RVMARKER on Windows (authored by sgraenitz). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D134441: [ObjC][ARC] Don't use operand bundle "clang.arc.attachedcall" in codegen for Windows

sgraenitz added a comment. Bisectioning showed that the regression started with this patch: https://reviews.llvm.org/D111331 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134441/new/ https://reviews.llvm.org/D134441

[PATCH] D134441: [ObjC][ARC] Don't use operand bundle "clang.arc.attachedcall" in codegen for Windows

sgraenitz added a comment. I assume is that this can only be a temporary fix as it probably has an impact on performance of compiled output. I am eager to fix this properly. The FIXME comment says that we need specific support in the target backend. Any ideas what the x86_64 backend for

[PATCH] D134441: [ObjC][ARC] Don't use operand bundle "clang.arc.attachedcall" in codegen for Windows

sgraenitz created this revision. sgraenitz added reviewers: ahatanak, rjmccall, theraven, rnk. Herald added a subscriber: pengfei. Herald added a project: All. sgraenitz requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This patch is an

[PATCH] D134441: [ObjC][ARC] Don't use operand bundle "clang.arc.attachedcall" in codegen for Windows

sgraenitz added a comment. The symptom is that Clang emits `movq %rax, %rdi` instead of `movq %rax, %rcx` while `objc_retainAutoreleasedReturnValue()` still expects the value in `%rcx`. It appears related to D94597 . Is this a calling convention issue?

[PATCH] D134441: [ObjC][ARC] Fix target register for call expanded from CALL_RVMARKER on Windows

sgraenitz added a comment. In D134441#3814794 , @fhahn wrote: > LGTM, thanks! You might want to extend the windows check lines to the other > tests as well. Thanks. Yes thought about that, but not all of them match the typical IR output on Windows

[PATCH] D134441: [ObjC][ARC] Don't use operand bundle "clang.arc.attachedcall" in codegen for Windows

sgraenitz updated this revision to Diff 462871. sgraenitz added a comment. Herald added a subscriber: hiraditya. Herald added a project: LLVM. Follow Reid's advice from https://github.com/llvm/llvm-project/issues/56952#issuecomment-122565 and fix the instruction sequence expanded from

[PATCH] D134441: [ObjC][ARC] Don't use operand bundle "clang.arc.attachedcall" in codegen for Windows

sgraenitz added a comment. Thanks for your feedback! For the moment, I continued the discussion in the GitHub ticket. My gut feeling is that I should fix the X86ExpandPseudo code to emit the right register-register move instead of preventing the use of the operand bundle here in CodeGen,

[PATCH] D137944: [ObjC][ARC] Teach the OptimizeSequences step of ObjCARCOpts about WinEH funclet tokens

sgraenitz added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137944/new/ https://reviews.llvm.org/D137944 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D137944: [ObjC][ARC] Teach the OptimizeSequences step of ObjCARCOpts about WinEH funclet tokens

sgraenitz added a comment. This is ready to land and I'd appreciate feedback from one of the authors. Anyone else I should add for review? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137944/new/ https://reviews.llvm.org/D137944

[PATCH] D137944: [ObjC][ARC] Teach the OptimizeSequences step of ObjCARCOpts about WinEH funclet tokens

sgraenitz updated this revision to Diff 483745. sgraenitz marked 2 inline comments as done. sgraenitz added a comment. Address feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137944/new/ https://reviews.llvm.org/D137944 Files:

[PATCH] D137944: [ObjC][ARC] Teach the OptimizeSequences step of ObjCARCOpts about WinEH funclet tokens

sgraenitz added a comment. Thanks for taking a look. That' really useful feedback! Yes, the coloring can be expensive and we shouldn't run it more than once. and there's more places indeed until it either stops making changes or // no retain+release pair nesting is detected

[PATCH] D137944: [ObjC][ARC] Teach the OptimizeSequences step of ObjCARCOpts about WinEH funclet tokens

sgraenitz updated this revision to Diff 477470. sgraenitz marked an inline comment as done. sgraenitz added a comment. Rebase and update check-lines after D137939 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D137944: [ObjC][ARC] Teach the OptimizeSequences step of ObjCARCOpts about WinEH funclet tokens

sgraenitz updated this revision to Diff 477471. sgraenitz added a comment. Handle potential multi-color basic blocks in addOpBundleForFunclet() Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137944/new/ https://reviews.llvm.org/D137944 Files:

[PATCH] D137944: [ObjC][ARC] Teach the OptimizeSequences step of ObjCARCOpts about WinEH funclet tokens

sgraenitz added a comment. Thanks for your feedback @rnk! I hope some of the ObjCARCOpts authors will share their opinions as well. Comment at: llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp:767 +const ColorVector = BlockColors.find(BB)->second; +assert(CV.size() == 1

[PATCH] D138122: Lift EHPersonalities from Analysis to IR (NFC)

sgraenitz created this revision. sgraenitz added reviewers: rnk, efriedma, dblaikie, compnerd. Herald added subscribers: Enna1, wenlei, pengfei, hiraditya. Herald added a project: All. sgraenitz requested review of this revision. Herald added subscribers: cfe-commits, aheejin. Herald added

[PATCH] D137939: [CGObjC] Open cleanup scope before SaveAndRestore CurrentFuncletPad and push CatchRetScope early

sgraenitz updated this revision to Diff 476150. sgraenitz marked an inline comment as done. sgraenitz added a comment. Run test with -fobjc-arc-exceptions and check cleanup pads as well Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137939/new/

[PATCH] D137939: [CGObjC] Open cleanup scope before SaveAndRestore CurrentFuncletPad and push CatchRetScope early

sgraenitz added inline comments. Comment at: clang/test/CodeGenObjCXX/arc-exceptions-seh.mm:36 +// CHECK: call +// CHECK: do_something +// CHECK: [ "funclet"(token [[CATCHPAD]]) ] rnk wrote: > Don't we need an exceptional cleanup

[PATCH] D137939: [CGObjC] Open cleanup scope before SaveAndRestore CurrentFuncletPad and push CatchRetScope early

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9a9d636caeea: [CGObjC] Open cleanup scope before SaveAndRestore CurrentFuncletPad and push… (authored by sgraenitz). Repository: rG LLVM Github

[PATCH] D137942: [CGObjC] Add run line for release mode in test arc-exceptions-seh.mm (NFC)

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG01023bfcd33f: [CGObjC] Add run line for release mode in test arc-exceptions-seh.mm (NFC) (authored by sgraenitz). Changed prior to commit:

[PATCH] D137942: [CGObjC] Add run line for release mode in test arc-exceptions-seh.mm (NFC)

sgraenitz added a comment. The Buildbot has detected a failed build on builder sanitizer-aarch64-linux-bootstrap-ubsan while building clang,compiler-rt. I will investigate tomorrow and reverted the change in the meantime. Full error output: TEST 'Clang ::

[PATCH] D138122: Lift EHPersonalities from Analysis to IR (NFC)

sgraenitz updated this revision to Diff 477433. sgraenitz added a comment. clang-format includes and sort clang-formatted-files.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138122/new/ https://reviews.llvm.org/D138122 Files:

[PATCH] D137944: [ObjC][ARC] Teach the OptimizeSequences step of ObjCARCOpts about WinEH funclet tokens

sgraenitz added a comment. This review belongs to a series of patches. I am planning to land it towards the end of next week, so that it's ready for the next release branching in February. If you have any more remarks, please let me know soon. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D137944: [ObjC][ARC] Teach the OptimizeSequences step of ObjCARCOpts about WinEH funclet tokens

sgraenitz marked an inline comment as done. sgraenitz added inline comments. Comment at: llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp:597 + assert(CV.size() > 0 && "Uncolored block"); + for (BasicBlock *EHPadBB : CV) +if (auto *EHPad =

[PATCH] D137944: [ObjC][ARC] Teach the OptimizeSequences step of ObjCARCOpts about WinEH funclet tokens

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. sgraenitz marked an inline comment as done. Closed by commit rGd9eece916a8a: [ObjC][ARC] Teach the OptimizeSequences step of ObjCARCOpts about WinEH funclet… (authored

[PATCH] D141824: [clang-repl] Add a command to load dynamic libraries

sgraenitz added inline comments. Comment at: clang/lib/Interpreter/Interpreter.cpp:223 +llvm::Error Interpreter::CreateExecutor() { + const clang::TargetInfo = argentite wrote: > sgraenitz wrote: > > Factoring out this function looks like an independent

[PATCH] D138122: Lift EHPersonalities from Analysis to IR (NFC)

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3b387d10707d: Lift EHPersonalities from Analysis to IR (NFC) (authored by sgraenitz). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D141824: [clang-repl] Add a command to load dynamic libraries

sgraenitz added a comment. Thanks for working on this! It looks like this is going to be the clang-repl equivalent for Cling's `.L` command. Do you think we can get test coverage for it? So far, the only tests I found for `libclangInterpreter` are unit tests in

[PATCH] D137939: [CGObjC] Open cleanup scope before SaveAndRestore CurrentFuncletPad and push CatchRetScope early

sgraenitz added a comment. The approach follows the C++ try/catch implementation in clang/lib/CodeGen/CGException.cpp . I think this is the correct solution for what I attempted to fix with D134866

[PATCH] D137944: [ObjC][ARC] Teach the OptimizeSequences step of ObjCARCOpts about WinEH funclet tokens

sgraenitz added a comment. There doesn't seem to be a 1-to-1 relationship between deleted and inserted retain/release calls and thus we don't attempt to preserve bundle operands in general. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D137944: [ObjC][ARC] Teach the OptimizeSequences step of ObjCARCOpts about WinEH funclet tokens

sgraenitz added a comment. There was a similar effort for CleanupPads in the past: https://reviews.llvm.org/rG8b342680bf62722e5099074e8bd23491c71d92b3 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137944/new/ https://reviews.llvm.org/D137944

[PATCH] D137939: [CGObjC] Open cleanup scope before SaveAndRestore CurrentFuncletPad and push CatchRetScope early

sgraenitz created this revision. sgraenitz added reviewers: theraven, rnk. Herald added a project: All. sgraenitz requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Pushing the `CatchRetScope` early causes cleanups for catch parameters to be

[PATCH] D137942: [CGObjC] Add run line for release mode in test arc-exceptions-seh.mm (NFC)

sgraenitz created this revision. Herald added a project: All. sgraenitz requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. In release mode the updated `arc-exceptions-seh.mm` test fails and needs `-enable-objc-arc-opts=false` to skip ObjC

[PATCH] D137944: [ObjC][ARC] Teach the OptimizeSequences step of ObjCARCOpts about WinEH funclet tokens

sgraenitz created this revision. sgraenitz added reviewers: gottesmm, rjmccall, rnk. Herald added a subscriber: hiraditya. Herald added a project: All. sgraenitz requested review of this revision. Herald added projects: clang, LLVM. Herald added a subscriber: cfe-commits. When optimizing

[PATCH] D141824: [clang-repl] Add a command to load dynamic libraries

sgraenitz added a comment. Thanks for the update Comment at: clang/test/Interpreter/dynamic-library.cpp:1 +// RUN: head -n 7 %s | %clang -xc++ -o %T/libdynamic-library-test.so -fPIC -shared - +int ultimate_answer = 0; The use of `head` and `tail` here is a

[PATCH] D141824: [clang-repl] Add a command to load dynamic libraries

sgraenitz added inline comments. Comment at: clang/test/Interpreter/dynamic-library.cpp:6 + +#include + bcain wrote: > This test fails for me like below. > > > ``` > FAIL: Clang :: Interpreter/dynamic-library.cpp (1 of 17751) > TEST

[PATCH] D148481: [clang-repl] Enable debugging of JIT-ed code.

sgraenitz accepted this revision. sgraenitz added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148481/new/ https://reviews.llvm.org/D148481 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D141824: [clang-repl] Add a command to load dynamic libraries

sgraenitz added a comment. In D141824#4274461 , @argentite wrote: > We should probably also address the lack of linker issue as well. Should we > go for a precompiled dynamic library file? There seems to be some "precedent" > of this in other tests.

[PATCH] D148992: [clang-repl] Fix dynamic library test to avoid cstdio and linker

sgraenitz accepted this revision. sgraenitz added a comment. Thanks. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148992/new/ https://reviews.llvm.org/D148992 ___ cfe-commits mailing list

[PATCH] D148992: [clang-repl] Fix dynamic library test to avoid cstdio and linker

sgraenitz added inline comments. Comment at: clang/test/Interpreter/dynamic-library.cpp:12 +// return 5; +// } Should we wrap this in a `extern "C"` block? Otherwise, the shared library has a C++ interface, which is not stable. It won't matter in this

[PATCH] D148434: [clang-repl] JITTargetAddress --> ExecutorAddr

sgraenitz accepted this revision. sgraenitz added a comment. This revision is now accepted and ready to land. Thanks. Maybe you can fix this one detail and add a description like "Most of Orc and JITLink are movinng away from JITTargetAddress and use ExecutorAddr instead"? Otherwise LGTM.

[PATCH] D141824: [clang-repl] Add a command to load dynamic libraries

sgraenitz added a comment. In D141824#4247891 , @zhuhan0 wrote: > This seems to be the first clang test that involves actual linking. Interesting, I think we all didn't have this on our radars. > If the intention is to test loading a dynamic library,

[PATCH] D141824: [clang-repl] Add a command to load dynamic libraries

sgraenitz accepted this revision. sgraenitz added a comment. This revision is now accepted and ready to land. Thanks! LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141824/new/ https://reviews.llvm.org/D141824

[PATCH] D141215: [clang-repl] Introduce Value to capture expression results

sgraenitz added a comment. This looks a lot better already than the implementation I know from Cling. Well done! OTOH it's a challenge to review since the patch is really big. Is there no way to break it up and test in isolation? Or strip away details that could be added in iterations? I had

[PATCH] D155716: [clang][CodeGen] Introduce `-frecord-command-line` for MachO

sgraenitz added a comment. That's an excellent patch! I didn't expect it could be so compact. Thanks for working on this! Please find below 2 inline comments on details. On July 25th release/17.x will branch away. It would be great to land this before and have it in the upcoming release.

[PATCH] D155716: [clang][CodeGen] Introduce `-frecord-command-line` for MachO

sgraenitz accepted this revision. sgraenitz added a comment. This revision is now accepted and ready to land. Great! This LGTM. Let's keep the review open for a few days and see if there is more feedback. Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1424

[PATCH] D141215: [clang-repl] Introduce Value to capture expression results

sgraenitz added a comment. In D141215#4272538 , @lhames wrote: > using regular global variable instances to manage the storage on the executor > side, an extended `MemoryAccess` interface to read/write the value from the > REPL side when needed (e.g.