Skip to content

uucore: name hardware features after the CPU instructions #15232

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

Open
ChihweiLHBird wants to merge 1 commit into
base: main
Choose a base branch
from
+23 −22
Open

uucore: name hardware features after the CPU instructions #15232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions src/uu/cksum/src/cksum.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ fn print_cpu_debug_info() {
if cfg!(any(target_arch = "x86", target_arch = "x86_64")) {
print_feature("avx512", features.has_avx512());
print_feature("avx2", features.has_avx2());
print_feature("pclmul", features.has_pclmul());
print_feature("pclmul", features.has_pclmulqdq());
}

// ARM aarch64
if cfg!(target_arch = "aarch64") {
print_feature("vmull", features.has_vmull());
print_feature("vmull", features.has_pmull());
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/uu/wc/src/wc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -831,8 +831,8 @@ fn hardware_feature_label(feature: HardwareFeature) -> &'static str {
match feature {
HardwareFeature::Avx512 => "AVX512F",
HardwareFeature::Avx2 => "AVX2",
HardwareFeature::PclMul => "PCLMUL",
HardwareFeature::Vmull => "VMULL",
HardwareFeature::Pclmulqdq => "PCLMUL",
HardwareFeature::Pmull => "VMULL",
Comment thread
ChihweiLHBird marked this conversation as resolved.
HardwareFeature::Sse2 => "SSE2",
HardwareFeature::Asimd => "ASIMD",
}
Expand Down
37 changes: 19 additions & 18 deletions src/uucore/src/lib/features/hardware.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ pub enum HardwareFeature {
/// AVX2 support (x86/x86_64 only)
Avx2,
/// PCLMULQDQ support for CRC acceleration (x86/x86_64 only)
PclMul,
Pclmulqdq,
/// PMULL (polynomial multiply) support for CRC acceleration (aarch64 only)
Vmull,
Pmull,
/// SSE2 support (x86/x86_64 only)
Sse2,
/// ARM ASIMD/NEON support (aarch64 only)
Expand All @@ -68,12 +68,14 @@ impl TryFrom<&str> for HardwareFeature {
type Error = InvalidHardwareFeature;

fn try_from(value: &str) -> Result<Self, Self::Error> {
use HardwareFeature::{Asimd, Avx2, Avx512, PclMul, Sse2, Vmull};
use HardwareFeature::{Asimd, Avx2, Avx512, Pclmulqdq, Pmull, Sse2};
// TODO: like GNU, accept `PCLMULQDQ` for `Pclmulqdq` and `PMULL` for
// `Pmull`.
match value {
"AVX512" | "AVX512F" => Ok(Avx512),
"AVX2" => Ok(Avx2),
"PCLMUL" | "PMULL" => Ok(PclMul),
"VMULL" => Ok(Vmull),
"PCLMUL" | "PMULL" => Ok(Pclmulqdq),

@sylvestre sylvestre Oct 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

with the new names, "PMULL" => Pclmulqdq looks wrong, is that intended?

@ChihweiLHBird ChihweiLHBird Oct 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's intended to keep this as a non-behavior-change PR and avoid a large hard-to-review diff. But I do have a plan to change them later after fixing another bug it's relying on for preventing regressions.

A sample of such regression of we make the change now for the keys here:

With GLIBC_TUNABLES=glibc.cpu.hwcaps=-PMULL

GNU:           cksum: pclmul support not detected
main:          cksum: pclmul support not detected
keys changed:  cksum: using pclmul hardware support

@ChihweiLHBird ChihweiLHBird Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hi @sylvestre, may I get this re-review to unblock the follow up work? Or if you still think this rename is not right, I can combine this PR with the follow up work but diff will be larger (might be harder to review).

"VMULL" => Ok(Pmull),
Comment thread
ChihweiLHBird marked this conversation as resolved.
Comment thread
ChihweiLHBird marked this conversation as resolved.
"SSE2" => Ok(Sse2),
Comment thread
ChihweiLHBird marked this conversation as resolved.
"ASIMD" => Ok(Asimd),
_ => Err(InvalidHardwareFeature),
Expand Down Expand Up @@ -105,14 +107,14 @@ pub trait HasHardwareFeatures {

/// Check if PCLMULQDQ is available (x86/x86_64 only)
#[inline]
fn has_pclmul(&self) -> bool {
self.has_feature(HardwareFeature::PclMul)
fn has_pclmulqdq(&self) -> bool {
self.has_feature(HardwareFeature::Pclmulqdq)
}

/// Check if VMULL is available (ARM only)
/// Check if PMULL is available (aarch64 only)
#[inline]
fn has_vmull(&self) -> bool {
self.has_feature(HardwareFeature::Vmull)
fn has_pmull(&self) -> bool {
self.has_feature(HardwareFeature::Pmull)
}

/// Check if SSE2 is available (x86/x86_64 only)
Expand Down Expand Up @@ -146,8 +148,8 @@ impl CpuFeatures {
let set = [
(HardwareFeature::Avx512, detect_avx512 as fn() -> bool),
(HardwareFeature::Avx2, detect_avx2),
(HardwareFeature::PclMul, detect_pclmul),
(HardwareFeature::Vmull, detect_vmull),
(HardwareFeature::Pclmulqdq, detect_pclmulqdq),
(HardwareFeature::Pmull, detect_pmull),
(HardwareFeature::Sse2, detect_sse2),
(HardwareFeature::Asimd, detect_asimd),
]
Expand Down Expand Up @@ -269,7 +271,7 @@ fn detect_avx2() -> bool {
}

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
fn detect_pclmul() -> bool {
fn detect_pclmulqdq() -> bool {
if cfg!(target_os = "android") {
false
} else {
Expand All @@ -278,7 +280,7 @@ fn detect_pclmul() -> bool {
}

#[cfg(not(any(target_arch = "x86", target_arch = "x86_64")))]
fn detect_pclmul() -> bool {
fn detect_pclmulqdq() -> bool {
false
}

Expand Down Expand Up @@ -311,9 +313,8 @@ fn detect_asimd() -> bool {
}

#[cfg(target_arch = "aarch64")]
fn detect_vmull() -> bool {
// VMULL on 64-bit lanes needs PMULL, an optional extension.
// ASIMD alone is not enough.
fn detect_pmull() -> bool {
// PMULL is an optional extension. ASIMD alone is not enough.
if cfg!(target_os = "android") {
false
} else {
Expand All @@ -322,7 +323,7 @@ fn detect_vmull() -> bool {
}

#[cfg(not(target_arch = "aarch64"))]
fn detect_vmull() -> bool {
fn detect_pmull() -> bool {
false
}

Expand Down
Loading