-
Notifications
You must be signed in to change notification settings - Fork 502
Axom: Update ROCm root var #2858
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
base: develop
Are you sure you want to change the base?
Axom: Update ROCm root var #2858
Conversation
|
@adrienbernede First of all, thank you for updating the Axom Spack package here. I'm curious if you need to set I haven't tried in awhile but I think other packages failed when I set the Also @bmhan12 @kennyweiss @white238 for awareness |
…hapman39/spack-packages into bugfix/chapman39/axom-update-rocm-path
|
@chapman39 this is moving in the opposite direction than #1402, as discussed in llnl/axom#1726 (comment). It was a conscious change in #2558 (see rocm_path in axom package). I think because this setting is shared in CachedCMakePackage it may not have shown up in your searches, but merging the current PR would in fact result in inconsistent requirements between several packages regarding the My experiments have told me that Spack was able to locate correctly llvm-amdgpu from the rocm prefix. @alecbcs initially required that change to be consistent with spack installed rocm software stack. See #1402 (comment). Paradoxically, your change in #2365 looks good to me (and to @alecbcs) because it removes the |
|
@adrienbernede okay. thanks for the info. I will re-attempt to build Axom without this change, and set the prefix to |
Similar to #2365, this PR updates Axom var
rocm_rootto move a previous directory back fromllvm-amdgpuprefix. The assumption is the prefix is/opt/rocm-6.4.2/llvm.If other's set the
llvm-amdgpuprefix differently than below, maybe we can handle both cases.Also adds unrelated fix to an incorrect version in MFEM from a previous PR of mine #2363 (comment)