Skip to content

[rocjitsu][amdisa][add-atomic 4/5] Add s_atomic_cmpswap* instructions - #13142

Merged
kuhar merged 1 commit into
developfrom
users/accauble/rj-s-atomic-cmpswap
Oct 21, 2026
Merged

kuhar merged 1 commit into
developfrom
users/accauble/rj-s-atomic-cmpswap

Conversation

@accauble

@accauble accauble commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Motivation

Adds the s_atomic_cmpswap* instructions, which needed to be treated differently since they use data from double the registers (one for compare, one for source).

Issue Tracking

Part of Issue #13025

@accauble
accauble added this pull request to stack #13105 October 8, 2026 19:30
@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

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

Copy link
Copy Markdown

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

@accauble
accauble force-pushed the users/accauble/rj-s-atomic-cmpswap branch 2 times, most recently from 28cb7c0 to e1944ab Compare October 20, 2026 19:58
@accauble
accauble force-pushed the users/accauble/rj-s-atomic-cmpswap branch from e1944ab to 7d93070 Compare October 20, 2026 20:39
@accauble
accauble marked this pull request as ready for review October 20, 2026 20:43
@accauble
accauble requested review from a team and atgutier as code owners October 20, 2026 20:43

@kuhar kuhar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

It would be useful to extend the compare-swap tests with a few of the neighboring scalar-atomic checks (optional coverage improvements):

  • In emulation/rocjitsu/tests/scalar_atomic_test.cpp:307, include source in the trace and seed/check SCC and check EXEC, so failures show both input operands and the new operation also checks preservation of wave state.
  • At emulation/rocjitsu/tests/scalar_atomic_test.cpp:576, check lgkmcnt == dwords after the unavailable tick and exercise both GLC values, checking that GLC=0 preserves SDATA after the lost exchange. This would cover pending-counter retention and suppressed writeback on that retry path.

@kuhar
kuhar force-pushed the users/accauble/rj-s-atomic-cmpswap branch 2 times, most recently from 1222617 to a64fe16 Compare October 21, 2026 10:49
Base automatically changed from users/accauble/rj-s-atomic-int-ops to develop October 21, 2026 11:10
* These instructions are slightly different since it can use data
  from double the registers (one for compare, one for source).
@kuhar
kuhar force-pushed the users/accauble/rj-s-atomic-cmpswap branch from a64fe16 to 4484cb4 Compare October 21, 2026 11:10
@kuhar
kuhar merged commit a7ef835 into develop Oct 21, 2026
74 checks passed
@kuhar
kuhar deleted the users/accauble/rj-s-atomic-cmpswap branch October 21, 2026 13:13
dgaliffiAMD pushed a commit that referenced this pull request Oct 21, 2026
…ns (#13142)

## Motivation

Adds the `s_atomic_cmpswap*` instructions, which needed to be treated
differently since they use data from double the registers (one for
compare, one for source).

## Issue Tracking

Part of Issue #13025
@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

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

Labels

documentation Improvements or additions to documentation organization: ROCm project: rocjitsu

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants