-
-
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
ppc64le abi #16677
ppc64le abi #16677
Conversation
@@ -113,9 +113,9 @@ | |||
# define MEMDEBUG | |||
# define KEEP_BODIES | |||
# endif | |||
// Memory sanitizer also needs thread-local storage | |||
// Memory sanitizer also needs small memory model |
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.
No, it needs thread local storage. Please don't keep changing this. Being able to use memory sanitizer is important.
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.
This flag doesn't directly toggle TLS, it toggles small memory model (and PIC). The small memory model isn't supported by the SectionMemoryManager (the result being occasional random segfaults and memory corruption). When you fix the SMM, I'll stop removing the flag.
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.
Both of which are required for TLS. I'd rather have a flag that occasionally segfaults but otherwise tells me where memory bugs are than one that doesn't work at all.
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.
Also, of course the proper fix is to teach LLVM how to do TLS in the large memory model.
// | ||
// LDC – the LLVM D compiler | ||
// | ||
// This file is distributed under the BSD-style LDC license: |
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.
add to list in contrib/add_license_to_files.jl
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.
I'll fix this one. There's a bunch more that need to be fixed though (src/support/dirname.c
is missing from the list and the script needs to be rerun).
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.
in a separate pr then?
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.
I was planning on just pushing the update to master, no?
I'm running into an llvm bug when trying to build the debug version, however, the fix will be included in llvm 3.8.1 (http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20160530/360643.html) |
future work: consider adding `align 8` to byval to match clang
LLVM patch committed upstream as http://reviews.llvm.org/rL271425. Shall we ask for a backport to 3.8.1? |
I think we missed the deadline for that, but we could try and see? |
Yes, you're right. They're expecting to release 3.8.1rc1 today. I think we're fine with 3.9. There's much more work to be done before people will want to use julia with non-bundled versions of LLVM, so I don't think there's much of a rush. |
rc1 was apparently pushed back to monday, maybe we could try our luck asking for this to be done between rc's? |
ref #16455 (vector abi is not yet implemented)
fix #16456
close #16036