hip support for umpire, chai, raja, camp (#19715)

* create HipPackage base class and do some refactoring

* comments and added conflict to raja for openmp with hip

7 files changed, 232 insertions, 43 deletions

diff --git a/lib/spack/spack/build_systems/hip.py b/lib/spack/spack/build_systems/hip.py
new file mode 100644
index 0000000000..da44f1428d
--- /dev/null
+++ b/lib/spack/spack/build_systems/hip.py
@@ -0,0 +1,138 @@
+# Copyright 2013-2020 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+# Troubleshooting advice for +hip builds:
+#
+# 1. When building with clang, go your compilers.yaml,
+#    add an entry for the amd version of clang, as below.
+#    This will ensure that your entire package is compiled/linked
+#    with the same compiler version. If you use a different version of
+#    clang which is linked against a different version of the gcc library,
+#    you will get errors along the lines of:
+#    undefined reference to
+#       `std::__throw_out_of_range_fmt(char const*, ...)@@GLIBCXX_3.4.20'
+#    which is indicative of a mismatch in standard library versions.
+#
+#    in compilers.yaml
+#    - compiler:
+#        spec: clang@amd
+#        paths:
+#          cc: /opt/rocm/llvm/bin/clang
+#          cxx: /opt/rocm/llvm/bin/clang++
+#          f77:
+#          fc:
+#        flags: {}
+#        operating_system: rhel7
+#        target: x86_64
+#        modules: []
+#        environment: {}
+#        extra_rpaths: []
+#
+#
+# 2. hip and its dependencies are currently NOT picked up by spack
+#    automatically, and should therefore be added to packages.yaml by hand:
+#
+#    in packages.yaml:
+#    hip:
+#      externals:
+#      - spec: hip@3.8.20371-d1886b0b
+#        prefix: /opt/rocm/hip
+#        extra_attributes:
+#          compilers:
+#            c: /opt/rocm/llvm/bin/clang++
+#            c++: /opt/rocm/llvm/bin/clang++
+#            hip: /opt/rocm/hip/bin/hipcc
+#      buildable: false
+#    hsa-rocr-dev:
+#      externals:
+#      - spec: hsa-rocr-dev
+#        prefix: /opt/rocm
+#        extra_attributes:
+#          compilers:
+#            c: /opt/rocm/llvm/bin/clang++
+#            cxx: /opt/rocm/llvm/bin/clang++
+#      buildable: false
+#    llvm-amdgpu:
+#      externals:
+#      - spec: llvm-amdgpu
+#        prefix: /opt/rocm/llvm
+#        extra_attributes:
+#          compilers:
+#            c: /opt/rocm/llvm/bin/clang++
+#            cxx: /opt/rocm/llvm/bin/clang++
+#      buildable: false
+#
+# 3. In part 2, DO NOT list the path to hsa as /opt/rocm/hsa ! You want spack
+#    to find hsa in /opt/rocm/include/hsa/hsa.h . The directory of
+#    /opt/rocm/hsa also has an hsa.h file, but it won't be found because spack
+#    does not like its directory structure.
+#
+
+from spack.package import PackageBase
+from spack.directives import depends_on, variant, conflicts
+
+
+class HipPackage(PackageBase):
+    """Auxiliary class which contains HIP variant, dependencies and conflicts
+    and is meant to unify and facilitate its usage. Closely mimics CudaPackage.
+
+    Maintainers: dtaller
+    """
+
+    # https://llvm.org/docs/AMDGPUUsage.html
+    # Possible architectures
+    amdgpu_targets = (
+        'gfx701', 'gfx801', 'gfx802', 'gfx803',
+        'gfx900', 'gfx906', 'gfx908', 'gfx1010',
+        'gfx1011', 'gfx1012', 'none'
+    )
+
+    variant('hip', default=False, description='Enable HIP support')
+
+    # possible amd gpu targets for hip builds
+    variant('amdgpu_target', default='none', values=amdgpu_targets)
+
+    depends_on('llvm-amdgpu', when='+hip')
+    depends_on('hsa-rocr-dev', when='+hip')
+    depends_on('hip', when='+hip')
+
+    # need amd gpu type for hip builds
+    conflicts('amdgpu_target=none', when='+hip')
+
+    # Make sure non-'none' amdgpu_targets cannot be used without +hip
+    for value in amdgpu_targets[:-1]:
+        conflicts('~hip', when='amdgpu_target=' + value)
+
+    # https://github.com/ROCm-Developer-Tools/HIP/blob/master/bin/hipcc
+    # It seems that hip-clang does not (yet?) accept this flag, in which case
+    # we will still need to set the HCC_AMDGPU_TARGET environment flag in the
+    # hip package file. But I will leave this here for future development.
+    @staticmethod
+    def hip_flags(amdgpu_target):
+        return '--amdgpu-target={0}'.format(amdgpu_target)
+
+    # https://llvm.org/docs/AMDGPUUsage.html
+    # Possible architectures (not including 'none' option)
+    @staticmethod
+    def amd_gputargets_list():
+        return (
+            'gfx701', 'gfx801', 'gfx802', 'gfx803',
+            'gfx900', 'gfx906', 'gfx908', 'gfx1010',
+            'gfx1011', 'gfx1012'
+        )
+
+    # HIP version vs Architecture
+
+    # TODO: add a bunch of lines like:
+    # depends_on('hip@:6.0', when='amdgpu_target=gfx701')
+    # to indicate minimum version for each architecture.
+
+    # Compiler conflicts
+
+    # TODO: add conflicts statements along the lines of
+    # arch_platform = ' target=x86_64: platform=linux'
+    # conflicts('%gcc@5:', when='+cuda ^cuda@:7.5' + arch_platform)
+    # conflicts('platform=darwin', when='+cuda ^cuda@11.0.2:')
+    # for hip-related limitations.
diff --git a/lib/spack/spack/pkgkit.py b/lib/spack/spack/pkgkit.py
index 4f25d41dfb..da519f2e16 100644
--- a/lib/spack/spack/pkgkit.py
+++ b/lib/spack/spack/pkgkit.py
@@ -20,6 +20,7 @@ from spack.build_systems.aspell_dict import AspellDictPackage
 from spack.build_systems.autotools import AutotoolsPackage
 from spack.build_systems.cmake import CMakePackage
 from spack.build_systems.cuda import CudaPackage
+from spack.build_systems.hip import HipPackage
 from spack.build_systems.qmake import QMakePackage
 from spack.build_systems.maven import MavenPackage
 from spack.build_systems.scons import SConsPackage
diff --git a/var/spack/repos/builtin/packages/camp/package.py b/var/spack/repos/builtin/packages/camp/package.py
index 67285e9742..0d0f26280c 100644
--- a/var/spack/repos/builtin/packages/camp/package.py
+++ b/var/spack/repos/builtin/packages/camp/package.py
@@ -6,7 +6,7 @@
 from spack import *
 
 
-class Camp(CMakePackage, CudaPackage):
+class Camp(CMakePackage, CudaPackage, HipPackage):
     """
     Compiler agnostic metaprogramming library providing concepts,
     type operations and tuples for C++ and cuda
@@ -22,27 +22,6 @@ class Camp(CMakePackage, CudaPackage):
     depends_on('cmake@3.8:', type='build')
     depends_on('cmake@3.9:', type='build', when="+cuda")
 
-    variant('hip', default=False, description='Enable HIP support')
-
-    # possible amd gpu targets for hip builds
-    # TODO: we should add a hip build system description equivalent to
-    # lib/spack/spack/build_systems/cuda.py, where possible hip amd gpu
-    # architectures are defined in a similar way as for cuda gpu
-    # architectures. In the meantime, require users to define
-    # amd gpu type for hip builds with a variant here.
-    amdgpu_targets = (
-        'gfx701', 'gfx801', 'gfx802', 'gfx803',
-        'gfx900', 'gfx906', 'gfx908', 'gfx1010',
-        'gfx1011', 'gfx1012', 'none'
-    )
-    variant('amdgpu_target', default='none', values=amdgpu_targets)
-
-    depends_on('llvm-amdgpu', when='+hip')
-    depends_on('hip', when='+hip')
-
-    # need amd gpu type for hip builds
-    conflicts('amdgpu_target=none', when='+hip')
-
     def cmake_args(self):
         spec = self.spec
 
@@ -66,7 +45,7 @@ class Camp(CMakePackage, CudaPackage):
             options.extend([
                 '-DENABLE_HIP=ON',
                 '-DHIP_ROOT_DIR={0}'.format(spec['hip'].prefix),
-                '-DHIP_HCC_FLAGS=--amdgpu-target={0}'.format(arch)])
+                '-DHIP_HIPCC_FLAGS=--amdgpu-target={0}'.format(arch)])
         else:
             options.append('-DENABLE_HIP=OFF')
 
diff --git a/var/spack/repos/builtin/packages/chai/package.py b/var/spack/repos/builtin/packages/chai/package.py
index c7bdebaac7..d4fa53071d 100644
--- a/var/spack/repos/builtin/packages/chai/package.py
+++ b/var/spack/repos/builtin/packages/chai/package.py
@@ -6,7 +6,7 @@
 from spack import *
 
 
-class Chai(CMakePackage, CudaPackage):
+class Chai(CMakePackage, CudaPackage, HipPackage):
     """
     Copy-hiding array interface for data migration between memory spaces
     """
@@ -36,6 +36,15 @@ class Chai(CMakePackage, CudaPackage):
     depends_on('umpire+cuda', when="+cuda")
     depends_on('raja+cuda', when="+raja+cuda")
 
+    # variants +hip and amdgpu_targets are not automatically passed to
+    # dependencies, so do it manually.
+    amdgpu_targets = HipPackage.amd_gputargets_list()
+    depends_on('umpire+hip', when='+hip')
+    depends_on('raja+hip', when="+raja+hip")
+    for val in amdgpu_targets:
+        depends_on('umpire amdgpu_target=%s' % val, when='amdgpu_target=%s' % val)
+        depends_on('raja amdgpu_target=%s' % val, when='+raja amdgpu_target=%s' % val)
+
     def cmake_args(self):
         spec = self.spec
 
@@ -54,6 +63,15 @@ class Chai(CMakePackage, CudaPackage):
         else:
             options.append('-DENABLE_CUDA=OFF')
 
+        if '+hip' in spec:
+            arch = self.spec.variants['amdgpu_target'].value
+            options.extend([
+                '-DENABLE_HIP=ON',
+                '-DHIP_ROOT_DIR={0}'.format(spec['hip'].prefix),
+                '-DHIP_HIPCC_FLAGS=--amdgpu-target={0}'.format(arch)])
+        else:
+            options.append('-DENABLE_HIP=OFF')
+
         if '+raja' in spec:
             options.extend(['-DENABLE_RAJA_PLUGIN=ON',
                             '-DRAJA_DIR=' + spec['raja'].prefix])
@@ -64,6 +82,13 @@ class Chai(CMakePackage, CudaPackage):
         options.append('-DENABLE_TESTS={0}'.format(
             'ON' if self.run_tests else 'OFF'))
 
+        # give clear error for conflict between self.run_tests and
+        # benchmarks variant.
+        if not self.run_tests and '+benchmarks' in spec:
+            raise InstallError(
+                'ENABLE_BENCHMARKS requires ENABLE_TESTS to be ON'
+            )
+
         options.append('-DENABLE_BENCHMARKS={0}'.format(
             'ON' if '+benchmarks' in spec else 'OFF'))
 
diff --git a/var/spack/repos/builtin/packages/hip/package.py b/var/spack/repos/builtin/packages/hip/package.py
index 72730c36a7..261b34e936 100644
--- a/var/spack/repos/builtin/packages/hip/package.py
+++ b/var/spack/repos/builtin/packages/hip/package.py
@@ -50,17 +50,25 @@ class Hip(CMakePackage):
     patch('0002-Fix-detection-of-HIP_CLANG_ROOT.patch', when='@3.5.0:')
 
     def setup_run_environment(self, env):
-        env.set('ROCM_PATH', '')
+        # NOTE: DO NOT PUT LOGIC LIKE self.spec[name] in this function!!!!!
+        # It DOES NOT WORK FOR EXTERNAL PACKAGES!!!! See get_rocm_prefix_info
+        rocm_prefixes = self.get_rocm_prefix_info()
+
+        env.set('ROCM_PATH', rocm_prefixes['rocm-path'])
         env.set('HIP_COMPILER', 'clang')
         env.set('HIP_PLATFORM', 'hcc')
-        env.set('HIP_CLANG_PATH', self.spec['llvm-amdgpu'].prefix.bin)
-        env.set('HSA_PATH', self.spec['hsa-rocr-dev'].prefix)
-        env.set('ROCMINFO_PATH', self.spec['rocminfo'].prefix)
-        env.set('HIP_PATH', self.prefix)
-        env.set('DEVICE_LIB_PATH',
-                self.spec['rocm-device-libs'].prefix.amdgcn.bitcode)
+        env.set('HIP_CLANG_PATH', rocm_prefixes['llvm-amdgpu'].bin)
+        env.set('HSA_PATH', rocm_prefixes['hsa-rocr-dev'])
+        env.set('ROCMINFO_PATH', rocm_prefixes['rocminfo'])
+        env.set('DEVICE_LIB_PATH', rocm_prefixes['rocm-device-libs'].lib)
+        env.set('HIP_PATH', rocm_prefixes['rocm-path'])
         env.set('HIPCC_COMPILE_FLAGS_APPEND',
-                '--rocm-path={0}'.format(self.prefix))
+                '--rocm-path={0}'.format(rocm_prefixes['rocm-path']))
+
+        if 'amdgpu_target' in self.spec.variants:
+            arch = self.spec.variants['amdgpu_target'].value
+            if arch != 'none':
+                env.set('HCC_AMDGPU_TARGET', arch)
 
     def setup_dependent_run_environment(self, env, dependent_spec):
         self.setup_run_environment(env)
@@ -85,31 +93,42 @@ class Hip(CMakePackage):
                 raise RuntimeError(msg)
 
             return {
+                'rocm-path': fallback_prefix,
                 'llvm-amdgpu': fallback_prefix.llvm,
                 'hsa-rocr-dev': fallback_prefix.hsa,
                 'rocminfo': fallback_prefix.bin,
                 'rocm-device-libs': fallback_prefix,
             }
         else:
-            return dict((name, self.spec[name].prefix)
-                        for name in ('llvm-amdgpu', 'hsa-rocr-dev', 'rocminfo',
-                                     'rocm-device-libs'))
+            mydict = dict((name, self.spec[name].prefix)
+                          for name in ('llvm-amdgpu', 'hsa-rocr-dev',
+                                       'rocminfo', 'rocm-device-libs'))
+            mydict['rocm-path'] = os.path.dirname(self.spec.prefix)
+            return mydict
 
     def setup_dependent_build_environment(self, env, dependent_spec):
         # Indirection for dependency paths because hip may be an external in
-        # Spack. See block comment on get_rocm_prefix_info
+        # Spack. See block comment on get_rocm_prefix_info .
+
+        # NOTE: DO NOT PUT LOGIC LIKE self.spec[name] in this function!!!!!
+        # It DOES NOT WORK FOR EXTERNAL PACKAGES!!!! See get_rocm_prefix_info
         rocm_prefixes = self.get_rocm_prefix_info()
 
-        env.set('ROCM_PATH', '')
+        env.set('ROCM_PATH', rocm_prefixes['rocm-path'])
         env.set('HIP_COMPILER', 'clang')
         env.set('HIP_PLATFORM', 'hcc')
         env.set('HIP_CLANG_PATH', rocm_prefixes['llvm-amdgpu'].bin)
         env.set('HSA_PATH', rocm_prefixes['hsa-rocr-dev'])
         env.set('ROCMINFO_PATH', rocm_prefixes['rocminfo'])
-        env.set('DEVICE_LIB_PATH',
-                self.spec['rocm-device-libs'].prefix.amdgcn.bitcode)
+        env.set('DEVICE_LIB_PATH', rocm_prefixes['rocm-device-libs'].lib)
+        env.set('HIP_PATH', rocm_prefixes['rocm-path'])
         env.set('HIPCC_COMPILE_FLAGS_APPEND',
-                '--rocm-path={0}'.format(self.prefix))
+                '--rocm-path={0}'.format(rocm_prefixes['rocm-path']))
+
+        if 'amdgpu_target' in dependent_spec.variants:
+            arch = dependent_spec.variants['amdgpu_target'].value
+            if arch != 'none':
+                env.set('HCC_AMDGPU_TARGET', arch)
 
     def setup_dependent_package(self, module, dependent_spec):
         self.spec.hipcc = join_path(self.prefix.bin, 'hipcc')
diff --git a/var/spack/repos/builtin/packages/raja/package.py b/var/spack/repos/builtin/packages/raja/package.py
index 8d1db659cc..7c139640f7 100644
--- a/var/spack/repos/builtin/packages/raja/package.py
+++ b/var/spack/repos/builtin/packages/raja/package.py
@@ -4,7 +4,7 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
 
-class Raja(CMakePackage, CudaPackage):
+class Raja(CMakePackage, CudaPackage, HipPackage):
     """RAJA Parallel Framework."""
 
     homepage = "http://software.llnl.gov/RAJA/"
@@ -33,6 +33,8 @@ class Raja(CMakePackage, CudaPackage):
     variant('examples', default=True, description='Build examples.')
     variant('exercises', default=True, description='Build exercises.')
 
+    conflicts('+openmp', when='+hip')
+
     depends_on('cmake@3.8:', type='build')
     depends_on('cmake@3.9:', when='+cuda', type='build')
 
@@ -54,6 +56,15 @@ class Raja(CMakePackage, CudaPackage):
         else:
             options.append('-DENABLE_CUDA=OFF')
 
+        if '+hip' in spec:
+            arch = self.spec.variants['amdgpu_target'].value
+            options.extend([
+                '-DENABLE_HIP=ON',
+                '-DHIP_ROOT_DIR={0}'.format(spec['hip'].prefix),
+                '-DHIP_HIPCC_FLAGS=--amdgpu-target={0}'.format(arch)])
+        else:
+            options.append('-DENABLE_HIP=OFF')
+
         options.append('-DBUILD_SHARED_LIBS={0}'.format(
             'ON' if '+shared' in spec else 'OFF'))
 
diff --git a/var/spack/repos/builtin/packages/umpire/package.py b/var/spack/repos/builtin/packages/umpire/package.py
index 6b39aad5f6..14eed00747 100644
--- a/var/spack/repos/builtin/packages/umpire/package.py
+++ b/var/spack/repos/builtin/packages/umpire/package.py
@@ -7,7 +7,7 @@ import llnl.util.lang as lang
 import llnl.util.tty as tty
 
 
-class Umpire(CMakePackage, CudaPackage):
+class Umpire(CMakePackage, CudaPackage, HipPackage):
     """An application-focused API for memory management on NUMA & GPU
     architectures"""
 
@@ -61,6 +61,14 @@ class Umpire(CMakePackage, CudaPackage):
     depends_on('cmake@3.9:', when='+cuda', type='build')
 
     depends_on('blt', type='build')
+
+    # variants +hip and amdgpu_targets are not automatically passed to
+    # dependencies, so do it manually.
+    depends_on('camp+hip', when='+hip')
+    amdgpu_targets = HipPackage.amd_gputargets_list()
+    for val in amdgpu_targets:
+        depends_on('camp amdgpu_target=%s' % val, when='amdgpu_target=%s' % val)
+
     depends_on('camp')
 
     conflicts('+numa', when='@:0.3.2')
@@ -70,7 +78,6 @@ class Umpire(CMakePackage, CudaPackage):
         spec = self.spec
 
         options = []
-
         options.append("-DBLT_SOURCE_DIR={0}".format(spec['blt'].prefix))
         options.append("-Dcamp_DIR={0}".format(spec['camp'].prefix))
 
@@ -90,6 +97,15 @@ class Umpire(CMakePackage, CudaPackage):
         else:
             options.append('-DENABLE_CUDA=Off')
 
+        if '+hip' in spec:
+            arch = self.spec.variants['amdgpu_target'].value
+            options.extend([
+                '-DENABLE_HIP=ON',
+                '-DHIP_ROOT_DIR={0}'.format(spec['hip'].prefix),
+                '-DHIP_HIPCC_FLAGS=--amdgpu-target={0}'.format(arch)])
+        else:
+            options.append('-DENABLE_HIP=OFF')
+
         options.append('-DENABLE_C={0}'.format(
             'On' if '+c' in spec else 'Off'))