Skip to content

JIRA ID : LCOMPILER-876 : Remove calls to hipconfig in all of rocm-libraries: read ROCm version from .info/version - #7235

Merged
david-salinas merged 35 commits into
developfrom
users/dsalinas/deprecate-hipcc
Oct 14, 2026
Merged

david-salinas merged 35 commits into
developfrom
users/dsalinas/deprecate-hipcc

Conversation

@david-salinas

@david-salinas david-salinas commented May 9, 2026 •

Copy link
Copy Markdown

JIRA ID : LCOMPILER-876

Motivation

hipconfig and hipcc are legacy HIP build tools being deprecated in favor
of amdclang++ and CMake-native HIP. As part of this migration, HIP_PLATFORM
should no longer be used — it was an environment variable consumed by hipcc
to select the AMD or NVIDIA backend, and has no meaning when building directly
with amdclang++. This PR removes hipconfig/hipcc dependencies and
replaces HIP_PLATFORM backend selection with an explicit USE_CUDA CMake
option in the components modified here. Other users in rocm-libraries (MIOpen,
hipDNN, dnn-providers) remain outside this pull request.

Technical Details

  • Shared Tensile and TensileLite replace hipconfig --version with a
    priority chain: ROCM_VERSION env → share/hip/version →
    hip_version.h → .info/version, preferring build-version sources so
    TensileLite patch-field thresholds remain correct.
  • TensileLite: removes hipconfig/hipcc from the validated toolchain
    tuple; updates characterization tests and restores Component.py coverage
    to 99.54%.
  • hipBLAS, hipSOLVER, and hipFFT: replace all HIP_PLATFORM checks with
    USE_CUDA throughout CMake files and clients.
  • hipBLAS: find_package(hip CONFIG) added to CUDA branch so hip::host
    is always defined; rmake.py corrected to pass -DCUDAToolkit_ROOT;
    FindCUDAToolkit imported targets used (requires CMake 3.17 from build(hipblas): require CMake 3.17 (March 2020) for finding util #11988).
  • hipFFT: USE_CUDA check moved before compiler-name detection so the
    explicit option takes effect with any CXX compiler.
  • TensileConfig.cmake: passes ${HIP_VERSION} (uppercase) to match the
    upstream find_package(HIP) variable.

Out of scope / future work:

  • Migrate remaining hipconfig users: MIOpen, hipDNN, dnn-providers
  • hipSOLVER CUDA backend: switch from legacy FindCUDA to FindCUDAToolkit
    (requires bumping cmake_minimum_required to 3.17)
  • Update CUDA CI job drivers to pass -DUSE_CUDA=ON instead of setting
    HIP_PLATFORM=nvidia
  • Python pip ROCm SDK version detection deferred to PR refactor(tensilelite): AIHPBLAS-3989 model ROCm installations lazily #11023

Test Plan

  • Run shared-Tensile and TensileLite unit and lint suites for version
    discovery and toolchain validation.
  • Configure hipBLAS, hipSOLVER, and hipFFT on their AMD paths with
    amdclang++.
  • Configure each CUDA backend with -DUSE_CUDA=ON.

Test Result

  • hipBLAS, hipSOLVER, hipFFT AMD configure with amdclang++: pass ✅
  • TensileLite get_rocm_version, toolchain_component, Validators unit
    tests: pass ✅
  • Shared-Tensile unit suite: pass ✅
  • codecov/patch: 73.81% (target 70.90%) ✅
  • codecov/project/hipBLAS: 90.62% ✅
  • codecov/project/TensileLite-CPP, TensileLite-Unit, hipSOLVER:
    failing at pre-existing levels, not caused by this PR
  • CUDA backend configure (-DUSE_CUDA=ON): expected failure — no CUDA
    toolkit available in local environment; AMD precheckins pass

Submission Checklist

Risk level

Medium (3/5): changes version discovery and backend selection across five
components; AMD precheckins pass; CUDA backends require a CUDA toolkit to
validate fully.

@codecov-commenter

codecov-commenter commented May 9, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Additional details and impacted files
@@             Coverage Diff              @@
##           develop    #7235       +/-   ##
============================================
- Coverage    72.37%   66.00%    -6.37%     
============================================
  Files         2879      691     -2188     
  Lines       478175   202565   -275610     
  Branches     70864    28533    -42331     
============================================
- Hits        346073   133699   -212374     
+ Misses      108153    59996    -48157     
+ Partials     23949     8870    -15079     
Flag Coverage Δ
TensileLite-CPP 49.70% <ø> (ø)
TensileLite-Unit 83.55% <100.00%> (+0.01%) ⬆️
hipBLAS 90.62% <ø> (ø)
hipBLASLt 42.73% <ø> (ø)
hipCUB ?
hipDNN ?
hipFFT ?
hipRAND ?
hipSOLVER 68.96% <ø> (ø)
hipSPARSE ?
rocBLAS 48.21% <ø> (ø)
rocFFT ?
rocRAND ?
rocSOLVER ?
rocSPARSE ?
rocThrust ?
Files with missing lines Coverage Δ
...tensilelite/tensilelite/Common/GlobalParameters.py 95.90% <100.00%> (-0.03%) ⬇️
...ensilelite/tensilelite/TensileCreateLibrary/Run.py 89.10% <100.00%> (ø)
...slt/tensilelite/tensilelite/Toolchain/Component.py 99.43% <100.00%> (+0.21%) ⬆️
...lt/tensilelite/tensilelite/Toolchain/Validators.py 98.92% <ø> (-0.03%) ⬇️

... and 2188 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@david-salinas
david-salinas force-pushed the users/dsalinas/deprecate-hipcc branch 2 times, most recently from a9b4e58 to ed46cb6 Compare May 9, 2026 18:57
@lamb-j
lamb-j marked this pull request as ready for review May 28, 2026 02:11
@lamb-j
lamb-j requested review from a team as code owners May 28, 2026 02:11
@accauble
accauble requested a review from TorreZuk May 28, 2026 20:27
@accauble
accauble marked this pull request as draft June 9, 2026 16:23
@accauble
accauble force-pushed the users/dsalinas/deprecate-hipcc branch from d01bb1e to 3a93b4c Compare June 9, 2026 16:47
@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

This pull request has been inactive for 25 days and will be marked as stale.

If you would like to keep this PR open, please:

  • Add new commits
  • Add a comment explaining why it should remain open

This PR will be automatically closed in 5 days if no further activity occurs.

@github-actions github-actions Bot added the Stale PR has no activity for 25+ days label Jul 5, 2026
@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been automatically closed due to inactivity (30 days with no updates).

If you'd like to continue working on this, feel free to reopen the PR or create a new one.

@github-actions github-actions Bot closed this Jul 11, 2026
@david-salinas david-salinas reopened this Aug 7, 2026
@therock-pr-bot

therock-pr-bot Bot commented Aug 7, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

therock-pr-bot Bot commented Aug 7, 2026 •

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@david-salinas
david-salinas force-pushed the users/dsalinas/deprecate-hipcc branch from 44398db to 90c19fb Compare August 7, 2026 21:57
@github-actions github-actions Bot removed the Stale PR has no activity for 25+ days label Aug 8, 2026
@david-salinas
david-salinas force-pushed the users/dsalinas/deprecate-hipcc branch from 90c19fb to 7cff3e6 Compare August 10, 2026 20:51
david-salinas and others added 27 commits October 13, 2026 10:21
… validateToolchain

Our PR removed HIP_CONFIG as the 5th argument to validateToolchain, reducing
its return tuple from 5 to 4 values. Update test mocks that still returned
5 values to match the new 4-value interface.

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
…avior tests

get_rocm_version() no longer calls hipconfig. Replace the two tests that
verified hipconfig invocation with tests for the new behavior:

- test_get_rocm_version_uses_rocm_version_env: verifies ROCM_VERSION env
  var is read and takes priority
- test_get_rocm_version_reads_info_version_file: verifies .info/version
  file is read from ROCM_PATH, covering both simple (7.1.0) and
  build-suffix (7.1.25424-abc) version string formats

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
hipconfig --platform reads HIP_PLATFORM if set, otherwise auto-detects
by checking if clang++ is runnable — always returning 'amd' for AMD-only
ROCm builds. Since NVIDIA support has been removed, replace all calls
to hipconfig --platform with the literal value 'amd'.

- hipblas/rmake.py: remove 7-line hipconfig detection block, replace
  with single assignment; update --cuda help text
- hipblas/install.sh: replace hipconfig call with 'amd'; update help text
- hipsolver/install.sh: replace hipconfig call with 'amd'

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
get_rocm_version() no longer calls hipconfig so the hipconfig code path
is no longer exercised by the replacement tests. Update the coverage
baseline from 99.19% to 94.63% to reflect the new test coverage.

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
The previous change unconditionally set HIP_PLATFORM='amd', which
overwrote the 'nvidia' value set by the --cuda flag handler above it.
Use else branch so --cuda builds correctly set HIP_PLATFORM='nvidia'.

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
- Component.py, Common.py: remove rocm_sdk (pip) version detection per
  Alex-Vasile's suggestion to leave Python ROCm support to PR #11023,
  which will add it with proper feature-gating infrastructure

- Component.py, Common.py: fix nightly/alpha version string parsing;
  version strings like "10.1.0a20260813" previously caused ValueError
  when int() was called on "0a20260813"; strip pre-release suffix with
  re.match(r'\d+', ...) before converting to int

- install.sh: correct --cuda help text to accurately reflect that the
  flag unconditionally sets HIP_PLATFORM=nvidia (previously the text
  implied HIP_PLATFORM could still be overridden via env var)

- Linux_Install_Guide.rst: remove stale "hipconfig --platform" reference;
  replace with accurate description of the --cuda flag and HIP_PLATFORM
  defaulting to amd

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
When HIP_PLATFORM=nvidia, rmake.py previously set CUDA_BIN_PATH and
relied on hipcc to wrap nvcc. Replace with direct nvcc detection:

- Locate nvcc from --cudapath/bin/nvcc or PATH
- Pass CMAKE_CUDA_COMPILER and CUDA_TOOLKIT_ROOT_DIR to CMake so
  find_package(CUDA) and find_package(CUDAToolkit) resolve without
  any hipcc involvement
- Skip the implicit g++ default for the CXX compiler when targeting
  CUDA (nvcc drives device compilation; letting CMake pick the host
  compiler is more appropriate than forcing g++)

Update install.sh help text and Linux_Install_Guide.rst to reflect
that nvcc is now invoked directly and must be on PATH or reachable
via --cudapath.

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
Remove the HIP_PLATFORM environment variable from the hipBLAS build
entirely and replace it with a first-class CMake boolean option USE_CUDA.
This decouples the CUDA backend selection from hipcc's internal env var
and invokes nvcc directly.

Changes:
- CMakeLists.txt: promote USE_CUDA from deprecated alias to the primary
  CMake option; use find_package(CUDAToolkit) instead of find_package(CUDA)
  for modern imported targets; remove all ENV{HIP_PLATFORM} reads/writes
- library/src/CMakeLists.txt: replace HIP_PLATFORM checks with USE_CUDA;
  link CUDA::cublas (modern CUDAToolkit target) instead of ${CUDA_CUBLAS_LIBRARIES}
- clients/*/CMakeLists.txt, build-options.cmake: replace HIP_PLATFORM
  checks with USE_CUDA / NOT USE_CUDA
- rmake.py: remove os.environ['HIP_PLATFORM'] entirely; use local
  use_cuda_backend flag; pass -DUSE_CUDA=ON to CMake when --cuda given;
  update --cuda help text to remove HIP_PLATFORM reference
- install.sh: replace HIP_PLATFORM env var checks with build_cuda flag;
  remove export HIP_PLATFORM lines; update --cuda help text
- Linux_Install_Guide.rst: remove HIP_PLATFORM references; describe
  USE_CUDA and nvcc-direct approach

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
…vars in clients

rmake.py: use_cuda_backend was defined as a local in main() but
referenced in config_cmd() which is a separate top-level function —
this caused a NameError on every invocation. Replace with args.use_cuda
directly since args is already module-level.

clients/{benchmarks,gtest,samples}/CMakeLists.txt: after switching from
FindCUDA to FindCUDAToolkit, the client files still referenced legacy
${CUDA_INCLUDE_DIRS} and ${CUDA_LIBRARIES} variables which are empty
with FindCUDAToolkit. Replace with CUDA::cublas which carries its own
include directories transitively, so the explicit target_include_directories
calls are no longer needed.

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
Two corrections aligned with #11634:

library/src/CMakeLists.txt:
- Move hip::host to unconditional linkage (PUBLIC on hipblas). The public
  hipBLAS headers expose HIP types so consumers need hip::host regardless
  of which backend was built, not just on the AMD path.
- Change CUDA::cublas from PRIVATE to PUBLIC and add CUDA::cudart PUBLIC.
  The public API exposes cuBLAS and CUDA runtime declarations on the CUDA
  backend, so those usage requirements must propagate to consumers.

clients/{benchmarks,gtest,samples}/CMakeLists.txt:
- Replace CUDA::cublas with hip::host + CUDA::cudart in client targets.
  Clients call the hipBLAS API (which wraps cuBLAS) but don't call cuBLAS
  directly; they need CUDA::cudart for device management and hip::host
  for HIP allocation/device calls, not the full cuBLAS link.

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
…not un-deprecation

Address review feedback from Alex-Vasile: the USE_CUDA option is NOT
un-deprecating the old flag. The previous USE_CUDA set ENV{HIP_PLATFORM}
for hipcc/hipconfig. The new USE_CUDA is a proper CMake option that drives
the CUDA backend via FindCUDAToolkit and nvcc directly, with no hipcc or
hipconfig involvement. Add a comment making this distinction explicit.

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
…mpatibility

FindCUDAToolkit (with CUDA:: imported targets) requires CMake 3.17.
The legacy FindCUDA module has been available since CMake 2.8 and avoids
forcing a cmake_minimum_required bump in this PR.

- CMakeLists.txt: find_package(CUDA) instead of find_package(CUDAToolkit)
- library/src: ${CUDA_CUBLAS_LIBRARIES}/${CUDA_LIBRARIES} instead of
  CUDA::cublas/CUDA::cudart; explicit CUDA_INCLUDE_DIRS needed since
  legacy variables don't propagate includes transitively
- clients/{benchmarks,gtest,samples}: same pattern

Note: FindCUDA is deprecated since CMake 3.10. A follow-on PR can migrate
to FindCUDAToolkit once cmake_minimum_required is raised separately.

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
…g comment

Extends the hipcc/hipconfig deprecation to hipsolver and hipfft:

hipsolver/CMakeLists.txt:
- Replace deprecated USE_CUDA + ENV{HIP_PLATFORM} detection block with a
  proper option(USE_CUDA ...) mirroring the hipblas approach
- Remove hipconfig/HIP_PLATFORM detection (find_package(hip CONFIG) always)
- Replace three if(NOT HIP_PLATFORM STREQUAL nvidia) checks with if(NOT USE_CUDA)

hipsolver/install.sh:
- Remove export HIP_PLATFORM="nvidia"/"amd" — now driven by -DUSE_CUDA=ON
  passed to CMake; drop the stale $HIP_PLATFORM == "nvidia" OR condition

hipfft/CMakeLists.txt:
- Add option(USE_CUDA ...) and replace ENV{HIP_PLATFORM} read with USE_CUDA
  in the BUILD_WITH_COMPILER detection block
- Remove set(HIP_PLATFORM "nvidia"/"amd") — these were set but never consumed
  by any downstream hipfft cmake file

hipblas/rmake.py:
- Fix misleading comment: find_package(CUDA) not find_package(CUDAToolkit)

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
Add a PATH-based fallback in get_rocm_version() (Component.py) and
assignGlobalParameters() (Common.py) so that TheRock CI builds succeed
when ROCm tools are on PATH but ROCM_PATH is not set and /opt/rocm
doesn't exist (e.g. build/core/clr/dist/ layouts).

Priority chain: ROCM_VERSION env → ROCM_PATH/HIP_PATH/.info/version →
/opt/rocm/.info/version → parent.parent of first found amdclang++,
rocm-smi, or amd-smi on PATH.

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
The previous PATH-based fallback walked up from the found executable
to derive the ROCm root and looked for .info/version, but in TheRock
component dist layouts (e.g. build/core/clr/dist/) that file does not
exist. Add a second probe that reads include/hip/hip_version.h and
extracts HIP_VERSION_MAJOR/MINOR/PATCH, which is present in both
standard ROCm installs and TheRock component dists.

Priority chain:
  1. ROCM_VERSION env var
  2. .info/version from ROCM_PATH / HIP_PATH / /opt/rocm
  3a. .info/version from parent.parent of amdclang++/rocm-smi on PATH
  3b. include/hip/hip_version.h from same candidate root

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
…depth

The previous fix used parent.parent (2 levels up) from the found
executable. But in TheRock, amdclang++ lives at dist/lib/llvm/bin/,
making parent.parent = dist/lib/llvm — still too deep to find
include/hip/hip_version.h or .info/version at dist/.

Switch to walking up to 5 levels from the executable's bin directory,
checking .info/version and include/hip/hip_version.h at each ancestor.
This covers both dist/bin/amdclang++ (1 step) and
dist/lib/llvm/bin/amdclang++ (3 steps).

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
The codecov patch coverage for this PR was 21.43% (target 70.34%),
largely because the PATH-based fallback added to get_rocm_version()
(Component.py) and assignGlobalParameters() (Common.py) lacked test
coverage. These code paths are exercised only in TheRock CI builds
where amdclang++ is on PATH but ROCM_PATH is not set and /opt/rocm
does not exist — conditions that the existing env-var and .info/version
tests do not replicate.

Two parametrized test cases are added to each test file to cover the
two representative TheRock filesystem layouts:
  - dist/bin/amdclang++         (1 level up to dist/)
  - dist/lib/llvm/bin/amdclang++ (3 levels up to dist/)

In both cases the test creates a fake executable at the given depth,
places include/hip/hip_version.h at the dist root, monkeypatches
shutil.which to return the fake executable, and verifies that
get_rocm_version() / HipClangVersion resolves to the expected version.

The Common.py test additionally patches Path.read_text to reject all
.info/version reads, forcing the code to fall through to the
hip_version.h branch, since the full assignGlobalParameters() requires
an extensive config beyond what version detection needs.

Also reverts cmake_minimum_required in hipblas/CMakeLists.txt from
3.17 back to 3.5. The 3.17 bump was a copy-paste error from an earlier
draft that used FindCUDAToolkit; the CUDA backend was subsequently
switched to legacy FindCUDA (available since CMake 2.8) precisely to
avoid raising the minimum version. This addresses Alex-Vasile's review
comments (#7, #8) noting the version bump appeared unrelated to the
PR's scope.

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
cmake_minimum_required was already bumped to 3.17 in develop (#11988).
Since FindCUDAToolkit (added in CMake 3.17) is now available, switch the
CUDA backend from legacy find_package(CUDA) to find_package(CUDAToolkit)
and replace the legacy variables with proper imported targets:
  ${CUDA_CUBLAS_LIBRARIES} -> CUDA::cublas
  ${CUDA_LIBRARIES}        -> CUDA::cudart
  ${CUDA_INCLUDE_DIRS}     -> handled automatically via CUDA::cudart

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
…agent review

Addresses findings from the AI assessment at
https://github.com/newling/agent-public-reviews/blob/main/rocm-libraries/review_pr7235_v7.md
(proposed fixes tracked in #12379).

- hipblas/CMakeLists.txt: add find_package(hip) to CUDA branch so hip::host is defined for both backends
- hipblas/rmake.py: replace -DCUDA_TOOLKIT_ROOT_DIR with -DCUDAToolkit_ROOT (correct FindCUDAToolkit hint)
- shared/tensile/TensileConfig.cmake: use ${HIP_VERSION} instead of ${hip_VERSION} to avoid empty ROCM_VERSION
- shared/tensile/test_Common.py: remove unused import tempfile (flake8 F401, fails static-analysis job)
- hipfft/CMakeLists.txt: check USE_CUDA before compiler-name detection so -DUSE_CUDA=ON is never silently ignored
- test_pchaos_Validators_L226_char.py: remove stale hipcc/hipconfig from POSIX_SUPPORTED model; add as rejected witnesses

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
… lint

Three CI failures caused by missing backwards-compatibility for existing
CUDA CI jobs that pass HIP_PLATFORM=nvidia without -DUSE_CUDA=ON:

- hipfft/CMakeLists.txt: also enter HIP-nvcc path when BUILD_WITH_LIB=CUDA
  is set, so the existing CI job (hipcc + BUILD_WITH_LIB=CUDA) does not
  hit the "Detected HIP_COMPILER=clang, but BUILD_WITH_LIB is not ROCM!"
  fatal error
- hipsolver/CMakeLists.txt: promote HIP_PLATFORM=nvidia to USE_CUDA=ON at
  configure time so all downstream if(USE_CUDA) guards route to the NVIDIA
  path; prevents find_package(rocblas) from running on a CUDA-only host
- shared/tensile/test_Common.py: remove now-unused "from pathlib import Path"
  import (Path.read_text monkeypatch was replaced by _DEFAULT_ROCM_ROOT
  redirect); fixes flake8 F401 lint failure

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
The BUILD_WITH_COMPILER="HIP-nvcc" branch wraps all compile and link
flags with -Xcompiler= and -Xlinker= prefixes, which are nvcc-specific
and rejected by g++ and clang++.

The previous condition triggered HIP-nvcc mode based solely on
BUILD_WITH_LIB=CUDA, without checking whether nvcc is actually the
CXX compiler.  This broke the pre-checkin CUDA CI job which passes
CMAKE_CXX_COMPILER=g++ alongside BUILD_WITH_LIB=CUDA.

Restrict HIP-nvcc mode to builds where CMAKE_CXX_COMPILER is actually
nvcc.  Compilers such as g++ that target the CUDA backend link against
cufft directly and do not need nvcc flag wrapping.

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
The previous fix only matched nvcc in the BUILD_WITH_COMPILER=HIP-nvcc
condition, breaking the hipcc+CUDA CI path.  hipcc wraps nvcc when
HIP_PLATFORM=nvidia and legitimately needs -Xcompiler=/-Xlinker=
flag wrapping, so include it alongside nvcc in the check.

  g++   + CUDA → HOST-default  (no -Xcompiler wrapping)
  hipcc + CUDA → HIP-nvcc      (-Xcompiler wrapping, hipcc/nvcc handle it)
  nvcc  + CUDA → HIP-nvcc      (-Xcompiler wrapping)
  clang++ + ROCM → HIP-clang   (no -Xcompiler wrapping)

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
…PR assessment

Changes based on the external review at:
https://github.com/newling/agent-public-reviews/blob/main/rocm-libraries/review_pr7235_v8.md

- hipfft: normalize USE_CUDA and BUILD_WITH_LIB to a single authoritative value
  before compiler detection so all downstream consumers see the correct backend
- hipfft: consolidate find_package(hip CONFIG) into one unconditional call in
  CMakeLists.txt, removing the tangled compiler-conditional HIP discovery spread
  across dependencies.cmake and library/CMakeLists.txt
- hipfft: remove the now-redundant set(BUILD_WITH_LIB "CUDA") inside the HIP-NVCC block
- hipblas: consolidate duplicate find_package(hip CONFIG) calls from both branches
  of if(USE_CUDA) into a single unconditional call
- hipblas: promote CUDA compiler defines from PRIVATE to PUBLIC so consumers
  inherit __HIP_PLATFORM_NVIDIA__ when linking against hipBLAS
- hipblas: export CUDAToolkit as a package dependency and add a consumer-facing
  helper that loads hip::host for the NVIDIA backend with a guard against
  accidentally using an AMD hip::host with a CUDA-built hipBLAS
- hipblas: align CUDAToolkit_ROOT with the nvcc found on PATH by resolving
  symlinks and deriving the root from the actual executable location, preventing
  mismatched compiler/toolkit installs when the default --cudapath has no nvcc
- hipsolver: remove redundant find_package(HIP CONFIG) block in library/CMakeLists.txt
  that called the same package identically in both if and else branches
- hipsolver: forward -DUSE_CUDA=ON to CMake when install.sh --cuda is used directly;
  previously only rmake.py forwarded this flag, leaving direct install.sh invocations
  silently building the AMD backend
- tensile: find HIP in the package scope before add_subdirectory and capture
  HIP_VERSION into Tensile_HIP_VERSION so TensileCreateLibraryFiles passes a
  non-empty ROCM_VERSION to TensileLite; add_subdirectory child scope variables
  do not propagate back, leaving HIP_VERSION empty at call time
- tensile: guard Source/CMakeLists.txt find_package(HIP) with NOT TARGET hip::host
  to avoid a redundant lookup when the parent scope already found it
- tensilelite: replace deprecated hipcc --version subprocess in GlobalParameters.py
  with get_rocm_version() from the unified version resolver
- tensilelite: remove .info/version fallback from Component.py and Common.py; the
  ROCm release file lacks the HIP build number used for feature gating and silently
  produces wrong results when patch-level thresholds are checked
- tensilelite: update characterization tests to use ROCM_VERSION env var instead of
  hipcc stubs, and add a test explicitly rejecting .info/version-only installs
- tensilelite: restore coverage floors for Assembly.py, HelperKernelCache.py, and
  Source.py that were unintentionally lowered; those files are unchanged by this PR

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…st_arch_steppings

The gfx1250-strict tests added in develop mock validateToolchain to
return 5 values (cxx, cc, bundler, asm, hipConfig). Our PR removed the
hipConfig return value, so Run.py now unpacks 4 values. Update both
mock lambdas at lines 1691 and 2900 from 5-tuple to 4-tuple to match.

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
… builds

When building hipfft with nvcc directly (no hipcc wrapper), CMake must
supply the platform defines and HIP headers that hipcc used to inject
automatically. Load find_package(hip CONFIG) unconditionally so hip::host
is available on both backends, and set __HIP_PLATFORM_NVCC__ and
__HIP_PLATFORM_NVIDIA__ for all CUDA builds (not just HOST-DEFAULT).

Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
PR #10524 (malcolmroberts) modernizes hipFFT's CMake by adding HIP as
a first-class CMake language, which is a more comprehensive approach
than our manual compiler-detection rewrite.  Drop our hipFFT changes
to avoid merge conflicts and let the maintainer's PR land cleanly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…overy

The find_package(HIP) call in TensileConfig.cmake ran before
add_subdirectory(Source), causing hip-config-amd.cmake to write
GPU_TARGETS into the CMake cache via amdgpu-arch auto-detection.
This clobbered Tensile's own GPU_TARGETS initialisation, making
CI reject targets like gfx950:sramecc+:xnack-.

Wrap the HIP discovery in a function that saves and restores the
GPU_TARGETS cache entry so Tensile's Source/CMakeLists.txt starts
from a clean slate.

Also remove the duplicate `import pytest` in test_Common.py that
triggered an F811 lint error.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@assistant-librarian

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

@cgmb

cgmb commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

CMake itself uses hipconfig --platform to set the default value of CMAKE_HIP_PLATFORM?

The hipconfig tool is also the method by which CMake finds the HIP compiler. The hipconfig lookup of the HIP_CLANG_PATH is the method by which distributions specify which version of clang is the default compiler for the HIP language. If hipconfig is being deprecated, what is replacing it?

@david-salinas

Copy link
Copy Markdown
Author

CMake itself uses hipconfig --platform to set the default value of CMAKE_HIP_PLATFORM?

The hipconfig tool is also the method by which CMake finds the HIP compiler. The hipconfig lookup of the HIP_CLANG_PATH is the method by which distributions specify which version of clang is the default compiler for the HIP language. If hipconfig is being deprecated, what is replacing it?

Hi @cgmb good question. I know this PR has already been merged but I should answer this. For now we're really just trying to migrate direct uses of hipconfig in favor of builds using the CMake HIP support. We will continue to ship hipcc/hipconfig for sometime. But the cmake HIP support is able to handle the case where hipconfig is not found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants