-
Notifications
You must be signed in to change notification settings - Fork 940
Replace custom alignment_of_long_double with alignof #13613
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR modernizes the codebase by replacing a custom alignment_of_long_double() helper function with the C11 standard alignof operator. The change removes legacy compatibility code now that the project requires C11.
Key changes:
- Removed the custom
alignment_of_long_double()function (lines 484-507 removed) - Updated two call sites to use
alignof(long double)directly
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ldbl_is_aligned = 1; | ||
| int alignment_mask = alignment_of_long_double() - 1; | ||
| int alignment_mask = alignof(long double) - 1; |
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code uses alignof without including <stdalign.h>. In C11, alignof is a macro defined in <stdalign.h> that expands to _Alignof, while _Alignof is the actual keyword. For consistency with other parts of the codebase (see opal/include/opal/align.h line 33), and to avoid requiring <stdalign.h>, consider using _Alignof(long double) directly instead of alignof(long double).
| int alignment_mask = alignof(long double) - 1; | |
| int alignment_mask = _Alignof(long double) - 1; |
|
|
||
| ldbl_is_aligned = 1; | ||
| int alignment_mask = alignment_of_long_double() - 1; | ||
| int alignment_mask = alignof(long double) - 1; |
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code uses alignof without including <stdalign.h>. In C11, alignof is a macro defined in <stdalign.h> that expands to _Alignof, while _Alignof is the actual keyword. For consistency with other parts of the codebase (see opal/include/opal/align.h line 33), and to avoid requiring <stdalign.h>, consider using _Alignof(long double) directly instead of alignof(long double).
| int alignment_mask = alignof(long double) - 1; | |
| int alignment_mask = _Alignof(long double) - 1; |
bosilca
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like copilot suggestion
_Alignof was standardized in C11 which OMPI now requires. Signed-off-by: Joseph Schuchart <[email protected]>
|
Agreed, makes it clear that it's a language feature. Force-pushed a change. |
alignof was standardized in C11 and OMPI now requires it.