Skip to content

build: Add static component pointers for LTO #13193

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

Merged
merged 2 commits into from
May 13, 2025

Conversation

bwbarrett
Copy link
Member

With GCC's link time optimization, the compiler ends up seeing two declarations of MCA component structures. One in the component itself, which has potentially a component-specific type, and one in static-components.h, which is just declared as a base MCA component struct. The compiler doesn't like this.

This patch adds a level of indirection used by the static component lookup code. static-components.h now declares the existance of a mca_component_t * and the list is of pointers to pointers. The MCA base framework has to do an extra dereference, but this is only in the initialization path. We also then have to have the actual pointer exist, so add a macro MCA_BASE_COMPONENT_INIT that we need to add to every component definition to create the pointer (and perhaps do more initialization later).

bosilca
bosilca previously approved these changes Apr 14, 2025
devreal
devreal previously approved these changes Apr 14, 2025
jsquyres
jsquyres previously approved these changes Apr 15, 2025
@bwbarrett bwbarrett dismissed stale reviews from jsquyres, devreal, and bosilca via 79b9f3d May 6, 2025 15:52
@bwbarrett bwbarrett force-pushed the bugfix/lto-is-a-pain branch from e338bf4 to 79b9f3d Compare May 6, 2025 15:52
bwbarrett and others added 2 commits May 13, 2025 07:07
Add a bit of indirection on the hook initialization code to add a
minimal amount of type checking around the component type.  This still
assumes that a const mca_base_component_t * can be cast to a
const ompi_hook_base_component_ti *, but at least if the thing that
we're reading from is giving us something other than a
const mca_base_component_t *, the compiler will get angry with us.

Signed-off-by: Brian Barrett <[email protected]>
With GCC's link time optimization, the compiler ends up seeing two
declarations of MCA component structures.  One in the component
itself, which has potentially a component-specific type, and one
in static-components.h, which is just declared as a base MCA
component struct.  The compiler doesn't like this.

This patch adds a level of indirection used by the static component
lookup code.  static-components.h now declares the existance of a
mca_component_t * and the list is of pointers to pointers.  The
MCA base framework has to do an extra dereference, but this is only
in the initialization path.  We also then have to have the actual
pointer exist, so add a macro MCA_BASE_COMPONENT_INIT that we need
to add to every component definition to create the pointer (and
perhaps do more initialization later).

Signed-off-by: Brian Barrett <[email protected]>
@bwbarrett bwbarrett force-pushed the bugfix/lto-is-a-pain branch from 79b9f3d to 93a4464 Compare May 13, 2025 14:08
@bwbarrett
Copy link
Member Author

@bosilca, @devreal, and @jsquyres I think this is now ready to go. Found the crash (missed updating one of the paths in the hook code).

@bosilca bosilca merged commit 4f40be6 into open-mpi:main May 13, 2025
15 checks passed
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.

4 participants