We may think in generalities, but weliveindetail
- User Since
- Jul 10 2018, 11:23 AM (131 w, 6 d)
Fri, Jan 15
Thu, Jan 14
Drop the Error output parameter and return an Expected instead
The follow-up change is D94690. It also moves the instantiation of ObjLinkingLayer from the initialization list to the ctor body.
Thanks! Eventually, I closed https://llvm.org/PR36209
Mon, Jan 11
I'd like to land this before release 12 branches. If you have any objections, please speak up before the end of the week. Thanks.
Dec 8 2020
Dec 2 2020
Friendly ping. @lhames Can you confirm this fix? I couldn't find a test for this piece of code.
Dec 1 2020
Does anyone have objections to this documentation update? Preview is here still:
Nov 23 2020
Nov 18 2020
Nov 12 2020
Ok, I grabbed your latest version and checked again for the EH frame registration on Linux. This works very well now! Thanks
Nov 11 2020
One issue with the orc-lazy JIT will be symbol renaming for lazy reexports: Upfront we only emit stubs for our function symbols and they trigger compilation of the actual code in a separate logical unit once they are called. The debug object we hand over to LLDB doesn't take that into account. (Maybe we are just inserting the wrong load address? Don't know.) Additionally, I am not sure how debug data is handled in the partitioning logic of the CompileOnDemandLayer.
Thanks for having a look.
Last issue: EH-frame registration isn't ready for ELF right? That would be great to fix (see the unticked inline comment).
I can offer to check the mangling prefix details once this had landed.
Nov 10 2020
Nov 6 2020
That's right, but I don't see how even the static ones could be resolved. Wouldn't it hard-wire the objects together? In the next session we want to load some object files from cache while others have changed.. Maybe one day I should check how ccache and the likes handle it.
Nov 5 2020
Thanks for having a look!
Nov 4 2020
Preview for the update: https://weliveindetail.github.io/docs-pub/html/DebuggingJITedCode.html
Nov 3 2020
Two issues when trying to build llvm-jitlink-executor with the libs separated:
I'm curious to get your take on the library breakdown and corresponding headers too.
Such a oneshot temporary label can usually be omitted:
Oct 28 2020
Polish comments and variable names in test
Oct 27 2020
Oct 26 2020
Great that there is so fast progress! Haven't reviewed in detail (yet), but it looks like there are no conceptual changes compared to the proposal we know? A few notes inline.
Oct 24 2020
One note on the optimizeELF_x86_64_GOTAndStubs pass that had to be considered for this patch: it currently only relaxes edges for MOVQ instructions to LEA and we avoid that in the test by using a MOVL. If I understand correctly, LLD doesn't have this restriction. It relaxes MOVQ to LEAQ and MOVL to LEAL. For reference see https://reviews.llvm.org/D15779#change-f8vl5AmubNpC
With movl we have to target eax
Oct 23 2020
Prevent the optimizer from relaxing the edge by using a movl instruction (anyway it's been only 4 bytes all the time)
I think it's not a big deal, but you may want to revert the named_data change. See inline comment.
Skipping the optimization pass, I had expected to get something like movq 0x1ff9(%rip), %rax but instead it's:
dis -s 0x00007ffff7ff5000 -e 0x00007ffff7ff5010 0x7ffff7ff5000: movq (%rip), %rax
I didn't get the test to work yet. Neither with a R_X86_64_REX_GOTPCRELX relocation for common_data (TOT clang and llvm-mc emit that, llvm-mc-10 too btw) nor with R_X86_64_GOTPCREL (clang-10). Some observations in the following.
Add test ELF_x86-64_common
Yes, that brings up the issue with relaxed R_X86_64_REX_GOTPCRELX (42) relocations you mentioned in Discord: JIT session error: Unsupported x86-64 relocation:42 I will check whether we can map it to PCRel32GOTLoad or otherwise what's the difference.
Thanks for coming up with a test case. I haven't looked into llvm-jitlink's check code yet. That serves a an excellent starting point!
Oct 22 2020
The ELF spec has an extra paragraph for special handling in dynamic linkers. I am not sure how much it's relevant here:
When the dynamic linker encounters a reference to a symbol that resolves to a definition of type STT_COMMON, it may (but is not required to) change its symbol resolution rules as follows: instead of binding the reference to the first symbol found with the given name, the dynamic linker searches for the first symbol with that name with type other than STT_COMMON. If no such symbol is found, it looks for the STT_COMMON definition of that name that has the largest size.
Drop unrelated switch case R_X86_64_32S
Of course, I must compile with -fPIC to avoid relocation types like R_X86_64_32S in the first place. Looking into the test case now.
It turned out the SIGSEGV: invalid address is caused by not handling the R_X86_64_32S type correctly. This is somewhat unrelated, but must be fixed in order to come up with a test case.
Relocations to the common section seem to be calculated correctly now, but at runtime I see a SIGSEGV: invalid address. Maybe memory for the section isn't allocated?
Fix NULL entry in symbol table
Oct 21 2020
Oct 20 2020
FYI The issue came up when I was trying to jitlink gzip: https://people.csail.mit.edu/smcc/projects/single-file-programs/gzip.c Next step would be an isolated test case.
Oct 1 2020
Aug 23 2020
Looks good. Two bots failed, both unrelated to my change.
Aug 20 2020
Back to the review: I'd like to keep the test for the example and see how the build servers behave. Generally it might be useful to have tests for all the "LLJITWith..." examples right?
Do you think it makes sense to land the patch on the weekend in order to keep the number of people getting annoyed by me breaking their builds at a minimum? :)
I will get back to performance evaluation maybe in a few weeks and sure I am happy to share my progress.
Aug 15 2020
Fix clang-tidy warnings
Aug 14 2020
Fix clang-format issue and rebase
Test discovery should ignore subdirectories that contain test inputs.
I would like to remove the ThinLtoJIT example. It needs a more decent threading library to speed-up multithreaded compile times and that's easier to do out-of-tree.
This might be a useful (minimal) portion to keep in-tree. What do you think?
Fixed with commit 28e1015e327ec56ac4bf93967d3aa2915b215e36