[WIP] hip-lang support by scheibelp · Pull Request #4411 · spack/spack-packages

scheibelp · 2026-04-20T22:14:18Z

Apply changes from spack/spack#49673 that apply to spack-packages.

Needs

Notes:

HIP language support spack#49673 changed SPACK_TEST_COMMAND=dump-env to dump-var, but that would be harder to change (the tests that use this are in spack-core ) and it didn't look essential
I edited a few cases of requires("%[virtuals=c,cxx] llvm-amdgpu") to include when='%c' to accommodate externals that do not specify a compiler (and in a couple cases I swapped requires for depends_on for the same reason)

Other than that this should be a copy of spack/spack#49673

Apply changes from spack/spack#49673 that apply to spack-packages. Co-authored-by: Peter Scheibel <scheibel1@llnl.gov> Signed-off-by: Peter Josef Scheibel <scheibel1@llnl.gov>

scheibelp · 2026-04-20T23:09:06Z

This will have to update the Spack commit used in .ci/env to use at least a5b4056 (from March 27) to pass the package audit (Currently pinned to a commit from March 20)

haampie · 2026-04-21T08:52:48Z

You have to bump the package api

Signed-off-by: Peter Josef Scheibel <scheibel1@llnl.gov>

…rom spack core Signed-off-by: Peter Josef Scheibel <scheibel1@llnl.gov>

… as of March 26, preceding new commit that is used) Signed-off-by: Peter Josef Scheibel <scheibel1@llnl.gov>

Signed-off-by: Peter Josef Scheibel <scheibel1@llnl.gov>

haampie · 2026-04-22T08:15:46Z

    depends_on("zlib-api", type="link")
    depends_on("z3", type="link")
    depends_on("ncurses", type="link")
-    requires("%[virtuals=c,cxx] llvm-amdgpu", when="%c")


The when=%c can be dropped here and elsewhere, right?

I want to be able to create a comgr external without having to specify %llvm-amdgpu on the spec

alalazo

Besides the comment, we may want to delay merging this, so that the next release of the repository is still readable by Spack v1.1

Signed-off-by: Peter Josef Scheibel <scheibel1@llnl.gov>

haampie · 2026-08-12T10:01:35Z

+    - adios2 +kokkos +rocm
+    - amrex +rocm
+    - arborx +rocm


Why is +rocm added if it's also under all:prefer?

haampie · 2026-08-12T10:06:00Z

+        Returns the path to libclang_rt.builtins-x86_64.a by running
+        amdclang++ --print-resource-dir and searching for the library.
+        """
+        import llnl.util.filesystem as fs


You should not import anything besides spack.package. This modules doesn't even exist. Why did that not break for you?

Why do you hard-code an architecture? Looks wrong/unnecessary.

haampie · 2026-08-12T10:08:04Z

        "7.13.0",
    ]:
-        # llvm libs are linked statically, so this *could* be a build dep
+        # in reality this should be a link type dep, but libLLVM is linked statically,


llvm-amdgpu is a link dep here, the rephrased comment suggests it's not. Better to leave the comment as it was.

haampie · 2026-08-12T10:23:55Z

    c_names = ["amdclang"]
    cxx_names = ["amdclang++"]
    fortran_names = ["amdflang"]
+    hip_lang_names = ["amdclang++"]


This shadows cxx_names, so you end up with compilers detected for the languages c, fortran, hip-lang, but cxx will be missing.

haampie · 2026-08-12T10:26:14Z

+    def hip(self) -> Optional[str]:
+        assert self.spec.concrete, "cannot retrieve HIP compiler, spec is not concrete"
+        if self.spec.external:
+            return self.spec.extra_attributes["compilers"].get("hip", None)


this is "hip" but in detection it's "hip-lang".

Also, extra_attributes["compilers"] is used here which is not consistent with the defensive extra_attributes.get("compiler", {}) elsewhere. I think the defensive version is better.

haampie · 2026-08-12T10:35:40Z

            return "."

    def setup_build_environment(self, env: EnvironmentModifications) -> None:
-        env.set("CXX", self.spec["hip"].hipcc)


Inconsistent with rocblas/rocsolver, why remove this only here?

haampie · 2026-08-12T10:46:00Z

        return None

+    @property
+    def hip(self) -> Optional[str]:


Is it useful to add this and produce str | None? It looks brittle. In particular I think you now run in a potential TypeError in compiler-wrapper when it is None in the _var_list.append(("hip-lang", "hip", "HIPCXX", "SPACK_HIPCXX")) bit.

In the GCC package we have the invariant that if languages=c then c is provided. Here you have something like hip is maybe provided. Would be good to make the None state impossible.

haampie · 2026-08-12T10:48:26Z

Can you extract changes that are not related to hip-lang as a virtual? That makes reverts easier.

… deal with possible shadowing from determine_compiler_paths or omission from LlvmDetection.determine_variants

…ld work more-generally

* extract legion changes for rocm build from #4411 * this issue has been fixed for legion after 26.06

adamjstewart · 2026-09-02T10:12:40Z

+    needs: [hip-compiler]
+  - group: hip-compiler
+    specs:
+    - llvm-amdgpu


What is this?

spackbot-triage Bot added dependencies Modifications with a `depends_on()` directive virtual-dependencies Modifications to virtual package dependencies update-package Modifications to packages in the repository labels Apr 20, 2026

spackbot-triage Bot requested review from afzpatel, haampie, renjithravindrankannath and srekolam April 20, 2026 22:14

hip-lang support

215bd48

Apply changes from spack/spack#49673 that apply to spack-packages. Co-authored-by: Peter Scheibel <scheibel1@llnl.gov> Signed-off-by: Peter Josef Scheibel <scheibel1@llnl.gov>

scheibelp force-pushed the hip-lang branch from 7f1e261 to 215bd48 Compare April 20, 2026 22:16

scheibelp added 2 commits April 21, 2026 09:58

this is a change in package API: update version

4fc37c2

Signed-off-by: Peter Josef Scheibel <scheibel1@llnl.gov>

update spack checkout version to include necessary hip-lang support f…

b56eae9

…rom spack core Signed-off-by: Peter Josef Scheibel <scheibel1@llnl.gov>

scheibelp requested a review from a team as a code owner April 21, 2026 18:28

scheibelp added the don't-merge-yet label Apr 21, 2026

error output is different for new installer (new installer is default…

ecbc782

… as of March 26, preceding new commit that is used) Signed-off-by: Peter Josef Scheibel <scheibel1@llnl.gov>

scheibelp requested review from alalazo, becker33, johnwparent and tldahlgren as code owners April 21, 2026 19:08

spackbot-triage Bot added the tests General test capability(ies) label Apr 21, 2026

style error

5c2a130

Signed-off-by: Peter Josef Scheibel <scheibel1@llnl.gov>

haampie reviewed Apr 21, 2026

View reviewed changes

Comment thread repos/spack_repo/builtin/build_systems/rocm.py Outdated

use newer virtual= syntax

ab2d917

Signed-off-by: Peter Josef Scheibel <scheibel1@llnl.gov>

spackbot-triage Bot requested a review from haampie April 21, 2026 22:51

haampie reviewed Apr 22, 2026

View reviewed changes

alalazo requested changes Apr 29, 2026

View reviewed changes

Comment thread repos/spack_repo/builtin/repo.yaml Outdated

version is updating to 2.5

e63e5ac

Signed-off-by: Peter Josef Scheibel <scheibel1@llnl.gov>

spackbot-triage Bot requested a review from haampie April 29, 2026 17:31

scheibelp mentioned this pull request Apr 29, 2026

updates to aotriton , composable-kernel, miopen-hip packages. #4205

Merged

scheibelp mentioned this pull request Aug 3, 2026

Allow compilers to define hipflags: or specs to add hipflags= spack/spack#52826

Open

haampie reviewed Aug 12, 2026

View reviewed changes

Comment thread repos/spack_repo/builtin/packages/legion/package.py Outdated

haampie reviewed Aug 12, 2026

View reviewed changes

scheibelp added 4 commits August 12, 2026 12:36

hip_lang_names -> hip_names; update llvm-amdgpu determine_variants to…

818215f

… deal with possible shadowing from determine_compiler_paths or omission from LlvmDetection.determine_variants

import moved, but has been available since 1.2.1 as a global

d9ae9e8

update comment

6908cc4

replace arch-specific clang_rt lib filtering with something that shou…

04519df

…ld work more-generally

spackbot-triage Bot requested a review from haampie August 13, 2026 02:16

scheibelp added 7 commits August 13, 2026 13:27

rocblas/rocsolver follow rocfft

933c35d

refactor legion parallel for rocm (will extract)

b77e4e8

Merge branch 'develop' into hip-lang

a36232c

explicitly build legion+rocm

0b23cc9

Merge branch 'develop' into hip-lang

d0d2704

rocblas/rocsolver should use amdclang++

11a2a03

set ROCM_PATH for realm

9c1ba50

scheibelp mentioned this pull request Aug 17, 2026

legion: disable parallel build for +rocm #6121

Merged

point to latest commit of compiler wrapper

c973eda

rbberger pushed a commit that referenced this pull request Aug 19, 2026

legion: disable parallel build for +rocm (#6121)

c61cfa7

* extract legion changes for rocm build from #4411 * this issue has been fixed for legion after 26.06

scheibelp added 3 commits September 1, 2026 11:23

merge from develop (minor conflict in legion)

290f35d

point to newest compiler wrapper update

3963d7f

spec group needs llvm-amdgpu pre-concretized

48fed55

scheibelp requested a review from adamjstewart as a code owner September 2, 2026 07:20

adamjstewart reviewed Sep 2, 2026

View reviewed changes

Uh oh!

Conversation

scheibelp commented Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

scheibelp commented Apr 20, 2026

Uh oh!

haampie commented Apr 21, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alalazo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haampie Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haampie commented Aug 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

scheibelp commented Apr 20, 2026 •

edited

Loading

haampie Aug 12, 2026 •

edited

Loading