Skip to content

Set compiler versions in context#755

Merged
rapids-bot[bot] merged 1 commit intorapidsai:branch-25.08from
jakirkham:set_comp_vers_ctx
Jun 17, 2025
Merged

Set compiler versions in context#755
rapids-bot[bot] merged 1 commit intorapidsai:branch-25.08from
jakirkham:set_comp_vers_ctx

Conversation

@jakirkham
Copy link
Member

Simplify the logic necessary to handle compiler versions by folding it into the context.

Simplify the logic necessary to handle compiler versions by folding it
into the context.
@jakirkham jakirkham requested review from a team as code owners June 13, 2025 19:01
@jakirkham jakirkham requested a review from KyleFromNVIDIA June 13, 2025 19:01
@jakirkham jakirkham added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Jun 13, 2025
Copy link
Member Author

@jakirkham jakirkham left a comment

Choose a reason for hiding this comment

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

Gave a rough overview of the relevant changes below

Generally the idea is push more things into context where it can be written compactly. Keeping only the relevant differentiator (CUDA version) in the variant file

Comment on lines +29 to +31
cat > variants.yaml << EOF
cuda_version:
- ${RAPIDS_CUDA_VERSION}
Copy link
Member Author

Choose a reason for hiding this comment

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

This writes out the common variant file content for all cases

Comment on lines +33 to +35
if [[ "$(arch)" == "aarch64" ]]; then
cat >> variants.yaml << EOF
- 12.1 # The last version to not support cufile
Copy link
Member Author

Choose a reason for hiding this comment

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

Then the legacy CUDA ARM builds are appended to the file

This also makes it easier to drop this section of code when it no longer applies

Comment on lines 12 to +14
should_use_cufile: ${{ x86_64 or (aarch64 and cuda_version >= "12.2") }}
c_compiler_version: ${{ 13 if should_use_cufile else 12 }}
cxx_compiler_version: ${{ c_compiler_version }}
Copy link
Member Author

Choose a reason for hiding this comment

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

Instead of setting different compiler versions in the CUDA variant file, we just set them here in the context. Also this ties them to the relevant thing. Namely whether we can build with cuFile or not

We can see this works as intended with the CUDA 12.1 ARM build

 │ │ │ cuda-version                     ┆ 12.1         ┆ h1d6eff3_3       ┆ conda-forge ┆  20.56 KiB │
 │ │ │ gcc_impl_linux-aarch64           ┆ 12.4.0       ┆ h628656a_2       ┆ conda-forge ┆  56.19 MiB │
 │ │ │ gcc_linux-aarch64                ┆ 12.4.0       ┆ heb3b579_10      ┆ conda-forge ┆  31.88 KiB │
 │ │ │ gxx_impl_linux-aarch64           ┆ 12.4.0       ┆ h0bf7a72_2       ┆ conda-forge ┆  11.36 MiB │
 │ │ │ gxx_linux-aarch64                ┆ 12.4.0       ┆ h3f57e68_10      ┆ conda-forge ┆  30.23 KiB │

Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

This approach seems sensible to me John, thanks for the cleanup!

@vyasr
Copy link
Contributor

vyasr commented Jun 17, 2025

/merge

@rapids-bot rapids-bot bot merged commit 5918cf3 into rapidsai:branch-25.08 Jun 17, 2025
51 checks passed
@jakirkham jakirkham deleted the set_comp_vers_ctx branch June 17, 2025 19:25
@jakirkham
Copy link
Member Author

Happy to help :)

vyasr added a commit to vyasr/kvikio that referenced this pull request Jul 25, 2025
rapids-bot bot pushed a commit that referenced this pull request Jul 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improves an existing functionality non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments