slice/ascii: Optimize eq_ignore_ascii_case with auto-vectorization

[okaneco]

• Refactor the current functionality into a helper function
• Use as_chunks to encourage auto-vectorization in the optimized chunk processing function
• Add a codegen test checking for vectorization and no panicking
• Add benches for eq_ignore_ascii_case

---

The optimized function is initially only enabled for x86_64 which has sse2 as part of its baseline, but none of the code is platform specific. Other platforms with SIMD instructions may also benefit from this implementation.

Performance improvements only manifest for slices of 16 bytes or longer, so the optimized path is gated behind a length check for greater than or equal to 16.

Benchmarks - Cases below 16 bytes are unaffected, cases above all show sizeable improvements.

before:
    str::eq_ignore_ascii_case::bench_large_str_eq         4942.30ns/iter +/- 48.20
    str::eq_ignore_ascii_case::bench_medium_str_eq         632.01ns/iter +/- 16.87
    str::eq_ignore_ascii_case::bench_str_17_bytes_eq        16.28ns/iter  +/- 0.45
    str::eq_ignore_ascii_case::bench_str_31_bytes_eq        35.23ns/iter  +/- 2.28
    str::eq_ignore_ascii_case::bench_str_of_8_bytes_eq       7.56ns/iter  +/- 0.22
    str::eq_ignore_ascii_case::bench_str_under_8_bytes_eq    2.64ns/iter  +/- 0.06
after:
    str::eq_ignore_ascii_case::bench_large_str_eq         611.63ns/iter +/- 28.29
    str::eq_ignore_ascii_case::bench_medium_str_eq         77.10ns/iter +/- 19.76
    str::eq_ignore_ascii_case::bench_str_17_bytes_eq        3.49ns/iter  +/- 0.39
    str::eq_ignore_ascii_case::bench_str_31_bytes_eq        3.50ns/iter  +/- 0.27
    str::eq_ignore_ascii_case::bench_str_of_8_bytes_eq      7.27ns/iter  +/- 0.09
    str::eq_ignore_ascii_case::bench_str_under_8_bytes_eq   2.60ns/iter  +/- 0.05

[rustbot]

r? @scottmcm

rustbot has assigned @scottmcm.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[okaneco]

I've pushed a commit to avoid falling back to the scalar checking for the remainder handling.

We reload the last 16 bytes of the slices if there's a remainder, which improves the 31 byte case and doesn't seem to regress the 17 byte case.

scalar tail handling
    ascii::eq_ignore_ascii_case::bench_long_str_eq          54.75ns/iter +/- 1.51
    ascii::eq_ignore_ascii_case::bench_str_17_bytes_eq       4.77ns/iter +/- 0.12
    ascii::eq_ignore_ascii_case::bench_str_31_bytes_eq      23.00ns/iter +/- 4.56
    ascii::eq_ignore_ascii_case::bench_str_of_8_bytes_eq     7.61ns/iter +/- 0.16
    ascii::eq_ignore_ascii_case::bench_str_under_8_bytes_eq  2.61ns/iter +/- 0.07
load last 16 bytes of the slice, newest commit
    ascii::eq_ignore_ascii_case::bench_long_str_eq          51.60ns/iter +/- 5.28
    ascii::eq_ignore_ascii_case::bench_str_17_bytes_eq       3.62ns/iter +/- 0.54
    ascii::eq_ignore_ascii_case::bench_str_31_bytes_eq       3.56ns/iter +/- 0.27
    ascii::eq_ignore_ascii_case::bench_str_of_8_bytes_eq     7.79ns/iter +/- 1.01
    ascii::eq_ignore_ascii_case::bench_str_under_8_bytes_eq  2.73ns/iter +/- 0.05

[scottmcm]

fix: you don't need always here. It's a non-generic function which is why it went badly with no attribute, but #[inline] is sufficient.

Suggested change

	#[inline(always)]
	#[inline]

[okaneco]

Without always, it fails the filecheck test I've written and doesn't seem to inline as shown in the godbolt.

Left is inline(always), right is inline
https://rust.godbolt.org/z/Px6nMPWvK

edit: I realize above I included const generics.
With all generics removed, it still doesn't seem to inline the inner call without always
https://rust.godbolt.org/z/4Tq5hWY6T

[scottmcm]

This is only correct because of a check in the caller that's not documented in the comments on this method nor in the code here.

I would suggest at least having a debug_assert!(self.len() >= N) or similar here to help communicate that invariant.

[okaneco]

Agreed, I've been thinking about this since my last push.

I think calling this function with a const generic parameter might be a better choice to encode the information, so I'm going to change it to that along with a debug_assert here.

[scottmcm]

fix: there should be const-hack comments on these while loops (or the functions).

[scottmcm]

Hmm, it feels wrong that we need to check both lengths here. The answer can never actually differ, right? (Because the caller checks equal-length first anyway.)

The function boundary split feels awkward, since it's losing information -- both in simple and non-simple -- that would be helpful to the optimizer when it comes to removing bounds checks.

What does the actual loop condition look like in llvm or asm here?

[okaneco]

Hmm, it feels wrong that we need to check both lengths here. The answer can never actually differ, right? (Because the caller checks equal-length first anyway.)

It does feel wrong, and you are correct about how they can never differ. I included this to eliminate an extra branch and panic in the loop. The comparison gets optimized out so there is no extra length check. I never got around to filing this one as a bug report.

What does the actual loop condition look like in llvm or asm here?

Left is with both checks, right is with only 1
https://rust.godbolt.org/z/5Ks3z9MTM

Difference in the loop condition is this on the right which jumps to a panic

.LBB0_3:
        sub     r8, 1
        jb      .LBB0_15

edit: here's a more minimal version with just the chunk loop https://rust.godbolt.org/z/MGEv3qszY

[okaneco]

Reported as #147953

[scottmcm]

This might be the classic problem of function boundaries really mattering to optimizations. If you look at

rust/library/core/src/slice/mod.rs

Lines 1005 to 1011 in b3cda16

	// Because this function is first compiled in isolation,
	// this check tells LLVM that the indexing below is
	// in-bounds. Then after inlining -- once the actual
	// lengths of the slices are known -- it's removed.
	// FIXME(const_trait_impl) replace with let (a, b) = (&mut a[..n], &mut b[..n]);
	let (a, _) = a.split_at_mut(n);
	let (b, _) = b.split_at_mut(n);

for example that just adds an assert to let this stuff optimize out, then that assert optimizes out later.

[okaneco]

Addressed most of the feedback:

• Added comments about the invariants to the caller function
• Added const-hack comments for using while-loops
• Documented the invariant for the _chunks function in the function documentation
• Added a debug assert for the tail handling invariant

I changed the _chunks function to take a const generic usize parameter to bubble up the invariant to the caller.

---

There's one other possibly missed optimization (if it's correct) in the tail handling portion, but it requires unsafe. It's a small difference in instructions.
Not sure if there's a safe way to golf this.
https://rust.godbolt.org/z/G1fPnE881

if let (Some(a_rem), Some(b_rem)) = (s.last_chunk::<N>(), other.last_chunk::<N>()) {
    if !eq_ignore_ascii_inner(a_rem, b_rem) {
        return false;
    }
}

// Can be rewritten as...
let a_rem: &[u8; N] = unsafe { &*s.as_ptr().add(s.len() - N).cast() };
let b_rem: &[u8; N] = unsafe { &*other.as_ptr().add(other.len() - N).cast() };
if !eq_ignore_ascii_inner(a_rem, b_rem) {
    return false;
}

[scottmcm]

Thanks for the updates. I think this can land as-is -- we can always tweak it more later if needed.

@bors r+ rollup=iffy (new codegen test is always scary)

[rust-bors]

📌 Commit 31bf836 has been approved by scottmcm

It is now in the queue for this repository.