Fix hardware capabilities detection; cksum --debug #9603
Merged
+183 −162
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
RenjiSann force-pushed the fix-hardware-detection branch 2 times, most recently from 41dd915 to 70f9707 Compare 
| GNU testsuite comparison: |
RenjiSann force-pushed the fix-hardware-detection branch from 70f9707 to 6a734c6 Compare | GNU testsuite comparison: |
RenjiSann force-pushed the fix-hardware-detection branch from 6a734c6 to e1fef5d Compare | GNU testsuite comparison: |
RenjiSann force-pushed the fix-hardware-detection branch from e1fef5d to 46c3bb6 Compare | GNU testsuite comparison: |
Contributor
| My string approach worked but was naive. Your enum-based design with TryFrom validation is the right way: type-safe, O(log n) lookups, feature alias support. Excellent work avx512: detect_avx512() && !disabled.contains(&"AVX512F".to_string()), avx2: detect_avx2() && !disabled.contains(&"AVX2".to_string()), pclmul: detect_pclmul() && !disabled.contains(&"PMULL".to_string()), vmull: detect_vmull() && !disabled.contains(&"VMULL".to_string()), sse2: detect_sse2() && !disabled.contains(&"SSE2".to_string()), asimd: detect_asimd() && !disabled.contains(&"ASIMD".to_string()), |
cakebaker reviewed Dec 12, 2025
Comment on lines 146 to 143
| let mut set = BTreeSet::new(); | ||
| if detect_avx512() { | ||
| set.insert(HardwareFeature::Avx512); | ||
| } | ||
| if self.avx2 { | ||
| features.push("AVX2"); | ||
| if detect_avx2() { | ||
| set.insert(HardwareFeature::Avx2); | ||
| } | ||
| if self.pclmul { | ||
| features.push("PCLMUL"); | ||
| if detect_pclmul() { | ||
| set.insert(HardwareFeature::PclMul); | ||
| } | ||
| if self.vmull { | ||
| features.push("VMULL"); | ||
| if detect_vmull() { | ||
| set.insert(HardwareFeature::Vmull); | ||
| } | ||
| if self.sse2 { | ||
| features.push("SSE2"); | ||
| if detect_sse2() { | ||
| set.insert(HardwareFeature::Sse2); | ||
| } | ||
| if self.asimd { | ||
| features.push("ASIMD"); | ||
| if detect_asimd() { | ||
| set.insert(HardwareFeature::Asimd); | ||
| } | ||
| features |
Contributor
There was a problem hiding this comment.
Here a functional approach might be cleaner, something like:
let set = [ (HardwareFeature::Avx512, detect_avx512 as fn() -> bool), (HardwareFeature::Avx2, detect_avx2), (HardwareFeature::PclMul, detect_pclmul), (HardwareFeature::Vmull, detect_vmull), (HardwareFeature::Sse2, detect_sse2), (HardwareFeature::Asimd, detect_asimd), ] .into_iter() .filter_map(|(feature, detect_func)| detect_func().then_some(feature)) .collect(); Collaborator Author
There was a problem hiding this comment.
Oh, that looks really clean, I will switch to that. Thanks !
RenjiSann force-pushed the fix-hardware-detection branch from 46c3bb6 to 231a857 Compare | GNU testsuite comparison: |
Contributor
Kudos, good work! |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
This MR fixes the GNU
cksum.shby correctly taking in account theGLIBC_TUNABLESenv varFixes #9518