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

LLVM crash with SIMD types and inline asm #27333

Closed
cesarb opened this issue Jul 27, 2015 · 7 comments
Closed

LLVM crash with SIMD types and inline asm #27333

cesarb opened this issue Jul 27, 2015 · 7 comments
Labels
A-inline-assembly Area: inline asm!(..) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. P-low Low priority requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@cesarb
Copy link
Contributor

cesarb commented Jul 27, 2015

I wrote some code which extracts the two halves of a u64x4, calls some arm neon inline assembly on each half, and combines them again. Trying to compile that code for arm crashes the compiler deep in the LLVM code.

I had previously used the exact same inline assembly without any problem, but I rewrote the surrounding code (previously, the two halves were already kept separate as a pair of u64x2).

The crashing code can be found at cesarb/blake2-rfc@36708b2 (the working code with the same inline asm can be found two commits before that one). Unfortunately, I failed to extract a reduced testcase; the crash only happens when the code is inlined inside the rest of the BLAKE2 core. The crash location within the LLVM code seems to point to some register allocation issue.

To reproduce the crash, you need a nightly rustc configured for cross-compiling to android, checkout that commit, and run the following command: cargo build -v --release --features="bench simd_asm" --target=arm-linux-androideabi (the offending code is gated by the "simd_asm" feature in the Cargo.toml).

Expected result: compile correctly so I can copy and modify the rustc command line to add a -C target-cpu=cortex-a9 -C target-feature=+neon and run the tests and benchmarks on my phone.

What happens: Process didn't exit successfully: [...] (signal: 4) (which is SIGILL according to kill -l). Using rust-gdb reveals the crash is deep within the LLVM code.

Versions:

rustc 1.3.0-dev (eefd6b2c1 2015-07-27)
binary: rustc
commit-hash: eefd6b2c12c8257c85f2121d7f5a3694b192e755
commit-date: 2015-07-27
host: x86_64-unknown-linux-gnu
release: 1.3.0-dev
cargo 0.4.0 (1ce0146 2015-07-27) (built 2015-07-27)
@huonw huonw added the A-inline-assembly Area: inline asm!(..) label Jul 27, 2015
@phillipCouto
Copy link

Maybe I am not seeing something but arm neon can only handle 64 and128 bit simd operations. So might that be why your previous version with 2 u64x2 version worked instead of the u64x4?

Your current version is trying to allocate a 256 bit number which is larger than what is supported.

@cesarb
Copy link
Contributor Author

cesarb commented Aug 5, 2015

@phillipCouto SSE2 can also handle only 64 and 128 bit SIMD. LLVM is able to work with a u64x4 just fine; it uses a pair of registers instead of a single one, both for SSE2 and NEON.

And the inline asm still receives a single u64x2 like before; I extract both halves of the u64x4, and call the inline asm once with each, as can be seen in the link I provided.

@eddyb
Copy link
Member

eddyb commented Jan 26, 2017

We should limit asm! inputs/outputs to the same kind of types that we allow in C variadics, I'd say.

@brson brson added P-low Low priority A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 26, 2017
@brson
Copy link
Contributor

brson commented Jan 26, 2017

cc #38735

@brson brson added the E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. label Jan 26, 2017
@eddyb
Copy link
Member

eddyb commented Jan 26, 2017

An issue with more discussion is #39083 - all of these have in common the fact that we're not limiting the types anyone can pass to asm even though we only support a limited subset of them in the first place.

@eddyb eddyb added E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. and removed E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. labels Jan 26, 2017
@eddyb
Copy link
Member

eddyb commented Jan 26, 2017

I was going to say "this is actually easy, copy a check from here to there", but, well, variadics only blacklist.

That said, whitelisting inline assembly shouldn't be too difficult - the condition would most likely be along the lines of ty.is_scalar() || ty.is_region_ptr() - but to use is_scalar, TyFnDef should be removed from its definition (as it's zero-sized now and the user probably intended using a function pointer instead).
I just checked and that last bit should not affect any of the other code using is_scalar.

@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 22, 2017
@Centril Centril added the requires-nightly This issue requires a nightly compiler in some way. label Oct 25, 2019
@Amanieu
Copy link
Member

Amanieu commented May 24, 2020

The new asm! implementation now properly supports passing in SIMD types. Please open a new issue if you encounter this bug with the new asm!.

@Amanieu Amanieu closed this as completed May 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-inline-assembly Area: inline asm!(..) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. P-low Low priority requires-nightly This issue requires a nightly compiler in some way. 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

8 participants