Skip to content

[stinkytofu] Preserve MUBUF scope modifiers through StinkyTofu lowering#6852

Merged
ThanHenderson merged 12 commits into
developfrom
users/nhenders/stinky-SCOPE
May 1, 2026
Merged

[stinkytofu] Preserve MUBUF scope modifiers through StinkyTofu lowering#6852
ThanHenderson merged 12 commits into
developfrom
users/nhenders/stinky-SCOPE

Conversation

@ThanHenderson
Copy link
Copy Markdown
Contributor

@ThanHenderson ThanHenderson commented Apr 27, 2026

Motivation

Scope-qualified MUBUF buffer ops (e.g. scope:SCOPE_DEV) were silently dropped during rocisa → StinkyTofu lowering, breaking cache-coherence semantics on targets that require explicit scope modifiers.

Technical Details

Carry rocisa MUBUF scope metadata through StinkyTofu conversion, IR serialization, and assembly emission so scope qualified buffer operations survive lowering.

  • Add MUBUFScope scope to stinkytofu::MUBUFModifiers.
  • convertMUBUFModifiers forwards rocisa::CacheScope as a stinkytofu::MUBUFScope.
  • IR serializer writes/reads scope = "..." on mod.mubuf.
  • Asm emitter prints scope:<value> alongside existing glc/sc1/nt.

Test Plan

  • rocisa pytest: lower BufferStore/LoadB32 with CacheScope.SCOPE_DEV and check final asm.
  • StinkyToFu FileCheck: verify scope survives IR round-trip.
  • C++ unit test: drive StinkyAsmEmitter directly and assert exact output.

Test Result

All three suites pass; emitted asm contains scope:SCOPE_DEV for both load and store.

Carry rocisa MUBUF scope metadata through StinkyTofu conversion, IR
serialization, and assembly emission so scope-qualified buffer
operations survive lowering.
@jaopaulolc jaopaulolc requested review from cycheng and hcman2 April 27, 2026 17:16
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 27, 2026

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    #6852      +/-   ##
===========================================
- Coverage    67.40%   67.39%   -0.00%     
===========================================
  Files         1953     1953              
  Lines       301851   301855       +4     
  Branches     42289    42289              
===========================================
  Hits        203434   203434              
- Misses       80941    80945       +4     
  Partials     17476    17476              
Flag Coverage Δ *Carryforward flag
hipBLAS 90.65% <ø> (ø) Carriedforward from 676fad7
hipBLASLt 39.74% <ø> (-0.01%) ⬇️
hipCUB 82.21% <ø> (ø) Carriedforward from 676fad7
hipDNN 79.96% <ø> (ø) Carriedforward from 676fad7
hipFFT 56.05% <ø> (ø) Carriedforward from 676fad7
hipRAND 76.12% <ø> (ø) Carriedforward from 676fad7
hipSOLVER 69.00% <ø> (ø) Carriedforward from 676fad7
hipSPARSE 84.70% <ø> (ø) Carriedforward from 676fad7
rocBLAS 48.11% <ø> (ø) Carriedforward from 676fad7
rocFFT 52.59% <ø> (ø) Carriedforward from 676fad7
rocRAND 57.11% <ø> (ø) Carriedforward from 676fad7
rocSOLVER 77.83% <ø> (ø) Carriedforward from 676fad7
rocSPARSE 72.81% <ø> (ø) Carriedforward from 676fad7

*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

@cycheng cycheng left a comment

Choose a reason for hiding this comment

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

LGTM.

Comment thread shared/stinkytofu/include/stinkytofu/ir/asm/StinkyModifiers.hpp Outdated
Copy link
Copy Markdown
Contributor

@KKyang KKyang left a comment

Choose a reason for hiding this comment

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

String is too hacky. Please change to enum.

@jaopaulolc
Copy link
Copy Markdown
Contributor

String is too hacky. Please change to enum.

Thanks for the suggestion. Done in e79e255

@jaopaulolc jaopaulolc requested a review from cycheng April 27, 2026 23:44
@jaopaulolc jaopaulolc enabled auto-merge (squash) April 28, 2026 20:22
@ThanHenderson ThanHenderson disabled auto-merge April 28, 2026 23:06
Ensure real-vaddr MUBUF instructions keep an address modifier when literal
zero soffsets are lowered to null, while preserving the off-vaddr form.
@ThanHenderson ThanHenderson requested review from KKyang and cycheng April 29, 2026 21:23
Copy link
Copy Markdown
Contributor

@cycheng cycheng left a comment

Choose a reason for hiding this comment

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

In general LGTM with a small notes :)

Comment thread shared/stinkytofu/include/stinkytofu/ir/asm/StinkyModifiers.hpp Outdated
@ThanHenderson ThanHenderson enabled auto-merge (squash) April 30, 2026 15:24
@ThanHenderson ThanHenderson merged commit 74d1d6d into develop May 1, 2026
38 checks passed
@ThanHenderson ThanHenderson deleted the users/nhenders/stinky-SCOPE branch May 1, 2026 02:32
aledudek pushed a commit that referenced this pull request May 20, 2026
…ng (#6852)

## Motivation
Scope-qualified MUBUF buffer ops (e.g. `scope:SCOPE_DEV`) were silently
dropped during rocisa → StinkyTofu lowering, breaking cache-coherence
semantics on targets that require explicit scope modifiers.
## Technical Details
Carry rocisa MUBUF scope metadata through StinkyTofu conversion, IR
serialization, and assembly emission so scope qualified buffer
operations survive lowering.
- Add `MUBUFScope scope` to `stinkytofu::MUBUFModifiers`.
- `convertMUBUFModifiers` forwards `rocisa::CacheScope` as a
`stinkytofu::MUBUFScope`.
- IR serializer writes/reads `scope = "..."` on `mod.mubuf`.
- Asm emitter prints ` scope:<value>` alongside existing
`glc`/`sc1`/`nt`.
## Test Plan
- rocisa pytest: lower `BufferStore/LoadB32` with `CacheScope.SCOPE_DEV`
and check final asm.
- StinkyToFu FileCheck: verify `scope` survives IR round-trip.
- C++ unit test: drive `StinkyAsmEmitter` directly and assert exact
output.
## Test Result
All three suites pass; emitted asm contains `scope:SCOPE_DEV` for both
load and store.

---------

Co-authored-by: Joao P. L. de Carvalho <joao.carvalho@amd.com>
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.

6 participants