Skip to content

[rocjitsu] Enable ordinary memory wait diagnostics by default - #13144

Merged
kuhar merged 1 commit into
developfrom
users/kuhar/waitcnt-diagnostics-default
Oct 9, 2026
Merged

kuhar merged 1 commit into
developfrom
users/kuhar/waitcnt-diagnostics-default

Conversation

@kuhar

@kuhar kuhar commented Oct 8, 2026 •

Copy link
Copy Markdown
Member

Enable ordinary memory wait warnings by default. Report reads or overwrites of pending memory results without a sufficient wait; execution continues with eagerly computed values. Keep gfx1250 XCNT replay-source checks opt-in.

Add --wait-checking=on|off|all to rocjitsu and mirage run, overriding every CU and GPU for one launch:

  • on: enable ordinary checks and disable XCNT checks.
  • off: disable both checks.
  • all: enable ordinary and gfx1250 XCNT checks.

If warnings appear incorrect or checking slows a workload, use:

rocjitsu --config /path/to/config.json --wait-checking=off -- <command>
mirage run --wait-checking=off -- <command>

No config edit or rebuild is needed. Both --wait-checking MODE and --wait-checking=MODE work; omitting the flag preserves config settings and defaults. A top-level "wait_checking": "off" (or "on" / "all") provides a persistent override.

Compose with --cpu-thread-budget in either order, applying both settings to one config copy and leaving the source unchanged. Mirage supports profiles and explicit --config files in daemon and in-process modes; dedicated flags beat -o options. Reject both flags for enabled DBT guests using an external simulator config, including generated Mirage profiles that previously lost the CPU budget override. Native rocjitsu --attach also refuses overrides.

Document the flag and disable command in both CLI guides and the Sphinx reference. Preserve effective per-CU policies across checkpoint restore. Runtime warnings include the disable command; invalid per-CU values remain errors under a global override.

Validation: 516 Clang 23 Release native tests and 223 Mirage tests pass. Checked-in CLI matrices cover 31 native launches and 24 Mirage combinations, including every mode, both flag orders/spellings, and both effective values and source-file preservation. Mirage coverage includes generated/supplied configs and daemon/in-process execution. Nine IREE application smokes on simulated gfx950/gfx1100/gfx1250 pass sampled numerical checks with no wait warnings; overhead was not measured. Two unrelated DBT byte assertions also reproduce in a pre-change build. Pre-commit, rustfmt, clippy and the rustdoc link check pass.

Completed two local peanut-review rounds for the native change and one additional round for Mirage. Addressed actionable findings, including a disabled-DBT config regression that fails before the fix. A reviewer also verified warning suppression with a deliberate missing-wait HIP kernel on simulated gfx950, in daemon and in-process modes.

@kuhar
kuhar requested review from amd-vivekag and newling and a balanced review from Copilot October 8, 2026 19:52
@kuhar
kuhar requested review from a team and atgutier as code owners October 8, 2026 19:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@kuhar
kuhar requested a review from accauble October 8, 2026 19:52
@github-actions github-actions Bot added documentation Improvements or additions to documentation project: rocjitsu labels Oct 8, 2026
@therock-pr-bot

therock-pr-bot Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

❌ PR Check — Action Required

Check Status Details
📝 PR Description ❌ Fail Error: PR description must reference a JIRA ID, ISSUE ID, or a GitHub closing keyword.
Expected: include a JIRA ID / ISSUE ID line (separator : or -, or omitted; value may be a JIRA key, a number with/without #, or a link), OR a closing keyword + issue reference. Accepted examples:
• JIRA ID : TESTAUTO-6039
• JIRA ID - #330
• JIRA ID #330
• JIRA ID (on separate line)
ROCM-25757
• ISSUE ID : TESTUTO-3334
• ISSUE ID #3334
• ISSUE ID - TESTAUTO-3433
• ISSUE ID (on separate line)
AIRUNTIME-2352
• ISSUE ID : https://github.com/<org_name>/<repo_name>/issues/1234
• Closes #10
• Fixes octo-org/octo-repo#100
• Resolves: #123
• #123
• https://github.com/<org_name>/<repo_name>/issues/123
Current: no valid JIRA/ISSUE/closing-keyword reference found
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

⚠️ 1 policy check(s) failed. Please address the issues above before this PR can be Reviewed.

🚫 Please fix the failed policies

  • ❌ PR Description

The Not ready to Review label was added to this PR. Once all policies pass, the label is removed automatically.

📖 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 Oct 8, 2026

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ PR Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

@kuhar
kuhar force-pushed the users/kuhar/waitcnt-diagnostics-default branch from 8fe78a0 to a5a56a9 Compare October 8, 2026 21:56
Default memory_wait_diagnostics to warn. Report reads or overwrites of
pending memory results without a sufficient wait; execution continues
with eagerly computed values. Keep gfx1250 XCNT replay-source checks
opt-in.

Add --wait-checking=on|off|all to rocjitsu and mirage run. The on mode
enables ordinary checks only; all also enables XCNT. Use
--wait-checking=off if warnings appear incorrect or checking slows a
workload. Omitting the flag preserves config settings and defaults.
Document the modes, precedence and disable command in both CLI guides.

Override every CU and GPU through the same config copy as
--cpu-thread-budget, preserving both settings and the source file.
Support Mirage profiles and explicit configs in daemon and in-process
modes. Reject native rocjitsu --attach overrides and both overrides for
enabled DBT guests using an external simulator config. This also catches
generated Mirage profiles that previously lost the CPU budget override.
Support a persistent top-level wait_checking setting and preserve
effective per-CU policies across checkpoint restore.

Validation: 516 native tests, 223 Mirage tests and nine simulated IREE
smokes pass. CLI matrices cover 31 native and 24 combined-flag Mirage
launches, checking both effective settings and source preservation.
The IREE smokes cover gfx950/gfx1100/gfx1250 with sampled numerical
checks; overhead was not measured. Pre-commit, rustfmt and clippy pass.
@kuhar
kuhar force-pushed the users/kuhar/waitcnt-diagnostics-default branch from a5a56a9 to 247ab35 Compare October 8, 2026 23:08
@kuhar
kuhar requested a review from amd-arosa as a code owner October 8, 2026 23:08
@kuhar
kuhar merged commit b30acfa into develop Oct 9, 2026
69 of 72 checks passed
@kuhar
kuhar deleted the users/kuhar/waitcnt-diagnostics-default branch October 9, 2026 00:11
@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

anujshuk-amd pushed a commit that referenced this pull request Oct 12, 2026
Enable ordinary memory wait warnings by default. Report reads or
overwrites of pending memory results without a sufficient wait;
execution continues with eagerly computed values. Keep gfx1250 XCNT
replay-source checks opt-in.

Add `--wait-checking=on|off|all` to `rocjitsu` and `mirage run`,
overriding every CU and GPU for one launch:

- `on`: enable ordinary checks and disable XCNT checks.
- `off`: disable both checks.
- `all`: enable ordinary and gfx1250 XCNT checks.

If warnings appear incorrect or checking slows a workload, use:

```sh
rocjitsu --config /path/to/config.json --wait-checking=off -- <command>
mirage run --wait-checking=off -- <command>
```

No config edit or rebuild is needed. Both `--wait-checking MODE` and
`--wait-checking=MODE` work; omitting the flag preserves config settings
and defaults. A top-level `"wait_checking": "off"` (or `"on"` / `"all"`)
provides a persistent override.

Compose with `--cpu-thread-budget` in either order, applying both
settings to one config copy and leaving the source unchanged. Mirage
supports profiles and explicit `--config` files in daemon and in-process
modes; dedicated flags beat `-o` options. Reject both flags for enabled
DBT guests using an external simulator config, including generated
Mirage profiles that previously lost the CPU budget override. Native
`rocjitsu --attach` also refuses overrides.

Document the flag and disable command in both CLI guides and the Sphinx
reference. Preserve effective per-CU policies across checkpoint restore.
Runtime warnings include the disable command; invalid per-CU values
remain errors under a global override.

Validation: 516 Clang 23 Release native tests and 223 Mirage tests pass.
Checked-in CLI matrices cover 31 native launches and 24 Mirage
combinations, including every mode, both flag orders/spellings, and both
effective values and source-file preservation. Mirage coverage includes
generated/supplied configs and daemon/in-process execution. Nine IREE
application smokes on simulated gfx950/gfx1100/gfx1250 pass sampled
numerical checks with no wait warnings; overhead was not measured. Two
unrelated DBT byte assertions also reproduce in a pre-change build.
Pre-commit, rustfmt, clippy and the rustdoc link check pass.

Completed two local peanut-review rounds for the native change and one
additional round for Mirage. Addressed actionable findings, including a
disabled-DBT config regression that fails before the fix. A reviewer
also verified warning suppression with a deliberate missing-wait HIP
kernel on simulated gfx950, in daemon and in-process modes.
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.

3 participants