Skip to content

[rocjitsu][amdisa][add-atomic 3/5] Add rest of s_atomic_* instructions (except cmpswap) - #13110

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

[rocjitsu][amdisa][add-atomic 3/5] Add rest of s_atomic_* instructions (except cmpswap)#13110
kuhar merged 1 commit into
developfrom
users/accauble/rj-s-atomic-int-ops

Conversation

@accauble

@accauble accauble commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Motivation

Adds the rest of the s_atomic_* instructions (except s_atomic_cmpswap* which will be treated differently) in the same way that s_atomic_dec and s_atomic_dec_x2 were added in PR #12963 and PR #13104.

Issue Tracking

Part of Issue #13025.

@accauble
accauble requested a review from kuhar October 8, 2026 14:22
@accauble
accauble added this pull request to stack #13105 October 8, 2026 14:22
@accauble accauble changed the title [rocjitsu][amdisa] Add rest of s_atomic_* instructions (except cmpswap) [rocjitsu][amdisa][add-atomic 3/5] Add rest of s_atomic_* instructions (except cmpswap) Oct 8, 2026
@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 •
edited
Loading

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 mentioned this pull request Oct 8, 2026
Open
49 of 57 tasks
@accauble
accauble force-pushed the users/accauble/rj-s-atomic-int-ops branch from 36b4f31 to efc6e88 Compare October 20, 2026 15:35
@accauble
accauble marked this pull request as ready for review October 20, 2026 15:37
@accauble
accauble requested a review from a team as a code owner October 20, 2026 15:37
kuhar
kuhar reviewed Oct 20, 2026
View reviewed changes

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

We should rename test_non_integer_scalar_atomic_operations_stay_nop to describe operations outside the supported scalar allowlist, and align the explanatory comments with that contract (emulation/rocjitsu/lib/python/amdisa/tests/test_sema_derive.py:2226–2238, emulation/rocjitsu/lib/python/amdisa/semantics.py:1787).

CSUB and COND_SUB_U32 are integer operations, and the shared apply_int_atomic used by the scalar pipeline already implements them. These synthetic scalar names stay nop because the derivation allowlist excludes them. Calling the cases “non-integer” and attributing their exclusion to pipeline capability obscures what the test protects. A name such as test_scalar_atomic_operations_outside_allowlist_stay_nop, with a comment explaining that these synthetic names must not gain scalar support through the flat operation table, would make the intent clear.

Comment thread emulation/rocjitsu/tests/scalar_atomic_test.cpp Resolved
@accauble
accauble force-pushed the users/accauble/rj-s-atomic-int-ops branch from efc6e88 to 573a20e Compare October 20, 2026 17:34
@accauble
accauble requested a review from kuhar October 20, 2026 17:34
@accauble

accauble commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Made those changes

kuhar
kuhar approved these changes Oct 20, 2026
View reviewed changes

@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

@accauble
accauble force-pushed the users/accauble/rj-s-atomic-int-ops branch from 573a20e to f25c18b Compare October 20, 2026 20:04
@accauble
accauble force-pushed the users/accauble/rj-s-atomic-int-ops branch from f25c18b to 499b242 Compare October 20, 2026 20:39
@kuhar
kuhar force-pushed the users/accauble/rj-s-atomic-int-ops branch from 499b242 to 2324024 Compare October 21, 2026 01:14
Base automatically changed from users/accauble/rj-s-atomic-dec-x2 to develop October 21, 2026 10:49
@kuhar
kuhar force-pushed the users/accauble/rj-s-atomic-int-ops branch from 2324024 to 296b8f5 Compare October 21, 2026 10:49
@kuhar
kuhar merged commit 0bd483d into develop Oct 21, 2026
74 checks passed
@kuhar
kuhar deleted the users/accauble/rj-s-atomic-int-ops branch October 21, 2026 11:10
dgaliffiAMD pushed a commit that referenced this pull request Oct 21, 2026
@accauble @dgaliffiAMD
[rocjitsu][amdisa][add-atomic 3/5] Add rest of s_atomic_* instruction…
1419ae8 
…s (except cmpswap) (#13110)

## Motivation

Adds the rest of the `s_atomic_*` instructions (except
`s_atomic_cmpswap*` which will be treated differently) in the same way
that `s_atomic_dec` and `s_atomic_dec_x2` were added in PR #12963 and PR
#13104.

## Issue Tracking

Part of Issue #13025.
@systems-assistant

systems-assistant Bot commented Oct 22, 2026

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

Reviewers

@kuhar kuhar kuhar approved these changes
@atgutier atgutier atgutier approved these changes

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@accauble @kuhar @atgutier