Skip to content

[stinkytofu] Fix stale-check for stinkytofu sources in rocisa#7197

Open
KKyang wants to merge 2 commits intodevelopfrom
users/kkyang/stale-check-fix
Open

[stinkytofu] Fix stale-check for stinkytofu sources in rocisa#7197
KKyang wants to merge 2 commits intodevelopfrom
users/kkyang/stale-check-fix

Conversation

@KKyang
Copy link
Copy Markdown
Contributor

@KKyang KKyang commented May 8, 2026

Summary

  • Fix broken staleness detection for stinkytofu sources when importing rocisa. The _build_info.py template referenced @STINKYTOFU_DIR@ which was never set — STINKYTOFU_SOURCE_ROOT always resolved to an empty string.
  • Rename STINKYTOFU_TOP_DIRSTINKYTOFU_ROOT_DIR with PARENT_SCOPE so it propagates from stinkytofu's CMake scope to callers (rocisa standalone, hipblaslt subdirectory builds).
  • Fix invoke rocisa for theRock SDK environments by auto-detecting ROCm path (matching stinkytofu's _detect_rocm() pattern).
  • Add ROCISA_INCLUDE_BUILD_INFO cmake option so _build_info.py is installed in editable dev builds but excluded from release wheels.

Developer workflow

First-time setup

source ~/.rocm/bin/activate && rocm-sdk init
cd projects/hipblaslt/tensilelite

# 1. Install rocisa Python package (editable, with stale-check enabled)
invoke rocisa

# 2. Build tensilelite-client
invoke build-client

After editing rocisa or stinkytofu C++ sources

# Re-run invoke rocisa to rebuild _rocisa.so with your changes
invoke rocisa

invoke build-client does not rebuild the rocisa Python module (_rocisa.so). If you edit stinkytofu or rocisa C++ code and only run invoke build-client, the stale-check will now correctly warn you on the next import rocisa:

ImportError: rocisa C++ sources are newer than the built _rocisa.so — bindings are stale.
  Modified: shared/stinkytofu/src/ir/asm/Function.cpp
  Rebuild:  cmake --build <build_dir> --target _rocisa

Test plan

  • invoke rocisa succeeds on theRock SDK (previously failed with find_package(hip) error)
  • import rocisa works after install
  • Touch a stinkytofu source file → import rocisa raises ImportError with rebuild hint
  • _build_info.py has correct STINKYTOFU_SOURCE_ROOT path (was empty before)
  • pip uninstall rocisa cleans up

🤖 Generated with Claude Code

The staleness detection in rocisa's __init__.py was broken for stinkytofu
sources: the _build_info.py template referenced @STINKYTOFU_DIR@ which
was never defined, causing STINKYTOFU_SOURCE_ROOT to resolve to an empty
string.

Fix by renaming STINKYTOFU_TOP_DIR to STINKYTOFU_ROOT_DIR with
PARENT_SCOPE so it propagates from stinkytofu's CMakeLists.txt up to
callers (rocisa standalone build, hipblaslt subdirectory build).

Also fix `invoke rocisa` for theRock SDK environments:
- Add _detect_rocm() to find ROCm via ROCM_PATH env, rocm-sdk CLI, or
  /opt/rocm fallback (matching stinkytofu's pattern)
- Pass ROCM_PATH to CMake so find_package(hip) works outside /opt/rocm
- Pass ROCISA_INCLUDE_BUILD_INFO=ON so _build_info.py is installed in
  editable dev builds (excluded from release wheels)
Replace the old "Rebuilding rocisa after C++ changes" section with a
clear table showing which command to run for each type of change, and
note that invoke build-client does not rebuild the Python module.
@KKyang KKyang requested a review from a team as a code owner May 8, 2026 12:33
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

❌ Your project status has failed because the head coverage (77.83%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #7197      +/-   ##
===========================================
- Coverage    65.22%   65.22%   -0.00%     
===========================================
  Files         2083     2083              
  Lines       323504   323508       +4     
  Branches     42437    42437              
===========================================
  Hits        211003   211003              
- Misses       94953    94957       +4     
  Partials     17548    17548              
Flag Coverage Δ *Carryforward flag
hipBLAS 90.65% <ø> (ø) Carriedforward from 68ec14f
hipBLASLt 39.85% <ø> (-0.01%) ⬇️
hipCUB 82.21% <ø> (ø) Carriedforward from 68ec14f
hipDNN 85.51% <ø> (ø) Carriedforward from 68ec14f
hipFFT 53.90% <ø> (ø) Carriedforward from 68ec14f
hipRAND 76.12% <ø> (ø) Carriedforward from 68ec14f
hipSOLVER 69.24% <ø> (ø) Carriedforward from 68ec14f
hipSPARSE 84.70% <ø> (ø) Carriedforward from 68ec14f
rocBLAS 48.11% <ø> (ø) Carriedforward from 68ec14f
rocFFT 48.15% <ø> (ø) Carriedforward from 68ec14f
rocPRIM 38.99% <ø> (ø) Carriedforward from 68ec14f
rocRAND 57.03% <ø> (ø) Carriedforward from 68ec14f
rocSOLVER 77.83% <ø> (ø) Carriedforward from 68ec14f
rocSPARSE 72.82% <ø> (ø) Carriedforward from 68ec14f

*This pull request uses carry forward flags. Click here to find out more.
see 1 file 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.

Copy link
Copy Markdown
Contributor

@bstefanuk bstefanuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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.

4 participants