ARM SHA256 intrinsics take state in natural order: ABCD + EFGH (hash_abcd/hash_efgh). Documented here
The code already uses that layout, only the ABEF_SAVE/CDGH_SAVE names were wrong.
Rename to ABCD_SAVE/EFGH_SAVE. No logic change.
Fix in original C code (Jeffrey): https://github.com/noloader/SHA-Intrinsics/pull/14
ARM SHA256 intrinsics take state in natural order: ABCD + EFGH
(hash_abcd/hash_efgh). The code already uses that layout, only the
`ABEF_SAVE`/`CDGH_SAVE` names were wrong.
Rename to `ABCD_SAVE`/`EFGH_SAVE`. No logic change.
Docs: https://developer.arm.com/architectures/instruction-sets/intrinsics/#f:@navigationhierarchiesinstructiongroup=[Cryptography,SHA256]
Fix in original C code (Jeffrey): https://github.com/noloader/SHA-Intrinsics/pull/14
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| Concept ACK | l0rinc |
If your review is incorrectly listed, please copy-paste <code><!--meta-tag:bot-skip--></code> into the comment that the bot should ignore.
<!--5faf32d7da4f0f540f40219e4f7537a3-->
Concept ACK.
Based on the description and the code path, this looks correct: STATE0 maps to s[0..3] (vld1q_u32(ptr) loads 4 consecutive lanes according to the docs) (and STATE1 is similar), so ABCD_SAVE / EFGH_SAVE is semantically aligned.
I checked other projects (e.g. https://botan.randombit.net/doxygen/sha1__armv8_8cpp_source.html#l00037) and did a compared the local binary and this appears to be a correct rename-only clarity fix with no logic change.
Consider keeping the scripted rename split into two explicit substitutions to make review simpler, e.g.
perl -pi -e 's/\bABEF_SAVE([AB])?\b/ABCD_SAVE$1/g' src/crypto/sha256_arm_shani.cppperl -pi -e 's/\bCDGH_SAVE([AB])?\b/EFGH_SAVE$1/g' src/crypto/sha256_arm_shani.cpp