Skip to content

mca/op: always define aarch64 macros #13246

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vogma
Copy link
Contributor

@vogma vogma commented May 10, 2025

This pull request refactors the aarch64 op component’s build configuration to ensure macros are consistently defined in compliance with the style guide.

There are no code logic changes. All changes in the source code files are preprocessor only.

Build system changes:
- configure.m4: remove check for Neon floating point support as it is never used
- configure.m4: always define macros to either 0 or 1
- Makefile.am: always declare GENERATE_NEON_CODE and GENERATE_SVE_CODE

Source code (only preprocessor):
- updated macro check to #if instead of #ifdef
- Add compile-time guard in op_aarch64_functions.c to ensure exactly one of GENERATE_SVE_CODE or GENERATE_NEON_CODE is enabled
- added comment above compile-time guard

This is a follow-up PR of #13204

    Build system changes:
    - configure.m4: remove check for Neon floating point support as it is never used
    - configure.m4: always define macros to either 0 or 1
    - Makefile.am: always declare GENERATE_NEON_CODE and GENERATE_SVE_CODE

    Source code (only preprocessor):
    - updated macro check to #if instead of #ifdef
    - Add compile-time guard in op_aarch64_functions.c to ensure exactly one of GENERATE_SVE_CODE or GENERATE_NEON_CODE is enabled
    - added comment above compile-time guard

    No changes to the source code logic. Only the build system and macro checks have been changed.

    Signed-off-by: Marco Vogel <[email protected]>
@@ -43,12 +43,12 @@ specialized_op_libs =
if MCA_BUILD_ompi_op_has_neon_support
specialized_op_libs += liblocal_ops_neon.la
liblocal_ops_neon_la_SOURCES = op_aarch64_functions.c
liblocal_ops_neon_la_CPPFLAGS = -DGENERATE_NEON_CODE
liblocal_ops_neon_la_CPPFLAGS = -DGENERATE_NEON_CODE=1 -DGENERATE_SVE_CODE=0
Copy link
Member

Choose a reason for hiding this comment

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

I know this isn't the point of this PR, but is there a difference between these macros and OMPI_MCA_OP_HAVE_NEON and OMPI_MCA_OP_HAVE_SVE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was wondering the same thing. As I understand it, if SVE and NEON support is detected during configuration, the file op_aarch64_functions.c is compiled twice (once with NEON intrinsics and once with SVE intrinsics) producing two libraries. So if both OMPI_MCA_OP_HAVE_NEON and OMPI_MCA_OP_HAVE_SVE are defined, we rely on the GENERATE_NEON_CODE and GENERATE_SVE_CODE macros to ensure that each library build emits only its respective feature implementations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants