-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Update to LLVM HEAD + assorted cleanups #8328
Conversation
Fantastic work. 🍓 As discussed on IRC, let's change |
As discussed on IRC, you have some more LLVM changes in the pipeline. We want to batch them all up to avoid unnecessary disruption. |
Right now I'm waiting on seeing what needs to happen to fix rusti again. Whenever there's a 32-bit target (regardless of the host), rusti tests are currently dying. It's not clear if this is the JIT specifically or just how we're using LLVM. |
Will this include http://llvm.org/viewvc/llvm-project?view=revision&revision=187656 ? We should be able to un-xfail the win32 debuginfo tests if that's the case. @michaelwoerister |
Good news! It does! Still sorting out this rusti business though |
@jdm, @michaelwoerister: r187656 fix was a bit buggy, you'll need to pick up LLVM at r188297 for a proper fix. |
This is pretty much ready to go, I'm not going to wait until I fix the rusti issues. I think they're all just issues in rustc anyway (or at least our half of using LLVM). For now I'm waiting on #8488 to see if those patches should be included or not. |
That's great |
r? @brson I'm not going to get around to fixing the rusti issues any time soon, and I don't want to let this just rotting here forever. I rebased on top of an LLVM from yesterday night. You raised concerns about building LLVM too much, and if #8488 lands soon it'll require another LLVM rebuild. Otherwise this should be ready to go (passing tests locally) |
* This has one workaround patch (everything's testing just fine...) * I reworked the fixedstacksegment attribute to be specified with a string rather than using a keyword and an integer and modifying the parser * I added a "no-split-stack" attribute along the same lines as the "fixedstacksegment" attribute for rust-lang#1226
Whoops, forgot to touch the llvm-trigger... |
This implements #[no_split_stack] and also changes #[fast_ffi] to using the new "fixedstacksegment" string attribute instead of integer attribute.
The first commit message is pretty good, but whomever reviews this should probably also at least glance at the changes I made in LLVM. I basically reorganized our pending patch queue to be a bit more organized and clearer in what needs to go where. After this, our queue would be: * Add the `no-split-stack` attribute * Add the `fixedstacksegment` attribute * Add split-stacks for arm android * Add split-stacks for arm linux * Add split stacks for mips Then there's a patch which I added to get rust to build at all on LLVM-head, and I'm not quite sure why it's there, but nothing seems to be crashing for now! (famous last words). Otherwise, I just updated code to reflect the changes I made in LLVM with the only major change being the advent of the new `no_split_stack` attribute. This is work towards #1226, but someone more familiar with the code should probably actually assign the attribute to the appropriate functions. Also as a bonus, I've verified that this closes #5774
The first commit message is pretty good, but whomever reviews this should probably also at least glance at the changes I made in LLVM. I basically reorganized our pending patch queue to be a bit more organized and clearer in what needs to go where. After this, our queue would be:
no-split-stack
attributefixedstacksegment
attributeThen there's a patch which I added to get rust to build at all on LLVM-head, and I'm not quite sure why it's there, but nothing seems to be crashing for now! (famous last words).
Otherwise, I just updated code to reflect the changes I made in LLVM with the only major change being the advent of the new
no_split_stack
attribute. This is work towards #1226, but someone more familiar with the code should probably actually assign the attribute to the appropriate functions.Also as a bonus, I've verified that this closes #5774