-
Notifications
You must be signed in to change notification settings - Fork 12.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
3.2_f32 formatted as 0 on powerpc64le #96306
Comments
Did you mean dtolnay/ryu#47 for the ryu issue? @rustbot label +O-PowerPC |
Oh yes, thank you for pointing this out |
I just tried to reproduce this on my own x86_64 linux box by running:
however, all tests passed without problem... But the test running on github action still failed on powerpc64le |
Marked this as I-unsound though I don’t really have a definite culprit here. Historically these sorts of problems have been caused by calling convention mismatches, codegen failures or some other similarly scary and “has potential to cause other code to break much more terribly” problem. |
You used a 32-bit PPC target here. Does this reproduce with a
|
Thanks for pointing this out again! With the updated command, I was able to reproduce that. |
Now I was also able to reproduce it using this:
And it gives me |
This is the assembly I get:
|
Assigning priority as discussed in the Zulip thread of the Prioritization Working Group. @rustbot label -I-prioritize +P-critical |
@NobodyXu any chance you can run your test on older versions of the compiler? If this is a regression, then one can even use a tool like https://github.com/rust-lang/cargo-bisect-rustc to identify the point where the regression was injected. |
Yes, but since I don't have a power pc, |
It seems that the bug is fixed on |
|
I think we need more regression tests in |
There are still some strange bugs for
failed with:
This works on all other platforms except for |
Those are definitely some worrisome points you raise, @NobodyXu ! I'm hoping someone can help investigate them, and it would certainly be great to flesh out our tests here some more |
On Edit: According to the CI log, it's reintroduced on |
@pnkfelix IMO we absolutely need more unit tests for it, as I just witnessed the regression of This strange regression that coming back makes it impossible to use Edit: Fixed again on |
The issues mentioned here might've been issues with QEMU 3.0.1 and fixed with QEMU 5.1.0 (or earlier, 5.1.0 is the version used by latest unreleased cross today), see discussion in cross-rs/cross#803 (comment) It seems this specific case has not been tested on real hardware, but on newer QEMU versions this works just fine. EDIT: if you're using the Github action
before your cargo test/run/build invocation. We're hoping to land these changes this month so that this will not have to be done. |
Out of curiosity, does rust-lang run CI on actual powerpc machine? If not, then is this the reason for powerpc to be a tier 2 platform? |
We don't run CI on a PowerPC host at all, emulated or baremetal, afaik. There are insufficient engineering resources for setting everything up and maintaining and there hasn't been a motivated party who would pick this sort of work up either.
16 Jun 2022 06:17:53 Jiahao XU ***@***.***>:
… Out of curiosity, does rust-lang run CI on actual powerpc machine?
If not, then is this the reason for powerpc to be a tier 2 platform?
--
Reply to this email directly or view it on GitHub:
#96306 (comment)
You are receiving this because you commented.
Message ID: ***@***.***>
|
FWIW I’ve just tested the affected crate on a real ppc64le-linux machine running a POWER9 chip. All of 1.58.0, 1.59.0, 1.60.0, 1.61.0, nightly-2022-05-{11,15} successfully ran
to completion at both master and 0ed53d042202550091a71980458d18cbb3919e64. |
It currently has tests disabled on powerpc, you would need to manually enable it. |
@nagisa Checkout this repository I used to reproduce the bug. |
As I wrote in the comment above, I tested the commit 0ed53d042202550091a71980458d18cbb3919e64 which removes the cfg in question as well. Was that insufficient? |
That's sufficient. |
It appears that this problem is resolved, according to the comments here. Therefore, closing. |
In my PR ParkMyCar/compact_str#16 , when formatting 3.2 using std, it gives me 0 on target powerpc64le instead of 3.2 (it works on all other target).
P.S. ryu also format it as 0.0 on it dtolnay/ryu#47
The text was updated successfully, but these errors were encountered: