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

Conditionally enable PCI in hwloc #25839

Merged
merged 5 commits into from
Aug 29, 2024
Merged

Conditionally enable PCI in hwloc #25839

merged 5 commits into from
Aug 29, 2024

Conversation

jhh67
Copy link
Contributor

@jhh67 jhh67 commented Aug 28, 2024

Co-locale support for multiple NICs and GPUs requires PCI support in hwloc.
Previously, PCI support was only enabled if CHPL_COMM==ofi, which was tested
in the third-party/hwloc/Makefile. Unfortunately, this meant it wasn't
embedded in the build path, so that changing CHPL_COMM required manually
rebuilding hwloc, which was obviously error-prone. Now PCI enable/disable is
controlled by the CHPL_HWLOC_PCI variable which is embedded in the build
paths for both the runtime and third-party/hwloc, and whose default value is
determined by CHPL_COMM, CHPL_LOCALE_MODEL, and CHPL_GPU.

Signed-off-by: John H. Hartman [email protected]

jhh67 added 2 commits August 28, 2024 14:31
Co-locale support for multiple NICs and GPUs requires PCI support in hwloc.
Previously, PCI support was only enabled if CHPL_COMM==ofi, which was tested
in the third-party/hwloc/Makefile. Unfortunately, this meant it wasn't
embedded in the build path, so that changing CHPL_COMM required manually
rebuilding hwloc, which was obviously error-prone. Now PCI enable/disable is
controlled by the CHPL_HWLOC_PCI variable which is embedded in the build
paths for both the runtime and third-party/hwloc, and whose default value is
determined by CHPL_COMM, CHPL_LOCALE_MODEL, and CHPL_GPU.

Signed-off-by: John H. Hartman <[email protected]>
@jhh67 jhh67 requested a review from jabraham17 August 28, 2024 22:27
Copy link
Member

@jabraham17 jabraham17 left a comment

Choose a reason for hiding this comment

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

Looks good!

Can you add comments in chpl_hwloc_pci explaining what the variable is and why one would want it enabled vs disable (colocale/gpu support, startup costs, etc)

@jhh67 jhh67 merged commit 46d97ed into chapel-lang:main Aug 29, 2024
7 checks passed
@jhh67 jhh67 deleted the hwloc branch August 29, 2024 17:02
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