Fix simd_bswap for i8/u8

[calebzulawski]

#114156 missed this test case ☹️
cc @workingjubilee

[rustbot]

r? @compiler-errors

(rustbot has picked a reviewer for you, use r? to override)

[compiler-errors]

Do we expect people to be calling this intrinsic on i8xN? This seems like a misuse of the intrinsic, or at least for LLVM, since llvm.bswap.* says the element needs an even number of bytes.

[workingjubilee]

indeed, what are we actually trying to achieve here?

[calebzulawski]

This mirrors the scalar bswap intrinsic:

rust/compiler/rustc_codegen_llvm/src/intrinsic.rs

Lines 251 to 260 in a8be6e0

	sym::bswap => {
	if width == 8 {
	args[0].immediate() // byte swap a u8/i8 is just a no-op
	} else {
	self.call_intrinsic(
	&format!("llvm.bswap.i{}", width),
	&[args[0].immediate()],
	)
	}
	}

It would be a bit of a pain to special-case i8/u8 vectors in std::simd.

[workingjubilee]

or more precisely, what is the current behavior we're trying to avoid?

[calebzulawski]

or more precisely, what is the current behavior we're trying to avoid?

I thought LLVM would handle a no-op, but instead the compiler crashes:

warning: build failed, waiting for other jobs to finish...
bswap must be an even number of bytes
  %4 = call <32 x i8> @llvm.bswap.v32i8(<32 x i8> %3), !dbg !858
in function _ZN113_$LT$core_simd..core_simd..vector..Simd$LT$i8$C$_$GT$$u20$as$u20$core_simd..core_simd..elements..int..SimdInt$GT$10swap_bytes17h00fedc46e657fc9cE
LLVM ERROR: Broken function found, compilation aborted!
error: could not compile `core_simd` (test "i8_ops")

[compiler-errors]

I guess r=me if @workingjubilee thinks that this is worthwhile -- the fact that we're special-casing i8 in both the simd and non-simd bswap intrinsics is a tiny bit odd, but I could believe it makes sense with like... macro-generated code or something. Still kinda sus, though.

[workingjubilee]

lmao llvm

[calebzulawski]

Both scalar integers and std::simd integer vectors are macro-generated 🙃 No users should be seeing these intrinsics.

[compiler-errors]

To be fair to LLVM, it says very clearly that the intrinsic is only defined when BitWidth % 16 == 0.

[workingjubilee]

I guess r=me if @workingjubilee thinks that this is worthwhile

yeaaaaah, I guess.

but I could believe it makes sense with like... macro-generated code or something

that describes all of std's integer impls, baby!

@bors r=compiler-errors

[bors]

📌 Commit 77ed437 has been approved by compiler-errors

It is now in the queue for this repository.

[workingjubilee]

@bors r- rollup=iffy
cc #114270

I have no idea but I'm almost certain it's this PR.

[calebzulawski]

The failed test is tests/ui/lint/must_not_suspend/handled.rs, I don't see any way this PR could affect that. Maybe spurious?

[workingjubilee]

...hm yeah you're right, I misread the logs at-a-glance.

Probably is spurious then.

@bors r=compiler-errors

[bors]

📌 Commit 77ed437 has been approved by compiler-errors

It is now in the queue for this repository.

[bors]

⌛ Testing commit 77ed437 with merge 3be07c1...

[bors]

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 3be07c1 to master...

[rust-timer]

Finished benchmarking commit (3be07c1): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	5.8%	[5.8%, 5.8%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	6.4%	[6.4%, 6.4%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	6.4%	[6.4%, 6.4%]	1

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.3%	[2.1%, 2.6%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 652.418s -> 653.862s (0.22%)

[nnethercote]

One possible, marginal regression, nothing to worry about.

@rustbot label: +perf-regression-triaged