Skip to content
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.

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

arm: Rename internal ARM ROL/ROR/LSR/LSL functions with a SIMDE prefix. #1252

Merged
merged 3 commits into from
Jan 3, 2025

Conversation

Syonyk
Copy link
Contributor

@Syonyk Syonyk commented Dec 31, 2024

Most SIMDe functions/macros/etc have the SIMDE_ or simde_ prefix internally. Several of the ARM hash functions do not have this prefix on their ROR/ROL/LSR/LSL macros, and this means that the standard names of can conflict with other projects using the library. This was an issue on an internal tool using the library, and while SIMDe undefines the macros after the file uses them, this is not compatible with other declarations of these macros before the SIMDe library is included.

This is purely a cosmetic change with internal function renaming. It changes no external interfaces, simply removes a source of potential conflict with other projects that have similarly named macros. Tests have not been changed, as there are no changes that would require test modification.

Copy link
Collaborator

@mr-c mr-c left a comment

Choose a reason for hiding this comment

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

@Syonyk Can you look at https://github.com/simd-everywhere/simde/actions/runs/12564616889/job/35027928010?pr=1252#step:8:2991 ?

../test/arm/neon/../../../simde/arm/neon/sm3.h: In function ‘test_simde_vsm3tt1aq_u32’:
../test/arm/neon/../../../simde/arm/neon/sm3.h:70:7: error: ‘r_.values’ may be used uninitialized [-Werror=maybe-uninitialized]
   70 |       r_,

@Syonyk
Copy link
Contributor Author

Syonyk commented Jan 2, 2025

Yeah, I'll take a look at that - not sure why my changes would have impacted that, but it shouldn't be a hard fix.

@mr-c
Copy link
Collaborator

mr-c commented Jan 2, 2025

Yeah, I'll take a look at that -

Thank you!

not sure why my changes would have impacted that

Welcome to the wonderful world of testing with many different compilers & configurations 😭 😃

@Syonyk
Copy link
Contributor Author

Syonyk commented Jan 2, 2025

Possible fix for it pushed, I don't see r_ being used uninitialized, but I've set it to zero on creation. Now to see if the tests all pass...

@mr-c
Copy link
Collaborator

mr-c commented Jan 2, 2025

Possible fix for it pushed, I don't see r_ being used uninitialized, but I've set it to zero on creation. Now to see if the tests all pass...

https://github.com/simd-everywhere/simde/actions/runs/12588180130/job/35085567566?pr=1252#step:8:3001

Maybe better to use SIMDE_DIAGNOSTIC_DISABLE_UNINITIALIZED_ in the tests or in simde/arm/neon/sm3.h

If you want the experience, you could also file a bug with the GCC people, letting them know of this spurious warning in gcc-14 using the riscv64 target and -03

@Syonyk
Copy link
Contributor Author

Syonyk commented Jan 2, 2025

I see that... let me try your approach. I'll gate it on RISCV for now, and see if that makes things happy.

@Syonyk
Copy link
Contributor Author

Syonyk commented Jan 2, 2025

Does this pass at head? I don't know how to kick off a set of tests against current master.

@mr-c
Copy link
Collaborator

mr-c commented Jan 2, 2025

Does this pass at head? I don't know how to kick off a set of tests against current master.

I'm going to debug locally and I'll force push any needed changes; thank you!

Syonyk and others added 3 commits January 2, 2025 15:38
Most SIMDe functions have the SIMDE_ or simde_ prefix internally.
Several of the ARM hash functions do not have this prefix, and this
means that the standard names of ROR32/ROL32 can conflict with other
projects using the library.

This is purely a cosmetic change with internal function renaming.  It
changes no external interfaces, simply removes a source of potential
conflict with other projects that have similarly named macros.
…eon SM3 functions

r_.values _is_ being initialized.  The compiler is missing this.

Possible compiler bug.

Co-authored-by: Michael R. Crusoe <[email protected]>
@mr-c mr-c enabled auto-merge (rebase) January 2, 2025 23:39
@mr-c mr-c merged commit ed042d5 into simd-everywhere:master Jan 3, 2025
96 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants