Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For certain
HIP-enabled builds (in my case, I have been testing this on MI300x GPUs),SuperLU_DIST'sCMakesystem fails to correctly findROCmand as a result one gets a flurry of compiler errors as some kernel objects (threadIdx_x,blockIdx_x,__syncthreadsetc) don't get correctly substituted.In this PR, the
HIPbuild path inSuperLU_DISTwas updated to support modernROCm(≥ ~4.5) andCMake(≥ 3.21). However, I have used branching to maintain the older path in order to keep backwards compatibility.CMake's first-classHIPlanguage support (find_package(hip CONFIG),enable_language(HIP),add_library()withLANGUAGE HIP). GPU architecture is selected via the standard-DCMAKE_HIP_ARCHITECTURES=<gfxNNN>variable. Linkship::devicealongsideroc::hipblas,roc::rocblas, androc::rocsolver.FindHIP MODULE+hip_add_library()path is preserved behind a new option-DTPL_HIP_USE_LEGACY_FINDHIP=ON. IfCMake< 3.21 is detected, this path is activated automatically. In both cases a deprecation warning is emitted at configure time.What changed
CMakeLists.txt: ForCMake< 3.21 there is an auto-fallback to the legacyFindHIPpath plus a deprecation warning. AddedTPL_HIP_USE_LEGACY_FINDHIPoption to let users opt in explicitly. Branched theHIP_LIBtarget list andHIP_HIPCC_FLAGShandling by path.SRC/CMakeLists.txt: Branched the library creation betweenhip_add_library()(legacy) andadd_library()+set_source_files_properties(... LANGUAGE HIP)(modern).From the user's perspective, the only difference with the new path is that the way to let the build system know about the HIP target architecture is by using the option
-DCMAKE_HIP_ARCHITECTURES=gfxNNNinstead of-DHIP_HIPCC_FLAGS="--offload-arch=gfxNNN". In the modern path, the latter will be silently ignored.