From 4a9c8ec1ad3c777371f4f82f00537b320b93037d Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Wed, 24 Feb 2021 10:37:16 +0100 Subject: [PATCH] HIP: document variables, apply patches to older versions, fix build issues (#21852) Since there are many variables being set I thought it would be a good idea to document them better and slightly simplify the logic for external vs not-external. --- ...-compilation-without-git-repo.3.7.0.patch} | 0 ...-builtins-linking-on-hip-host.3.7.0.patch} | 0 .../repos/builtin/packages/hip/package.py | 150 +++++++++++------- 3 files changed, 94 insertions(+), 56 deletions(-) rename var/spack/repos/builtin/packages/hip/{0003-Improve-compilation-without-git-repo.3.9.0.patch => 0003-Improve-compilation-without-git-repo.3.7.0.patch} (100%) rename var/spack/repos/builtin/packages/hip/{0004-Drop-clang-rt-builtins-linking-on-hip-host.3.9.0.patch => 0004-Drop-clang-rt-builtins-linking-on-hip-host.3.7.0.patch} (100%) diff --git a/var/spack/repos/builtin/packages/hip/0003-Improve-compilation-without-git-repo.3.9.0.patch b/var/spack/repos/builtin/packages/hip/0003-Improve-compilation-without-git-repo.3.7.0.patch similarity index 100% rename from var/spack/repos/builtin/packages/hip/0003-Improve-compilation-without-git-repo.3.9.0.patch rename to var/spack/repos/builtin/packages/hip/0003-Improve-compilation-without-git-repo.3.7.0.patch diff --git a/var/spack/repos/builtin/packages/hip/0004-Drop-clang-rt-builtins-linking-on-hip-host.3.9.0.patch b/var/spack/repos/builtin/packages/hip/0004-Drop-clang-rt-builtins-linking-on-hip-host.3.7.0.patch similarity index 100% rename from var/spack/repos/builtin/packages/hip/0004-Drop-clang-rt-builtins-linking-on-hip-host.3.9.0.patch rename to var/spack/repos/builtin/packages/hip/0004-Drop-clang-rt-builtins-linking-on-hip-host.3.7.0.patch diff --git a/var/spack/repos/builtin/packages/hip/package.py b/var/spack/repos/builtin/packages/hip/package.py index ac992785dc..c0fb028d7b 100644 --- a/var/spack/repos/builtin/packages/hip/package.py +++ b/var/spack/repos/builtin/packages/hip/package.py @@ -37,7 +37,8 @@ class Hip(CMakePackage): depends_on('rocm-device-libs@' + ver, type=('build', 'link', 'run'), when='@' + ver) depends_on('rocminfo@' + ver, type=('build', 'run'), when='@' + ver) - # Notice: most likely this will only be a hard dependency on 3.7.0 + # hipcc likes to add `-lnuma` by default :( + # ref https://github.com/ROCm-Developer-Tools/HIP/pull/2202 depends_on('numactl', when='@3.7.0:') # Note: the ROCm ecosystem expects `lib/` and `bin/` folders with symlinks @@ -52,74 +53,107 @@ class Hip(CMakePackage): patch('0002-Fix-detection-of-HIP_CLANG_ROOT.patch', when='@:3.9.0') # See https://github.com/ROCm-Developer-Tools/HIP/pull/2218 - patch('0003-Improve-compilation-without-git-repo.3.9.0.patch', when='@3.9.0') + patch('0003-Improve-compilation-without-git-repo.3.7.0.patch', when='@3.7.0:3.9.0') patch('0003-Improve-compilation-without-git-repo.3.10.0.patch', when='@3.10.0:4.0.0') # See https://github.com/ROCm-Developer-Tools/HIP/pull/2219 - patch('0004-Drop-clang-rt-builtins-linking-on-hip-host.3.9.0.patch', when='@3.9.0') + patch('0004-Drop-clang-rt-builtins-linking-on-hip-host.3.7.0.patch', when='@3.7.0:3.9.0') patch('0004-Drop-clang-rt-builtins-linking-on-hip-host.3.10.0.patch', when='@3.10.0:4.0.0') - def get_rocm_prefix_info(self): - # External packages in Spack do not currently contain dependency - # information. External installations of hip therefore must compute - # necessary paths to other rocm components by relative paths. This - # assumes all components are installed under a single umbrella - # directory. Manual edits to `fallback_path` may be necessary if this - # assumption does not hold. + def get_paths(self): if self.spec.external: - # typically, self.spec.prefix is /opt/rocm/hip, so fallback_path - # will be /opt/rocm. The rocminfo executable is usually - # found at /opt/rocm/bin/rocminfo. - fallback_prefix = Prefix(os.path.dirname(self.spec.prefix)) - if not os.path.isdir(fallback_prefix): + # For external packages we only assume the `hip` prefix is known, + # because spack does not set prefixes of dependencies of externals. + # We assume self.spec.prefix is /opt/rocm-x.y.z/hip and rocm has a + # default installation with everything installed under + # /opt/rocm-x.y.z + rocm_prefix = Prefix(os.path.dirname(self.spec.prefix)) + + if not os.path.isdir(rocm_prefix): msg = "Could not determine prefix for other rocm components\n" msg += "Either report a bug at github.com/spack/spack or " - msg += "manually edit fallback_prefix in the package file as " + msg += "manually edit rocm_prefix in the package file as " msg += "a workaround." 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.lib, - 'device_lib_path': fallback_prefix.lib + paths = { + 'rocm-path': rocm_prefix, + 'llvm-amdgpu': rocm_prefix.llvm, + 'hsa-rocr-dev': rocm_prefix.hsa, + 'rocminfo': rocm_prefix, + 'rocm-device-libs': rocm_prefix } else: - mydict = dict((name, self.spec[name].prefix) - for name in ('llvm-amdgpu', 'hsa-rocr-dev', - 'rocminfo', 'rocm-device-libs')) - mydict['rocm-path'] = self.spec.prefix - if '@:3.8.0' in self.spec: - device_lib_path = mydict['rocm-device-libs'].lib - else: - device_lib_path = mydict['rocm-device-libs'].amdgcn.bitcode - mydict['device_lib_path'] = device_lib_path - return mydict + paths = { + 'rocm-path': self.spec.prefix, + 'llvm-amdgpu': self.spec['llvm-amdgpu'].prefix, + 'hsa-rocr-dev': self.spec['hsa-rocr-dev'].prefix, + 'rocminfo': self.spec['rocminfo'].prefix, + 'rocm-device-libs': self.spec['rocm-device-libs'].prefix + } + + # `device_lib_path` is the path to the bitcode directory + if '@:3.8.0' in self.spec: + paths['device_lib_path'] = paths['rocm-device-libs'].lib + else: + paths['device_lib_path'] = paths['rocm-device-libs'].amdgcn.bitcode + + return paths def set_variables(self, env): - # Indirection for dependency paths because hip may be an external in - # Spack. See block comment on get_rocm_prefix_info . + # Note: do not use self.spec[name] here, since not all dependencies + # have defined prefixes when hip is marked as external. + paths = self.get_paths() - # 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() + # Used in hipcc, but only useful when hip is external, since only then + # there is a common prefix /opt/rocm-x.y.z. + env.set('ROCM_PATH', paths['rocm-path']) - env.set('ROCM_PATH', rocm_prefixes['rocm-path']) - env.set('HIP_COMPILER', 'clang') + # hipcc recognizes HIP_PLATFORM == hcc and HIP_COMPILER == clang, even + # though below we specified HIP_PLATFORM=rocclr and HIP_COMPILER=clang + # in the CMake args. 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', rocm_prefixes['device_lib_path']) - env.set('HIP_PATH', rocm_prefixes['rocm-path']) - # this guy is used in comgr, see the following file: + env.set('HIP_COMPILER', 'clang') + + # bin directory where clang++ resides + env.set('HIP_CLANG_PATH', paths['llvm-amdgpu'].bin) + + # Path to hsa-rocr-dev prefix used by hipcc. + env.set('HSA_PATH', paths['hsa-rocr-dev']) + + # This is a variable that does not exist in hipcc but was introduced + # in a patch of ours since 3.5.0 to locate rocm_agent_enumerator: + # https://github.com/ROCm-Developer-Tools/HIP/pull/2138 + env.set('ROCMINFO_PATH', paths['rocminfo']) + + # This one is used in hipcc to run `hipcc --hip-device-lib-path=...` + env.set('DEVICE_LIB_PATH', paths['device_lib_path']) + + # And this is used in clang whenever the --hip-device-lib-path is not + # used (e.g. when clang is invoked directly) + env.set('HIP_DEVICE_LIB_PATH', paths['device_lib_path']) + + # Just the prefix of hip (used in hipcc) + env.set('HIP_PATH', paths['rocm-path']) + + # Used in comgr and seems necessary when using the JIT compiler, e.g. + # hiprtcCreateProgram: # https://github.com/RadeonOpenCompute/ROCm-CompilerSupport/blob/rocm-4.0.0/lib/comgr/src/comgr-env.cpp - # it's necessary on runtime when using hiprtcCreateProgram and such - env.set('LLVM_PATH', rocm_prefixes['llvm-amdgpu']) - env.set('HIPCC_COMPILE_FLAGS_APPEND', - '--rocm-path={0}'.format(rocm_prefixes['device_lib_path'])) + env.set('LLVM_PATH', paths['llvm-amdgpu']) + + # Finally we have to set --rocm-path= ourselves, which is not + # the same as --hip-device-lib-path (set by hipcc). It's used to set + # default bin, include and lib folders in clang. If it's not set it is + # infered from the clang install dir (and they try to find + # /opt/rocm again...). If this path is set, there is no strict checking + # and parsing of the /bin/.hipVersion file. Let's just set this + # to the hip prefix directory for non-external builds so that the + # bin/.hipVersion file can still be parsed. + # See also https://github.com/ROCm-Developer-Tools/HIP/issues/2223 + if '@3.8.0:' in self.spec: + env.append_path('HIPCC_COMPILE_FLAGS_APPEND', + '--rocm-path={0}'.format(paths['rocm-path']), + separator=' ') def setup_run_environment(self, env): self.set_variables(env) @@ -175,11 +209,15 @@ def flag_handler(self, name, flags): def cmake_args(self): args = [ - '-DHIP_COMPILER=clang', - '-DHIP_PLATFORM=rocclr', - '-DHSA_PATH={0}'.format(self.spec['hsa-rocr-dev'].prefix), - '-DHIP_RUNTIME=ROCclr', - '-DLIBROCclr_STATIC_DIR={0}/lib'.format - (self.spec['hip-rocclr'].prefix) + self.define('HIP_COMPILER', 'clang'), + self.define('HIP_PLATFORM', 'rocclr'), + self.define('HSA_PATH', self.spec['hsa-rocr-dev'].prefix), + self.define('HIP_RUNTIME', 'ROCclr'), ] + + # LIBROCclr_STATIC_DIR is unused from 3.6.0 and above + if '@3.5.0' in self.spec: + args.append(self.define('LIBROCclr_STATIC_DIR', + self.spec['hip-rocclr'].prefix.lib)) + return args