core: use compare_bytes for more slice element types

[joboet]

bool, NonZero<u8>, Option<NonZero<u8>> and ascii::Char can be compared the same way as u8.

[rustbot]

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
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

[rustbot]

The Miri subtree was changed

cc @rust-lang/miri

[oli-obk]

slices of all of these seem fairly rare. Not sure how I feel about adding more specialization stuff for them. Is there any motivation for this beyond "can be done"?

[joboet]

It's mostly "cause I can" but I can imagine that this is really beneficial when comparing ASCII-only strings.

[saethlin]

I think it would be a great shame if ascii::Char can't get the same optimizations as u8 and i8 (which probably sneaks in a fair bit as ffi::c_char).

But also I wonder if this optimization is better delegated to the compiler. The strategy here does improve ascii::Char, but it leaves out all types in user crates.

[scottmcm]

(Note that since this is talking about Ord for slices it can't do i8, as that compares differently.)

I don't know how feasible this would be to do in LLVM instead. Certainly it'll replace loops with calls to memset already; dunno if it'd be able to detect dynamic-length memcmp. There's probably silly reasons it's can't, like the byte-by-byte loop not asserting readability of any bytes after the last one, which memcmp might actually try to read...

[saethlin]

I wasn't thinking of LLVM specifically, just that it would be better if we could automate the detection of types eligible for this optimization by inspecting their layout, instead of attempting to enumerate all of them.

[scottmcm]

detection of types eligible for this optimization by inspecting their layout

I think the problem is that we need more than just their layout, because cmp::Reverse<u8> has the same layout as u8, but we can't do this. That's why I was thinking LLVM, because "detect a loop with these exact semantics" feels like an LLVM thing more than a rustc thing.

But maybe we could detect when it's from derive(Ord) on something where that works? I guess if slice-ord was directly an intrinsic and we could then detect cases that can call compare_bytes, otherwise leave it to fallback MIR?

[Mark-Simulacrum]

r? scottmcm

I tend to agree that a new specialization trait doesn't make me feel enthusiastic about this.

[scottmcm]

One small thing above, then I think this is good to go.

I'm less convinced about things like [bool], but being able to get a [NonZero<u8>] to the before-the-terminator in a CString for example would make sense, and it would be unfortunate if that meant a worse Ord.

So once we're doing it for two types, might as well do it for bool (and ascii::Char and …) as well.

[scottmcm]

I know it's a module-private thing, but can you elaborate on the name and add # Safety for the trait? Just "unsigned byte" could accidentally be interpreted as being more a layout thing, where say Rev<u8> is arguably an "unsigned byte" but definitely shouldn't be implementing this.

I see above it's BytewiseEq, so maybe UnsignedBytewiseOrd here or something?

[joboet]

@rustbot ready

[scottmcm]

Thanks!

@bors r+

[bors]

📌 Commit 8301dd4 has been approved by scottmcm

It is now in the queue for this repository.