sirius: inherit from ROCmPackage#31207
sirius: inherit from ROCmPackage#31207alalazo merged 3 commits intospack:developfrom albestro:alby/sirius_use_rocmpackage
Conversation
|
We can merge it as-is and then test on our build farm. @AdhocMan do you need to introduce similar changes to SpFFT and SPLA? |
I just gave a look: |
simonpintarelli
left a comment
There was a problem hiding this comment.
Thank you @albestro. It compiled successfully for me.
AdhocMan
left a comment
There was a problem hiding this comment.
If None is a possible value of amdgpu_target, then this is currently not properly handled, because the values will be used to define HIP_HCC_FLAGS and I think --amdgpu-target=None is not a valid flag. So I'd suggest to just not define this CMake argument it in this case, which will let SIRIUS make the choice.
SPLA does not have any GPU kernels, so it should not require these changes. But yes, SpFFT should be similar to SIRIUS and could benefit from it. |
@AdhocMan, I've just checked and it is already managed in spack/lib/spack/spack/build_systems/rocm.py Lines 112 to 113 in 35d07a6 |
AdhocMan
left a comment
There was a problem hiding this comment.
@AdhocMan, I've just checked and it is already managed in
ROCmPackage😉spack/lib/spack/spack/build_systems/rocm.py
Lines 112 to 113 in 35d07a6
Great, this looks good to me then.
|
Can we merge this? |
|
There's a conflict to be solved before merging |
|
sorry, I didn't see the conflict after the merge. I just solved the conflicts, can @simonpintarelli have a look if everything is correct? thanks! |
|
Thank you @albestro, this looks good to me! |
I admit I didn't test this...so I would kindly ask maintainers who may have a ready build environment for
siriusif they can check it works correctly...sorry for that.It is not expected to change (much) the overall behavior: ROCmPackage privides
hipdependency, together with the variantsrocmandamdgpu_target(with a centralised set of existing architectures).For what concerns
amdgpu_targetvariant:nonesee any_combination_ofThese changes may requires some additional fix either in the package itself (is none supported by the CMake logic?) and/or in the user workflow (no defaults, may require the user to specify actual architecture).
Looking forward for your reviews.