-
Notifications
You must be signed in to change notification settings - Fork 232
compiler-rt's ARM implementation of udivsi3 seems... wrong? #173
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
Comments
Although compiler-rt presumably has a more optimized implementation written in assembly, it appears buggy for whatever reason, causing rust-lang#173. For now let's see if integration into rust-lang/rust will work with the Rust-defined implementation!
Use the Rust implementation of udivsi3 on ARM Although compiler-rt presumably has a more optimized implementation written in assembly, it appears buggy for whatever reason, causing #173. For now let's see if integration into rust-lang/rust will work with the Rust-defined implementation!
@japaric I think you're more familiar with ARM assembly than I am, mind double-checking me on this to make sure I'm not off the rails? |
I'll also note that I don't actually know what the error, if any, is in the |
Although compiler-rt presumably has a more optimized implementation written in assembly, it appears buggy for whatever reason, causing rust-lang#173. For now let's see if integration into rust-lang/rust will work with the Rust-defined implementation!
Use the Rust implementation of udivsi3 on ARM Although compiler-rt presumably has a more optimized implementation written in assembly, it appears buggy for whatever reason, causing #173. For now let's see if integration into rust-lang/rust will work with the Rust-defined implementation!
Although compiler-rt presumably has a more optimized implementation written in assembly, it appears buggy for whatever reason, causing rust-lang#173. For now let's see if integration into rust-lang/rust will work with the Rust-defined implementation!
Use the Rust implementation of udivsi3 on ARM Although compiler-rt presumably has a more optimized implementation written in assembly, it appears buggy for whatever reason, causing #173. For now let's see if integration into rust-lang/rust will work with the Rust-defined implementation!
Would pulling in llvm-mirror/compiler-rt@6b34053 fix the issue? |
Oh that's actually the commit I meant to mention above, unfortunately pulling that in didn't fix it |
@alexcrichton I think I can fix this, how did you reproduce it locally? |
@parched I think it should suffice to revert alexcrichton@681aaa9 and then run |
@alexcrichton Thanks, trying that I get to
Have you seen that before? |
@parched I believe that's related to Linux's support to run non-native binaries by hook them with something like a QEMU interpreter. You may need to do something like https://github.com/rust-lang-nursery/compiler-builtins/blob/238647af806470dc73e585c03682083931d29cd5/.travis.yml#L48-L49 on your host system. |
@parched Alternatively you pass @alexcrichton That reminds me, now that Cargo has "runner" support we might be able to drop the qemu-user-static stuff and replace it with |
Oh right yeah, that migh work great! |
Ok thanks guys, I originally tried with just |
@alexcrichton IMO for now we should just revert llvm-mirror/compiler-rt@2fb759f |
We could, yes, but ideally we'd get that upstream |
Fyi https://reviews.llvm.org/D31220 has been accepted upstream now |
Great! As soon as we upgrade we can switch and close out this issue |
I updated the compiler-rt submodule in #172 which notably pulled in llvm-mirror/compiler-rt@2fb759f I believe. I merged #172 despite failing tests believing it was a weird nightly thing, but the integration PR also failed with weird errors.
I did some more investigation locally and found I could reproduce the error on #172 locally relatively easily. When forced the compiler-builtins crate to use the Rust implementation of the
__udivsi3
intrinsic then the error went away. The only other change to happen to this file since we last updated compiler-rt isllvm-mirror/compiler-rt@2624197llvm-mirror/compiler-rt@6b34053, which when cherry-picked didn't seem to fix the problem.I'm going to send a PR to avoid usage of compiler-rt's implementation, but it's presumably faster than what we have in Rust, so this is a tracking issue for closing that perf gap.
The text was updated successfully, but these errors were encountered: