Axom local Spack package

[adrienbernede]

I would like to update the Axom spack package in https://github.com/spack/spack-packages.

While comparing the local axom package to the upstream one and to other radiuss packages, several questions were raised:

ROCm root dir and path

Since spack/spack-packages#1402, the CachedCMakePackage changed in the following way:

-            rocm_root = os.path.dirname(spec["llvm-amdgpu"].prefix)
+            rocm_root = spec["llvm-amdgpu"].prefix

Meaning that we now expect llvm-amdgpu prefix to be set to the rocm root dir, in particular for external installations.
This was tested in radiuss-spack-configs without trouble.

The Axom package does not override this setting, but adds ROCM_ROOT_DIR. Are you OK with:

• applying the same change to that variable
  https://github.com/LLNL/axom/blob/cf27b02d7ce6ea7c6a2d865e55b3a5f836b4b773/scripts/spack/packages/axom/package.py#L370

  - rocm_root = os.path.dirname(spec["llvm-amdgpu"].prefix) 
  + rocm_root = spec["llvm-amdgpu"].prefix

• changing the spack configs accordingly?
  E.g. https://github.com/LLNL/axom/blob/cf27b02d7ce6ea7c6a2d865e55b3a5f836b4b773/scripts/spack/configs/toss_4_x86_64_ib_cray/spack.yaml#L70

  - prefix: /opt/rocm-6.3.1/llvm
  + prefix: /opt/rocm-6.3.1

Without this change, I think we are going toward problems of divergent expectations regarding the spack configuration files.

llvm-amdgpu rpath

This may already have been discussed, but the local Axom package is setting rpaths specific to certain compilers directly in the spack package. However, there is a mechanism to set this at the compiler definition level. In radiuss-spack-configs, we leverage this to avoid having to repeat the same logic in every package:

It looks like the following logic...

https://github.com/LLNL/axom/blob/cf27b02d7ce6ea7c6a2d865e55b3a5f836b4b773/scripts/spack/packages/axom/package.py#L382

...could be replaced by something in the like of:

https://github.com/LLNL/radiuss-spack-configs/blob/d9937d28dd4f321163eb4620da5ff8013a55add2/toss_4_x86_64_ib_cray/packages.yaml#L88

extra_rpaths: [/opt/rocm-6.1.2/lib, /opt/rocm-6.1.2/llvm/lib]

Thoughts?

Mfreeform flag

I also noticed a section of the package to "Remove unusable -Mfreeform flag injected by spack".

At the same time the flag is added in the local spack configs for cray machines:
https://github.com/LLNL/axom/blob/cf27b02d7ce6ea7c6a2d865e55b3a5f836b4b773/scripts/spack/configs/toss_4_x86_64_ib_cray/spack.yaml#L77

Was it added as a test and never removed? Is this still needed?

[bmhan12]

Mfreeform flag

For context,
This is a hack I made while moving to Spack 1.0.
-Mfreeform is only necessary when building with the llvm-amdgpu "compiler", not with cce "compiler".
Building +rocm causes both specs to depend on llvm-amdgpu, so the -Mfreeform flag gets added to the cce host-config, which is why you see the "Remove unusable..." hack to remove it during host-config generation.
In pre-1.0, this was not an issue because llvm-amdgpu and cce were treated as separate compilers, not packages.

A cleaner and simpler way is to define the fortran flag at the toolchain level, instead of how it's done currently with the flags tied to the llvm-amdgpu and cce packages:

  toolchains:
    rocm_6_4_2:
    - spec: fflags=-Mfreeform
    - spec: '%[email protected]'
      when: '%c'
    - spec: '%[email protected]'
      when: '%cxx'
    - spec: '%[email protected]'
      when: '%fortran'
    - spec: '%[email protected]_6_4_2'
      when: '%mpi'
    rocm_6_3_1:
    - spec: fflags=-Mfreeform
    - spec: '%[email protected]'
      when: '%c'
    - spec: '%[email protected]'
      when: '%cxx'
    - spec: '%[email protected]'
      when: '%fortran'
    - spec: '%[email protected]_6_3_1'
      when: '%mpi'
    cce_20:
      # Flag for lowercase Fortran module names
    - spec: fflags=-ef
    - spec: '%[email protected]'
      when: '%c'
    - spec: '%[email protected]'
      when: '%cxx'
    - spec: '%[email protected]'
      when: '%fortran'
    - spec: '%[email protected]_20'
      when: '%mpi'

The "Remove unusable..." hack snippet can be removed with the above change.

[adrienbernede]

Thank you @bmhan12 for the explanation.

If I understand correctly, asking for +rocm %cce adds a dependency to llvm-amdgpu, and this dependency results in the flag being added to the hostconfig.

In the cached_cmake package, amdclang++ is indeed defined as hip compiler as soon as +rocm is set.
https://github.com/spack/spack-packages/blob/0afece241a7d4f827afccf3f60f812904922a10e/repos/spack_repo/builtin/build_systems/cached_cmake.py#L322

Is that where the issue comes from ? amdclang++ for hip code, needing Mfreeform, while the rest of the code is built with cce and does not need the flag ?

That's still confusing because amdclang++ used in the +rocm case is not a spack wrapper, so I wonder how it would end up with the Mfreefrom flag... especially as you said the flag is added to the hostconfig. I am couldn't find the portion of code that does that.

In the end, I like the toolchain suggestion, I think it makes things clear although I am confused by the above considerations and wonder whether it solves the issue for good.

Would you mind explaining what I missed?

[bmhan12]

Would you mind explaining what I missed?

As I understand it, it comes from the RoCmPackage class that HIP packages inherit from: https://github.com/spack/spack-packages/blob/bc06d4c4a72c7378653583e1bc13ebbf36acd02a/repos/spack_repo/builtin/build_systems/rocm.py#L147-L149 , there's an explicit depends_on(llvm-amdgpu) that occurs.