uucore: name hardware features after the CPU instructions by ChihweiLHBird · Pull Request #15232 · uutils/coreutils

ChihweiLHBird · 2026-10-10T17:59:43Z

Summary

HardwareFeature::PclMul and HardwareFeature::Vmull take their names from the two CRC paths that cksum --debug reports as pclmul and vmull. But they detect CPU instructions: is_x86_feature_detected!("pclmulqdq") and is_aarch64_feature_detected!("pmull"). The distinction matters because a path needs more than its instruction. For example, the pclmul path also needs AVX, so a check such as has_pclmul() && has_avx() (in a follow up PR) reads as if pclmul needs more than pclmul.

This PR renames the variants to Pclmulqdq and Pmull, with has_pclmulqdq(), has_pmull() and the detect_* functions to match. Now the enum names the instruction and cksum names the path. Behavior does not change: cksum --debug still prints pclmul and vmull, and the GLIBC_TUNABLES names and wc --debug labels stay the same but may be updated in the follow up PRs to align with GNU behaviors.

No behavior change is intended in this PR.

Copilot

🟢 Approval recommended

The rename is complete and preserves existing detection and output behavior.

0 open findings

What changed in this PR

Renames hardware feature APIs to match the detected CPU instructions without changing user-visible debug labels or behavior.

Changes:

Renames PCLMULQDQ and PMULL feature variants, methods, and detectors.
Updates cksum and wc call sites while preserving output labels.

File	Description
`src/uucore/src/lib/features/hardware.rs`	Renames hardware feature APIs.
`src/uu/cksum/src/cksum.rs`	Uses the renamed detection methods.
`src/uu/wc/src/wc.rs`	Updates variants while retaining existing labels.

🧠 Review effort: Balanced

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

codspeed · 2026-10-10T18:43:25Z

Merging this PR will not alter performance

⚠️

123 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

sort_very_long_lines_utf8_locale

cksum_multiple_files

du_all_wide_tree[(5000, 500)]

cksum_raw_output

du_summarize_balanced_tree[(5, 4, 10)]

cksum_crc

cksum_crc32b

factor_multiple_u64s[2]

du_wide_tree[(5000, 500)]

dd_copy_64k_blocks

…and 113 more

ℹ️ 1 benchmark has no flame graph

The flamegraph could not be generated because too much time was spent in runtime internals, making the visualization not useful for performance analysis.

sort_dictionary_order[500000]

✅ 401 untouched benchmarks
⏩ 70 skipped benchmarks¹

_{Comparing ChihweiLHBird:zhiwei/hardware-feature-names (c4cdefb) with main (54d1309)²}

70 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
No successful run was found on main (bf9513b) during the generation of this report, so 54d1309 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

Copilot

🟢 Approval recommended

The rename is complete and consistent, with no unresolved references or behavior changes.

0 open findings

🧠 Review effort: Balanced

sylvestre · 2026-10-13T06:42:02Z

            "AVX2" => Ok(Avx2),
-            "PCLMUL" | "PMULL" => Ok(PclMul),
-            "VMULL" => Ok(Vmull),
+            "PCLMUL" | "PMULL" => Ok(Pclmulqdq),


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

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

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).

Copilot

🟢 Approval recommended

The rename is complete and preserves existing behavior across all affected call sites.

0 open findings

🧠 Review effort: Balanced

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

1 open finding

PMULL currently parses to HardwareFeature::Pclmulqdq, which is the x86 instruction, and… · New

🧠 Review effort: Lite

github-actions · 2026-10-15T08:48:50Z

GNU testsuite comparison:

Congrats! The gnu test tests/cat/cat-distinct-err is no longer failing!

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

2 open findings

try_from maps "PMULL" to HardwareFeature::Pclmulqdq, but PMULL is the AArch64 instruction… · New
These labels intentionally use the path names (PCLMUL/VMULL) rather than the instruction… · New

1 resolved since last review

PMULL currently parses to HardwareFeature::Pclmulqdq, which is the x86 instruction, and…

🧠 Review effort: Lite

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

1 open finding

"PMULL"is currently parsed asHardwareFeature::Pclmulqdq`, which is an x86 instruction feature… · New

2 resolved since last review

try_from maps "PMULL" to HardwareFeature::Pclmulqdq, but PMULL is the AArch64 instruction…
These labels intentionally use the path names (PCLMUL/VMULL) rather than the instruction…

🧠 Review effort: Lite

Copilot

🔵 Needs a closer look

No code issues were identified, but required cross-platform CI results remain unverified.

0 open findings

1 resolved since last review

"PMULL"is currently parsed asHardwareFeature::Pclmulqdq`, which is an x86 instruction feature…

🧠 Review effort: Balanced

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

1 open finding

TryFrom<&str> maps the string PMULL to HardwareFeature::Pclmulqdq, which is incorrect (PMULL… · New

🧠 Review effort: Lite

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.

`PclMul` and `Vmull` were named after the cksum CRC paths, not after the PCLMULQDQ and PMULL instructions that they detect. Rename them, and their `has_*` and `detect_*` functions, after the instructions. Signed-off-by: Zhiwei Liang <zhiwei.liang@zliang.me>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.

sylvestre · 2026-10-19T19:59:43Z

Thanks for your PR

Copilot AI balanced review requested due to automatic review settings October 10, 2026 17:59

Copilot started reviewing on behalf of ChihweiLHBird October 10, 2026 18:00

View session

Copilot AI reviewed Oct 10, 2026

View reviewed changes

ChihweiLHBird force-pushed the zhiwei/hardware-feature-names branch from b9051d8 to 53687b2 Compare October 13, 2026 00:20

Copilot AI balanced review requested due to automatic review settings October 13, 2026 00:20

Copilot started reviewing on behalf of ChihweiLHBird October 13, 2026 00:27

View session

Copilot AI reviewed Oct 13, 2026

View reviewed changes

sylvestre reviewed Oct 13, 2026

View reviewed changes

Comment thread src/uucore/src/lib/features/hardware.rs Outdated

sylvestre reviewed Oct 13, 2026

View reviewed changes

ChihweiLHBird force-pushed the zhiwei/hardware-feature-names branch from 53687b2 to 556b621 Compare October 13, 2026 15:11

Copilot AI balanced review requested due to automatic review settings October 13, 2026 15:11

Copilot started reviewing on behalf of ChihweiLHBird October 13, 2026 15:12

View session

Copilot AI reviewed Oct 13, 2026

View reviewed changes

ChihweiLHBird requested a review from sylvestre October 14, 2026 04:17

ChihweiLHBird force-pushed the zhiwei/hardware-feature-names branch from 556b621 to 69dfb27 Compare October 15, 2026 07:37

Copilot AI balanced review requested due to automatic review settings October 15, 2026 07:37