We may think in generalities, but weliveindetail
- User Since
- Jul 10 2018, 11:23 AM (153 w, 2 d)
Thu, Jun 10
However, while investigating I found the threading issue that this patch aims to fix. @lhames Might the unjoined thread have caused the assertion failure in SymbolStringPool?
After commit 2f9ba6aa8b6d the test failure https://lab.llvm.org/buildbot/#/builders/61/builds/10796 occurred on one of the few build bots that include the examples. The patch doesn't affect any code exercised in this test and the bot turned green with the subsequent build. Thus, the test might be considered flaky. So far I failed to reproduce the exact error:
Fri, May 21
Hey, just a few notes on proper error handling.
May 10 2021
Apr 7 2021
Apr 6 2021
Mar 31 2021
LGTM. Thanks for fixing this!
Mar 30 2021
Mar 29 2021
Mar 28 2021
Mar 26 2021
Don't apply lazy-MCJIT tests to the default (greedy) mode for Orc. They probably work well with OrcLazy, but that's a different story.
Apply MCJIT small code-model tests to ORC in preparation for default JIT engine switch
This can be used to fix test failures on Windows when re-applying D98931
Mar 23 2021
Fix error No callback manager available on powerpc64le-unknown-linux-gnu with eccd7ae2fdb3
Mar 22 2021
For now keep eh-lg-pic.ll a MCJIT-only test
Alright fixed with 9cdbdbea29ce
Mar 19 2021
Thanks for the confirmation. I will land this as soon as D98579 is done.
Patch 5 other tests that slipped through
Make returnError() static (oops). Rename RTraits -> AltRetTraits. Turn some autos into concrete typenames to aid readability.
I actually didn't manage to reproduce the warning with GCC 10 on Debian, so I couldn't really test this. @thakis Do you want to run a pre-test? Otherwise, we could land it right away and see if it is effective.
Add special-case for calling accept(2) on AIX. I cannot test it here, @rzurob does this work for you?
Patch tests for MCJIT-specific behavior
Run tests for common JIT tasks against both, MCJIT and Orc
Mar 18 2021
Thanks for elaborating.
Pending resources are irrelevant in both, notifyTransferringResources() and notifyRemovingResources(). They are guaranteed to be either finalizd in notifyEmitted() or discarded in notifyFailed().
Mar 17 2021
This is hard to reproduce with lli, because single-threaded execution with LLLazyJIT will implicitly sequence materialization. LLJIT instead materializes module dependencies recursively, which makes this issue easy to reproduce. Unfortunately, we have no LLJIT tool that I could use for a test. It's not the first time this limitation comes up for me and I wonder if it's worth adding an orc-greedy kind to lli. What do you think?
Mar 13 2021
Print 'Connection established' only when connecting via a TCP socket
Some sample output for llvm-jitlink with various broken addresses:
Failed to connect TCP socket 'localhost:1': Connection refused (inactive)
Failed to connect TCP socket '22.214.171.124:1': Operation timed out (non-responding)
Failed to connect TCP socket '127.0.0.1:0': Can't assign requested address
Failed to connect TCP socket '127.0.0.1:-1': Address resolution failed (nodename nor servname provided, or not known)
Failed to connect TCP socket '127.0.0.1': Port for -oop-executor-connect can not be empty
Follow-up patch that submits learnings from fixing D98579. The error it fixed would now print as:
Failed to connect TCP socket 'localhost:13921': Address resolution failed (Bad hints)
Unfortunately, we seem to have no test coverage for this code. My last take was that LIT doesn't exactly make it easy to run client and server concurrently. Is that still the case or could we do it somehow differently?
I had to take a detailed look into this code now anyway, so I could just as well add a test case.
The patch proposed here is a minimal fix. I am eager to streamline the code and add better error messages in a follow-up commit.
Mar 12 2021
Thanks. From my side this looks good now.
Thanks for elaborating. LGTM
I think you're right, but rather than approach this through the plugin API I think the solution will come from rethinking "finalization" on the memory manager API
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.
Mar 11 2021
Factoring out architecture specific code sounds like a reasonable approach to reduce code duplication across the JITLink backends. As far as I can see, the patch only ports the MachOJITLinker_x86_64 to use the generic functionality. The ELFJITLinker_x86_64 mostly remains unchanged. Is there a conceptual reason for it or would that be a rather straightforward next step?
This means that materialization is blocked until finalization of the debug object allocation is done, i.e.:
- memory was copied over to the target
- an entry was added to the JIT descriptor (locks a global mutex)
- the executing process ran into the rendezvous breakpoint (1st process switch)
- the debugger processed the debug object, read the symbol table and returned control back to the executor (2nd process switch)
Mar 9 2021
@ASDenysPetrov I guess you set the Needs Revision status accidentally?
Attempting to close the review after it was re-opened.
For the entry point, prefer lli over llvm-jitlink. D97339 added debug support for JITLink in lli.
Mar 8 2021
Mar 3 2021
@klausler Thanks for reporting this. lli was missing JITLink as a link component in CMake. I fixed it with https://reviews.llvm.org/rG295ea050ad594b8868b6dd944824ba9a16273f91. Sorry for the inconvenience.
This fixes the symbol lookup issue on Windows that was discussed post-review in D97339. Sorry for the inconvenience.