We may think in generalities, but weliveindetail
- User Since
- Jul 10 2018, 11:23 AM (50 w, 10 h)
Wed, Jun 19
Tue, Jun 18
Mon, Jun 17
Thanks for the initiative! Three inline comments for cosmetics.
Thu, Jun 13
Wed, Jun 12
Fri, Jun 7
Wed, Jun 5
Unfortunately lit is a little more complicated
Remove Xcode restriction and polish
Tue, Jun 4
Add CMAKE_EXECUTABLE_SUFFIX when searching for llvm-tblgen
That will be fine with post-commit review. Will submit tomorrow.
Patch the replacement logic also in lit/CMakeLists.txt
IIUC Clang uses the same mechanism. Moving append_configuration_directories() to AddLLVM.cmake could help there too. We may consider it for the future once D62859 is through.
Chose the replacement for remaining matches of CMAKE_CFG_INTDIR depdending on LLVM_CONFIGURATION_TYPES
Thanks for your feedback. I should have mentioned that this is pretty much a draft. For now I was happy it works at all.
Wed, May 29
Fixed debugserver; excluded changes in add_lldb_library() for now; result of this test looks as good as before:
Tue, May 28
Thanks for having a look. I'd to focus on the mechanism here and keep the contents of the framework bundle unchanged. Happy to discuss this in separate reviews like D62474.
Fix copy/paste mistake for darwin-debug
Sun, May 26
lldb-vscode is no fremwork tool, but instead it needs the RPATHs
Closing this in favor of the reduced proposal
May 17 2019
Ok thanks. I will be OOO next week, so no hurries.
Fix default install locations and add comments on how to use DESTDIR
Sure, ideally CMake defined a global install order and this order would handle overlap. I think that's unlikely to happen. It took quite some time to find and fix an overlap case downstream and I think it's worth avoiding it in general in the future. OTOH I see that the solution shouldn't be too intrusive. For the swift resources: I am not familiar with the details; all I know is that tests fail if they are missing.
May 16 2019
Add back CACHE STRING "" for CMAKE variables
Yet another option: we could install the framework tools to a fake location and copy them to their final destination (overwriting their build-tree pendants) in a manual install step at the very end of the install process. The problem here is, that there may be more things in the build-tree framework than we overwrite, and thus remain in the install-tree (thinking about Swift resources in swift-lldb for example).
I thought about one more option, but I don't think it's better than the current proposal: instead of deleting the build-tree resources in the very first install step, I could install the framework manually at this point and skip its regular install target. We would loose implicit stripping and RPATH replacement for the dylib though. Should be possible to do it manually, but if it breaks, we won't notice. So, it's not quite appealing.
Actually, why not make the copy operations PRE_BUILD actions of the test suite instead of POST_BUILD actions of their executables?
Thanks for your input.
Rename macOS specific cache file.
Add settings and comments.
Fix install destinations by appending Developer and SharedFrameworks respectively.
May 15 2019
Yes, sorry for that. I realized it after the commit was in. The commit I got from arc patch did have the original author information. It must have changed in git llvm push, probably because it's still going to SVN and then gets mirrored back to git. I missed this last detail.
May 14 2019
Sure, will commit this on your behalf tomorrow. (If nothing else comes up.)
Thanks for adding this. Would it make sense to use LLVM_ENABLE_PROJECTS_USED? https://github.com/llvm/llvm-project/blob/a568222d/llvm/CMakeLists.txt#L128
May 13 2019
It looks like this test is flaky [...] what would you say to just deleting the "# CHECK: 1 location added to breakpoint 1" line from the test?
May 9 2019
Add back test requirement target-x86_64
- XFAIL: system-windows
@stella.stamenova Can you have a look at the lit test please? It works on macOS and Linux, but I didn't test Windows. Should I add something like # REQUIRES: nowindows or is it fine like this?
Generalize lit test
Ok, I went with MCJIT for now. Will leave this here for future reference.
May 8 2019
Thanks for your reply and thoughts about that.
Add lit test
May 6 2019
May 4 2019
Apr 25 2019
For illustration: pass ownership to ObjectLinkingLayer and hand out a MemBufferRef in NotifyLoaded
Ok I see, as long as the original MemoryBuffer exists, we can recreate an identical ObjectFile on the client side. Currently it gets deleted together with JITLinkContext when returning from jitLink().
Apr 24 2019
Maybe not the most elegant solution, but this seems to work: https://reviews.llvm.org/D61065
One crucial part that I found missing here is a way to connect JITEventListener (like GDBJITRegistration, PerfJITEvents, etc.), because the new ObjectLinkingLayer::NotifyLoaded function only provides the module key but no ObjectFile so we cannot create and pass over a LoadedObjectInfo as with RuntimeDyld. Please see my inline comments for some pointers.
Apr 23 2019
LGTM too. Did this land?
Apr 18 2019
This was mostly to illustrate usage of the matching LLVM commit. Unlikely to break something. Post-commit review should be sufficient.
Yes, scopes are per directory; macros operate on the caller's scope and functions add one.