Skip to content
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

1.19 powerpc64le run-pass test failures: simd-intrinsic-generic-cast #44670

Closed
infinity0 opened this issue Sep 18, 2017 · 12 comments
Closed

1.19 powerpc64le run-pass test failures: simd-intrinsic-generic-cast #44670

infinity0 opened this issue Sep 18, 2017 · 12 comments
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. O-PowerPC Target: PowerPC processors T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@infinity0
Copy link
Contributor

infinity0 commented Sep 18, 2017

On Debian:

------stderr------------------------------
thread 'main' panicked at 'f64 -> i32 (i32x4(-1234567, 12345678, -123456792, 1234567936) != i32x4(-1234567, 12345678, -123456789, 1234567890))', /<<BUILDDIR>>/rustc-1.19.0+dfsg3/src/test/run-pass/simd-intrinsic-generic-cast.rs:127
note: Run with `RUST_BACKTRACE=1` for a backtrace.

------------------------------------------

Not sure if this is related to #42778

@infinity0
Copy link
Contributor Author

@TimNN do you think this could be rust-lang/llvm#72 ? We haven't patched our LLVM in Debian yet.

@TimNN
Copy link
Contributor

TimNN commented Sep 18, 2017

@infinity0: I honestly have no idea. It doesn't look like, did you enable LLVM assertions in that build?

@infinity0
Copy link
Contributor Author

infinity0 commented Sep 18, 2017

I don't enable LLVM assertions on purpose and I guess they're not enabled by default.

The same test also failed on Fedora.

From IRC:

<markos> infinity0, I'm almost sure there is no support for VSX intrinsics for rust (yet) so it's natural for the test to fail
<markos> infinity0, what may be the case, is rust might have scalar fallback for simd code on engines that don't have actual support -so the code may compile and run albeit slower- however once the simd engine is enabled if it's incomplete, tests will fail
<markos> but I don't know exactly the case here

I guess if no-one pipes up I'll just ignore this test for the time being.

@infinity0
Copy link
Contributor Author

@cuviper

@infinity0 infinity0 changed the title 1.19 powerpc64le run-pass test failures: 1.19 powerpc64le run-pass test failures: simd-intrinsic-generic-cast, smallest-hello-world Sep 18, 2017
@Mark-Simulacrum Mark-Simulacrum added A-linkage Area: linking into static, shared libraries and binaries A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 18, 2017
@infinity0
Copy link
Contributor Author

I re-ran simd-intrinsic-generic-cast with LD_LIBRARY_PATH set to a rebuilt LLVM (with rust-lang/llvm#72 applied) and the failure remained, so I think it's unrelated.

BTW this is technically a regression, this test passed on Debian and Fedora before with 1.18, although markos from Debian's IRC thinks it's not really a regression, see the above quoted comments for details.

Also the smallest-hello-world error I think now is unrelated, I will move that to another bug report once I have some more details about it.

@infinity0 infinity0 changed the title 1.19 powerpc64le run-pass test failures: simd-intrinsic-generic-cast, smallest-hello-world 1.19 powerpc64le run-pass test failures: simd-intrinsic-generic-cast Sep 18, 2017
@infinity0
Copy link
Contributor Author

smallest-hello-world issue moved to #44683

@nagisa nagisa added the regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. label Sep 20, 2017
@nagisa
Copy link
Member

nagisa commented Sep 20, 2017

Presence or lack of VSX intrinsics in rust is irrelevant in this context, as the generated LLVM is calling regular fptosi <4 x double> %_ to <4 x i32>. This LLVM instruction is supposed to select proper instructions that execute this conversion.

If LLVM was upgraded in any way recently, that’s where the bug is happening. The code rustc generates appears to be exactly the same as I remember it being ages ago.

@nagisa nagisa removed the regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. label Sep 20, 2017
@infinity0
Copy link
Contributor Author

Thanks for the clarification. Indeed, both Fedora and Debian are using a less-patched LLVM 4.0 than Rust, and this is the first Rust version that uses 4.0. I'll go over the differences when I next have time and see if any patches are suggestive.

@cuviper
Copy link
Member

cuviper commented Apr 9, 2018

I can still reproduce this, even with rust using its own LLVM 6, and it gets the exact same output!

------------------------------------------
stderr:
------------------------------------------
thread 'main' panicked at 'f64 -> i32 (i32x4(-1234567, 12345678, -123456792, 1234567936) != i32x4(-1234567, 12345678, -123456789, 1234567890))', /home/jistone/rust/src/test/run-pass/simd-intrinsic-generic-cast.rs:128:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.

------------------------------------------

@cuviper
Copy link
Member

cuviper commented Jul 27, 2018

It appears that LLVM is generating the fptosi like f64 -> f32 -> i32, which loses significant bits in the f32. e.g. we want 1234567890 = 0x499602d2, but we got 1234567936 = 0x49960300.

See also:
https://bugzilla.redhat.com/show_bug.cgi?id=1562196
https://bugs.llvm.org/show_bug.cgi?id=38342

@cuviper cuviper added O-PowerPC Target: PowerPC processors and removed A-linkage Area: linking into static, shared libraries and binaries labels Jul 27, 2018
@programmerjake
Copy link
Member

the linked llvm bug has been fixed quite a while ago, has this issue been resolved by that?

@cuviper
Copy link
Member

cuviper commented Oct 13, 2020

Yeah, we should be good here.

@cuviper cuviper closed this as completed Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. O-PowerPC Target: PowerPC processors T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants