LLVM regression in beta with -Ctarget-feature=-simd128

[DaniPopes]

Code

I tried this code:

use std::arch::wasm32::*;

#[target_feature(enable = "simd128")]
pub unsafe fn some_simd128_fn(chunk: v128) -> bool {
    u8x16_all_true(chunk)
}

Flags: -Ctarget-feature=-simd128 --target wasm32-wasip1

Godbolt

I expected to see this happen: compiles

Instead, this happened: fails to compile with "rustc-LLVM ERROR: Do not know how to split this operator's operand!"

Version it worked on

It most recently worked on: 1.81.0

Version with regression

rustc --version --verbose:

rustc 1.82.0-beta.4 (8c27a2ba6 2024-09-21)
binary: rustc
commit-hash: 8c27a2ba6b21f3406a51118643080f0591949827
commit-date: 2024-09-21
host: x86_64-unknown-linux-gnu
release: 1.82.0-beta.4
LLVM version: 19.1.0
Compiler returned: 0

rustc 1.83.0-nightly (2bd1e894e 2024-09-26)
binary: rustc
commit-hash: 2bd1e894efde3b6be857ad345914a3b1cea51def
commit-date: 2024-09-26
host: x86_64-unknown-linux-gnu
release: 1.83.0-nightly
LLVM version: 19.1.0
Compiler returned: 0

@rustbot modify labels: +regression-from-stable-to-beta -regression-untriaged +A-LLVM

[apiraino]

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-medium

[hanna-kruppe]

Note that enabling simd128 only for some functions is not really useful because wasm doesn’t support dynamic feature detection: the entire module has to pass validation before any of it is executed, so either you have SIMD support or your module won’t load anyway. See https://doc.rust-lang.org/core/arch/wasm32/index.html#simd and BurntSushi/memchr#144 for discussion.

There’s still a bug here that should be fixed, but the workaround of building the entire program (except the sysroot) with -Ctarget-feature=+simd128 should not have any drawbacks in 99% of cases.

[nikic]

Looks like CoalesceFeaturesAndStripAtomics replaces "-simd128,+simd128" with "+multivalue,+mutable-globals,+reference-types,+sign-ext,+simd128,-simd128,"

[nikic]

Unfortunately, I can't seem to figure out how to reproduce this outside rustc. Just feeding the IR through llc gives be "+multivalue,+mutable-globals,+reference-types,+sign-ext,+simd128," without the incorrect -simd128 at the end.

[nikic]

@alexcrichton Do you happen to know whether there is some kind of special option one has to pass to llc to reproduce how rust emits wasm?

[alexcrichton]

This is a part of LLVM I've never personally fully understood but I believe that the target-feature attribute on functions is not equivalent to what's passed on the command line. I got the above to crash and with -Csave-temps it emitted foo.foo.93210e5e38e8209c-cgu.0.rcgu.bc as the crashing bitcode. Locally I then see:

$ llc foo.foo.93210e5e38e8209c-cgu.0.rcgu.bc -o wat.o -filetype=obj
$ llc foo.foo.93210e5e38e8209c-cgu.0.rcgu.bc -o wat.o -filetype=obj -mattr=-simd128
SplitVectorOperand Op #1: t13: i32 = llvm.wasm.alltrue TargetConstant:i32<12090>, t25

LLVM ERROR: Do not know how to split this operator's operand!

PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
...

which is a long way of saying "maybe -mattr=-simd128 perhaps?"

[nikic]

@alexcrichton Oh yeah, that's what I was missing. It looks like the behavior for this case was changed in llvm/llvm-project#80094, but it doesn't explain the reasoning behind it.

Edit: I think what that PR should probably have done is to explicitly materialize the - features rather than only the + features, rather than doing a forced override with the features from the target machine.

[alexcrichton]

Ah yeah I also don't know the motivation of that PR or what's behind it either. I've often run into small issues as well here and there with features and LLVM and I'm not sure if it's something that the WebAssembly backend is doing wrong or differently than other backends, or if it's just that I was holding things wrong at the time. I don't have anything specific on-hand though.

[nikic]

Candidate fix: llvm/llvm-project#110647

Unfortunately we can't actually use -mattr=-simd128 to test this, because the semantics of that one is to override the per-function features, unlike the semantics of -C target-feature in Rust.

[DianQK]

@rustbot label -llvm-fixed-upstream +O-wasm