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

CachedCMakePackage: remove hardcoded hipcc #43644

Merged
merged 1 commit into from
Apr 13, 2024

Conversation

white238
Copy link
Contributor

This was in multiple packages in a place that had no impact due to cache variables being immutable in CMake. It got moved into a common location but higher in the generated CMake initial cache.

This should not be hardcoded due to wanting to use crayCC, amdclang++, or hipcc as your ROCM compiler.

@spackbot-app spackbot-app bot added build-systems core PR affects Spack core functionality labels Apr 12, 2024
@becker33 becker33 enabled auto-merge (squash) April 12, 2024 22:30
@becker33 becker33 merged commit 6d244b3 into develop Apr 13, 2024
33 checks passed
@becker33 becker33 deleted the bugfix/white238/hipcc_cached_cmake branch April 13, 2024 01:38
@adrienbernede
Copy link
Contributor

adrienbernede commented Apr 18, 2024

@white238 This is causing issues (with mixed %gcc +rocm targets) that were not detected at testing time because changes in CachedCMakePackage are not interpreted as changes in the package themselves.

Now, looking at the history, I think you might have missed a tiny detail about the way it used to be:
E.g, in RAJA it used to look like this:

            entries.insert(0, cmake_cache_path("CMAKE_CXX_COMPILER", spec["hip"].hipcc))

Meaning the CMAKE_CXX_COMPILER was inserted at the top of the file!

@becker33 @trws for reference.

@adrienbernede
Copy link
Contributor

I found that in the commit at the root of my PR:

entries.insert(0, cmake_cache_path("CMAKE_CXX_COMPILER", spec["hip"].hipcc))

@white238
Copy link
Contributor Author

white238 commented Apr 18, 2024

Yea @becker33 and I discussed this directly yesterday. When I made this PR i missread that line as after it was defined (too many moving parts at a time, my bad). It was definitely having an impact on the packages that had it. @becker33 is addressing the pipeline fallout in #43638

tldahlgren pushed a commit to tldahlgren/spack that referenced this pull request Apr 23, 2024
stephenmsachs pushed a commit to stephenmsachs/spack that referenced this pull request Apr 29, 2024
teaguesterling pushed a commit to teaguesterling/spack that referenced this pull request Jun 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build-systems core PR affects Spack core functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants