-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable JITLink in aarch64 linux. #49745
Conversation
src/jitlayers.h
Outdated
// #define JL_FORCE_JITLINK | ||
|
||
#if defined(_COMPILER_ASAN_ENABLED_) || defined(_COMPILER_MSAN_ENABLED_) || defined(_COMPILER_TSAN_ENABLED_) | ||
# define HAS_SANITIZER | ||
#endif | ||
// The sanitizers don't play well with our memory manager | ||
|
||
#if defined(_OS_DARWIN_) && defined(_CPU_AARCH64_) || defined(JL_FORCE_JITLINK) || JL_LLVM_VERSION >= 150000 && defined(HAS_SANITIZER) | ||
#if defined(_CPU_AARCH64_) || defined(JL_FORCE_JITLINK) || JL_LLVM_VERSION >= 150000 && defined(HAS_SANITIZER) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vchuravy @pchintalapudi just to be sure: if we're using a sanitizer with LLVM 15+ you want to force JITLink? I think we may want to remove defined(_CPU_AARCH64_)
here and handle that below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes we want JITLink if santitizers are enabled on >= 150000, because we hit a bug in our memory manager for Rtdyld. I'm not opposed to handling that below.
@pchintalapudi the only listener integration that is missing is the perf one right? GDB works with JITLink on 15? |
Oh we might want #47037 since we need a different plugin for ELF debugger support. |
Looks good to me regarding the code here! |
Might just want to rebase with master since #47037 just merged. |
Looks like we're running out of memory of the aarch64 tester? |
Would this be a 1.9 backport candidate? |
This requires LLVM 15, so no. |
Oh, right! :-) Sorry, should have looked more closely. |
Do we have a compiled build for this PR? I want to see if this fixes #49474 as I want to get qemu working |
Go to the aarch64-linux-gnu build job and open the artifacts tab: https://buildkite.com/julialang/julia-master/builds/23816#01880bd0-6922-44b0-b77c-67fcaf2c1c66 |
I captured with
while running
trace.tar.xz . It fails on Complex{BigFloat} but removing the other types makes the crash go away.
|
More breadcrumbs: backtrace and strace log. Highlights are
and
|
Reporting here the progress discussed on Slack. @gbaraldi figured out what was the problem: this issue in Java reported a similar Not sure where to go from here. Document the possibility of having to increase the memory mappings limit and merge this PR? Linux aarch64 is tier 2 and currently suffers from many segmentation faults anyway (although those should be solved by JITLink, enabled with this PR), so situation wouldn't be much worse. |
I'm in favor of merging and documenting the memory mapping limit issue+resolution in the release notes. The memory manager issues aren't likely to be fixed in the near term, and resource limits are easier to fix than segmentation faults. |
Yeah, I'm fine with merging too. The issue is common to all platforms that use JITLink, it seems it just reared it's head here, the solution for the user is pretty easy, but it probably requires quite a bit of LLVM work. |
Agreed! We should add a news entry, and additional an entry in the docs. Do we have an upstream issue number? |
There isn't an upstream issue just yet. |
Do you refer to the explanation of the work around potentially needed to increase the limit of mappings? Where do you suggest to place it specifically?
Is there one now? |
This is not an issue with our existing cgmemmgr, so it seems like this is something that should be fixed upstream |
Yeah, I was wondering if @gbaraldi opened a ticket upstream in llvm so that we can add a reference to it in the docs. |
ae73ae2
to
e3fd8dc
Compare
I added the NEWS entry and mentioned the ticket opened by Gabriel in the ARM build devdocs page. Please check if it's all OK. Anything else to do this for this PR? 🙂 |
LGTM |
FWIW, I'm still seeing an occasional segfault on a Jetson (linux-aarch64, NVIDIA ARM chip):
This seems to happen when exiting the process, but haven't been able to debug yet because GDB crashes (see the last line, that assertion is GDB's). So I'm not sure it's related, but it does at least point to the JIT. |
That segfault is happening everywhere :( |
tho this is not simple to do on non-rooted android, right? additionally does windows allow you to increase such limit? |
Reland of #45859, which had some conflicts with f11bfc6 (#49530). I hope I got the logic right. CC: @sunho.
Fix #42295, fix #47399, fix #41852, fix #49474.