Skip to content

Make is_ascii_hexdigit branchless #103024

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

Make is_ascii_hexdigit branchless #103024

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
acb42cf
Make is_ascii_hexdigit branchless
GKFX Oct 13, 2022
5b624ac
Fix typo in range
GKFX Oct 13, 2022
f624897
Clarify comment and use character literals in char::is_ascii_hexdigit
GKFX Oct 14, 2022
8d1d17f
Fully optimize is_ascii_hexdigit by hand
GKFX Oct 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion library/core/src/char/methods.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1510,7 +1510,8 @@ impl char {
#[rustc_const_stable(feature = "const_ascii_ctype_on_intrinsics", since = "1.47.0")]
#[inline]
pub const fn is_ascii_hexdigit(&self) -> bool {
matches!(*self, '0'..='9' | 'A'..='F' | 'a'..='f')
// Bitwise or can avoid need for branches in compiled code.
matches!(*self, '0'..='9') || matches!(*self as u32 | 0x20, 0x61..=0x66)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make this more maintainable, how about using (b'a' as u32)..=(b'f' as u32) instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately that would be a syntax error. I can't figure out a nice-looking way to use character literals here; RangeInclusive::contains isn't const, ('a' as u32 <= *self as u32 | 0x20) && ('f' as u32 >= *self as u32 | 0x20) is very long.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can still use that range in a pattern if you define it as a const item first.

Copy link
Contributor

@eggyal eggyal Oct 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One could make it slightly more concise:

let lower = *self as u32 | 0x20;
matches!(*self, '0'..='9') || (lower >= 'a' as u32 && lower <= 'f' as u32)

As an aside, there's a (currently private) const ASCII_CASE_MASK: u8 in core::num which might be preferable to a literal 0x20.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can still use that range in a pattern if you define it as a const item first.

    const LOWER_ASCII: RangeInclusive<u32> = ('a' as u32)..=('f' as u32);
    matches!(*c, '0'..='9') || matches!(*c as u32 | 0x20, LOWER_ASCII)

doesn't seem to compile? I can get it to work with A and F const items though which I have done.

}

/// Checks if the value is an ASCII punctuation character:
Expand Down
3 changes: 2 additions & 1 deletion library/core/src/num/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,8 @@ impl u8 {
#[rustc_const_stable(feature = "const_ascii_ctype_on_intrinsics", since = "1.47.0")]
#[inline]
pub const fn is_ascii_hexdigit(&self) -> bool {
matches!(*self, b'0'..=b'9' | b'A'..=b'F' | b'a'..=b'f')
// Bitwise or can avoid need for branches in compiled code.
matches!(*self, b'0'..=b'9') || matches!(*self | 0x20, b'a'..=b'f')
}

/// Checks if the value is an ASCII punctuation character:
Expand Down