Skip to content

[rocroller] Coherently handle wait states for conditionals#4908

Draft
ThanHenderson wants to merge 1 commit intodevelopfrom
users/nhenders/conditionals
Draft

[rocroller] Coherently handle wait states for conditionals#4908
ThanHenderson wants to merge 1 commit intodevelopfrom
users/nhenders/conditionals

Conversation

@ThanHenderson
Copy link
Contributor

@ThanHenderson ThanHenderson commented Feb 26, 2026

Motivation

The WaitcntObserver previously had no awareness of control flow. When processing conditionals, the observer walked through if-body and else-body sequentially, accumulating queue state across both blocks as if they executed together. This caused incorrect waitcnt assertions for forward branches and could lead to missed wait insertions at join points where in-flight operations from one branch path were invisible to the merged state.

Technical Details

  • Forward/backward branch classification: addBranchState now classifies branches as forward (conditional) or backward (loop) based on whether the target label has already been encountered in the linear traversal. Forward branch labels are recorded in m_forwardBranchLabels.
  • Live branch state snapshots: A new LiveBranchState struct and m_liveBranchStates map capture the full observer state (queues, flags, types) at forward branch points, preserving Register::ValuePtr entries needed for restore and merge operations.
  • State reset after unconditional branches: An m_afterUnconditionalBranch flag ensures that when addLabelState is reached after an s_branch (e.g., end of the if-body), the observer resets to the saved branch state instead of continuing with the previous block's accumulated state. This processes each conditional block independently.
  • Per-position union merge at join points: At conditional join labels (e.g., ConditionalBottom), the fall-through and branch queue states are merged via a back-aligned per-position union. Entries are aligned by distance from the end of the queue, and registers from the branch path are inserted into empty slots at corresponding positions. If no slot is available, m_needsWaitZero is set as a fallback.
  • Relaxed vs. strict assertion modes: assertSafeToBranchTo now accepts a strict parameter. Backward branches use strict checking (both sides must match). Forward branches use relaxed checking (label state may be a superset of branch state due to merging).

Test Plan

  • Verifiy existing tests pass
  • Verify that the waitcnt assertions no longer fire false positives for forward branches while still catching loop-edge inconsistencies

Test Result

TBD

Submission Checklist

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.

❌ Your project status has failed because the head coverage (76.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    #4908   +/-   ##
========================================
  Coverage    65.35%   65.35%           
========================================
  Files         1718     1718           
  Lines       267217   267217           
  Branches     37047    37047           
========================================
  Hits        174637   174637           
  Misses       77072    77072           
  Partials     15508    15508           
Flag Coverage Δ *Carryforward flag
hipBLAS 90.67% <ø> (ø) Carriedforward from f80ec00
hipBLASLt 43.55% <ø> (ø)
hipCUB 81.98% <ø> (ø) Carriedforward from f80ec00
hipDNN 80.74% <ø> (ø) Carriedforward from f80ec00
hipFFT 55.93% <ø> (ø) Carriedforward from f80ec00
hipRAND 76.12% <ø> (ø) Carriedforward from f80ec00
hipSOLVER 68.81% <ø> (ø) Carriedforward from f80ec00
hipSPARSE 84.70% <ø> (ø) Carriedforward from f80ec00
rocBLAS 47.97% <ø> (ø) Carriedforward from f80ec00
rocFFT 47.75% <ø> (ø) Carriedforward from f80ec00
rocRAND 57.06% <ø> (ø) Carriedforward from f80ec00
rocSOLVER 76.83% <ø> (ø) Carriedforward from f80ec00
rocSPARSE 71.53% <ø> (ø) Carriedforward from f80ec00

*This pull request uses carry forward flags. Click here to find out more.

🚀 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.

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.

2 participants