Hip wrapper

[scheibelp]

Used by spack/spack-packages#4411

Pipelines there fully passed with latest commit 3e2f803 d6cc6b2 2b8ef4e

[scheibelp]

Question: should cxxflags attribute from compiler definitions (in packages.yaml) be added when detected language is HIP? or should there be a separate hipflags entry that adds flags to hip invocations. See https://github.com/spack/compiler-wrapper/blob/main/cc.sh#L789

Also, note that if it were processed like cxxflags: it would not work to add -x hip (because that gets placed after the source file, which is apparently important to amdclang++).

[haampie]

Did you check what CMake does? They implemented it twice, so presumably the second time they learned from mistakes from the past. See https://cmake.org/cmake/help/latest/variable/CMAKE_LANG_FLAGS.html#variable:CMAKE_%3CLANG%3E_FLAGS and various other pages, or sources.

Edit: I did that:

• CMake passes -x hip, it's not extension based; we should do the same, otherwise the compiler wrapper may at some point need compiler+version specific parsing in case the real compiler changes behavior.
• CMAKE_HIP_FLAGS_INIT is dervied from HIPFLAGS
• CXXFLAGS is unrelated to HIPFLAGS.

That makes sense cause it's a different language.

This is the case for ~5 years, so I would say let's do -x hip only and fix upstream packages if needed, or deprecate them if too difficult.

[scheibelp]

Commit ed41d0c depends on spack/spack#52826. This PR can be merged first (hipflags isn't useful until this, spack/spack-packages#4411, and spack/spack#52826 merge)

[haampie]

You should add tests.

[scheibelp]

@haampie thanks for the review. I think this is ready again

[haampie]

See comments, you could adapt

haampie-llms@f79171b
haampie-llms@192a24a

[scheibelp]

Thanks for the review @haampie, I think this is ready again (all comments addressed) - I used the suggested commits nearly as is, except I renamed a couple functions.

I also updated spack/spack-packages#4411 to point to this and it rebuilt everything successfully (currently one pipeline appears to have a non-build-related error)

[haampie]

I think I missed the notifications for this. Generally lgtm, the machine suggests some simplifications, but more importantly the test not running is worth fixing ;p

[scheibelp]

@haampie bad test reference is addressed (and I also followed the suggestion to look for and fail on non-existent test names).

Many of the nits I resolved as suggested (those are collapsed); for 3 of them I preferred what was already there.

This changed cc.sh so I updated the pin in spack/spack-packages#4411 and it ran all pipelines relevant to this change successfully.