Skip to content

[rocjitsu] Preserve atomic semantics and GLOBAL-only decoding - #11489

Merged
kuhar merged 2 commits into
developfrom
users/kuhar/atomic-semantics-and-decode
Sep 12, 2026
Merged

[rocjitsu] Preserve atomic semantics and GLOBAL-only decoding#11489
kuhar merged 2 commits into
developfrom
users/kuhar/atomic-semantics-and-decode

Conversation

@kuhar

@kuhar kuhar commented Sep 11, 2026

Copy link
Copy Markdown
Member

Conditional and clamped subtraction were lowered to ordinary subtraction,
causing unsigned underflow. Keep their distinct operations through lowering,
execution and DBT, including returning and non-returning DS forms.
Correct modern min/max NaN payload selection and preserve legacy cache-add
denormal results while flushing their inputs.

The decoder discarded GLOBAL encodings without a FLAT counterpart. Retain
unique entries through profile policy, enforce their segment at decode time,
and preserve implicit operand metadata. Restore GLOBAL ADDTID scalar-address
generation with each profile's signed offset width and payload field.

Swap compare-store data fields when translating between different operand
orders. Require semantic expansion for floating atomics whose NaN or denormal
policies differ; unsupported translations now fail instead of changing results.

Regenerate all AMDGPU targets and translation tables. Add decoded boundary,
segment-rejection and translation regressions, and qualify packed atomics on
older CDNA and RDNA4 targets.

Copilot AI lite review requested due to automatic review settings September 11, 2026 02:06
@kuhar
kuhar requested review from a team and atgutier as code owners September 11, 2026 02:06
@therock-pr-bot

therock-pr-bot Bot commented Sep 11, 2026 •
edited
Loading

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 Sep 11, 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.

Copilot AI reviewed Sep 11, 2026
View reviewed changes

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.

🟡 Changes recommended

Unresolved decoder-collision and floating-atomic semantic issues remain.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR preserves AMDGPU atomic semantics across execution, decoding, DBT translation, and regenerated target outputs.

Changes:

  • Preserves conditional/clamped subtraction and floating-point NaN/denormal behavior.
  • Retains and validates GLOBAL-only encodings, ADDTID generation, and operand metadata.
  • Updates translation rules and adds regression coverage.
File summaries
File Reviewed change
emulation/rocjitsu/tests/dbt/translate_test.cpp Adds translation regression coverage.
emulation/rocjitsu/tests/CMakeLists.txt Registers test coverage.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/vm/amdgpu/mem_state.h Carries atomic operation and policy state.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/shared/fp_mode.h Applies floating atomic policies; FCMPSWAP failure and modern MIN/MAX output paths need correction.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna4/vglobal_exec.cpp Regenerates RDNA4 GLOBAL execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna4/vflat_exec.cpp Regenerates RDNA4 FLAT execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna4/vbuffer_exec.cpp Regenerates RDNA4 BUFFER execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3/opcodes.h Regenerates RDNA3 opcode metadata.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3/mubuf_exec.cpp Regenerates RDNA3 buffer execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3/flat.h Regenerates RDNA3 FLAT declarations.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3/flat.cpp Regenerates RDNA3 FLAT implementation.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3/flat_exec.cpp Regenerates RDNA3 FLAT execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3/execution_backend.h Regenerates RDNA3 backend declarations.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3/execution_backend_exec.cpp Regenerates RDNA3 backend execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3/decoder.cpp Regenerates RDNA3 decoding.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3_5/opcodes.h Regenerates RDNA3.5 opcode metadata.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3_5/mubuf_exec.cpp Regenerates RDNA3.5 buffer execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3_5/flat.h Regenerates RDNA3.5 FLAT declarations.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3_5/flat.cpp Regenerates RDNA3.5 FLAT implementation.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3_5/flat_exec.cpp Regenerates RDNA3.5 FLAT execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3_5/execution_backend.h Regenerates RDNA3.5 backend declarations.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3_5/execution_backend_exec.cpp Regenerates RDNA3.5 backend execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3_5/decoder.cpp Regenerates RDNA3.5 decoding.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna2/opcodes.h Regenerates RDNA2 opcode metadata.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna2/mubuf_exec.cpp Regenerates RDNA2 buffer execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna2/flat.h Regenerates RDNA2 FLAT declarations.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna2/flat.cpp Regenerates RDNA2 FLAT implementation.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna2/execution_backend.h Regenerates RDNA2 backend declarations.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna2/execution_backend_exec.cpp Regenerates RDNA2 backend execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna2/decoder.cpp Regenerates RDNA2 decoding.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna1/mubuf_exec.cpp Regenerates RDNA1 buffer execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna1/flat_exec.cpp Regenerates RDNA1 FLAT execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna5/vglobal_exec.cpp Regenerates CDNA5 GLOBAL execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna5/vflat_exec.cpp Regenerates CDNA5 FLAT execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna5/vbuffer_exec.cpp Regenerates CDNA5 BUFFER execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna4/opcodes.h Regenerates CDNA4 opcode metadata.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna4/mubuf_exec.cpp Regenerates CDNA4 buffer execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna4/flat.h Regenerates CDNA4 FLAT declarations.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna4/flat_exec.cpp Regenerates CDNA4 FLAT execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna4/execution_backend.h Regenerates CDNA4 backend declarations.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna4/execution_backend_exec.cpp Regenerates CDNA4 backend execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna4/decoder.cpp Regenerates CDNA4 decoding.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna3/opcodes.h Regenerates CDNA3 opcode metadata.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna3/mubuf_exec.cpp Regenerates CDNA3 buffer execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna3/flat.h Regenerates CDNA3 FLAT declarations.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna3/flat_exec.cpp Regenerates CDNA3 FLAT execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna3/execution_backend.h Regenerates CDNA3 backend declarations.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna3/execution_backend_exec.cpp Regenerates CDNA3 backend execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna3/decoder.cpp Regenerates CDNA3 decoding.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna2/opcodes.h Regenerates CDNA2 opcode metadata.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna2/mubuf_exec.cpp Regenerates CDNA2 buffer execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna2/flat.h Regenerates CDNA2 FLAT declarations.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna2/flat.cpp Regenerates CDNA2 FLAT implementation.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna2/flat_exec.cpp Regenerates CDNA2 FLAT execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna2/execution_backend.h Regenerates CDNA2 backend declarations.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna2/execution_backend_exec.cpp Regenerates CDNA2 backend execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna1/opcodes.h Regenerates CDNA1 opcode metadata.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna1/mubuf_exec.cpp Regenerates CDNA1 buffer execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna1/flat.h Regenerates CDNA1 FLAT declarations.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna1/flat.cpp Regenerates CDNA1 FLAT implementation.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna1/flat_exec.cpp Regenerates CDNA1 FLAT execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna1/execution_backend.h Regenerates CDNA1 backend declarations.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna1/execution_backend_exec.cpp Regenerates CDNA1 backend execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/dbt/generated/legalization_cdna2_to_rdna3.h Regenerates CDNA2-to-RDNA3 legalization.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/dbt/generated/legalization_cdna2_to_cdna4.h Regenerates CDNA2-to-CDNA4 legalization.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/dbt/generated/legalization_cdna2_to_cdna3.h Regenerates CDNA2-to-CDNA3 legalization.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/dbt/generated/legalization_cdna1_to_rdna3.h Regenerates CDNA1-to-RDNA3 legalization.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/dbt/generated/legalization_cdna1_to_rdna2.h Regenerates CDNA1-to-RDNA2 legalization.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/dbt/generated/legalization_cdna1_to_rdna1.h Regenerates CDNA1-to-RDNA1 legalization.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/dbt/generated/legalization_cdna1_to_cdna4.h Regenerates CDNA1-to-CDNA4 legalization.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/dbt/generated/encoding_cdna4_to_rdna4.h Regenerates CDNA4-to-RDNA4 encoding.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/dbt/generated/encoding_cdna4_to_rdna3.h Regenerates CDNA4-to-RDNA3 encoding.
emulation/rocjitsu/lib/python/amdisa/tests/test_sema_derive.py Tests updated semantic derivation.
emulation/rocjitsu/lib/python/amdisa/tests/test_generator_profile_gates.py Tests profile gating behavior.
emulation/rocjitsu/lib/python/amdisa/parser.py Retains GLOBAL entries and validates segments; scratch-only opcode collisions require correction.
emulation/rocjitsu/lib/python/amdisa/legalization.py Applies fail-closed semantic translation rules.
emulation/rocjitsu/lib/python/amdisa/gpuisa.py Updates ISA and profile handling.
emulation/rocjitsu/lib/python/amdisa/encoding_translator_codegen.py Handles compare-store operand ordering and translation policy.
Review details

Suppressed comments (1)

emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/shared/fp_mode.h:406

  • When legacy_minmax is false, the selected denormal is returned from old_input/source_input without applying the output-denormal bit. Thus MODE denorm mode 1 (preserve inputs, flush outputs) makes modern DS MIN/MAX write a denormal back to LDS instead of flushing it. Apply the output policy to every modern result branch while retaining the legacy original-bit behavior.
  • Files reviewed: 16/116 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread emulation/rocjitsu/lib/python/amdisa/parser.py Resolved
Comment thread emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/shared/fp_mode.h Resolved
sjain-stanford
sjain-stanford reviewed Sep 11, 2026
View reviewed changes

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

Left one thought

Comment thread emulation/rocjitsu/tests/atomic_semantics_and_decode_test.cpp Resolved
@kuhar
kuhar force-pushed the users/kuhar/atomic-semantics-and-decode branch 2 times, most recently from ed30644 to a0ce8d9 Compare September 11, 2026 21:18
@kuhar
[rocjitsu] Preserve floating atomic policies and compare-store operands
e4908da 
Scalar floating atomics inherited host rounding and lost the denormal
controls defined by the ISA. Floating compare-store used integer equality,
and older DS compare-store forms reversed comparison and replacement.

Retain floating compare-swap in semantic lowering and normalize DS source
order through architecture profiles. Capture the LDS and L2 denormal
policies at issue, apply fixed nearest-even addition, and handle signed
zero, NaNs and older min/max selection rules without host dependence.
Keep floating compare-swap returns narrow for register dataflow. Give DS
conditional exchange its own one-source operation and per-dword enables.

Regenerate all targets with generate-amdisa.sh. Cover indexed LDS across
all ten architectures, buffer and global operations, F32/F64 and returning
forms, and FLAT-to-LDS routing. Check opposing MODE and host rounding,
denormal inputs/results, signed-zero equality, NaNs, operand order, and
conditional-exchange address masking and independent dword enables.
@kuhar
[rocjitsu] Preserve atomic semantics and GLOBAL-only decoding
d3cee1f 
Conditional and clamped subtraction were lowered to ordinary subtraction,
causing unsigned underflow. Keep their distinct operations through lowering,
execution and DBT, including returning and non-returning DS forms.
Correct modern min/max NaN payload selection and preserve legacy cache-add
denormal results while flushing their inputs.

The decoder discarded GLOBAL encodings without a FLAT counterpart. Retain
unique entries through profile policy, enforce their segment at decode time,
and preserve implicit operand metadata. Restore GLOBAL ADDTID scalar-address
generation with each profile's signed offset width and payload field.

Swap compare-store data fields when translating between different operand
orders. Require semantic expansion for floating atomics whose NaN or denormal
policies differ; unsupported translations now fail instead of changing results.

Regenerate all AMDGPU targets and translation tables. Add decoded boundary,
segment-rejection and translation regressions, and qualify packed atomics on
older CDNA and RDNA4 targets. Exercise CDNA1/2 GLOBAL F32 addition and
the input-only denormal control for DS min/max and compare-store.
@kuhar
kuhar force-pushed the users/kuhar/atomic-semantics-and-decode branch from a0ce8d9 to d3cee1f Compare September 12, 2026 00:46
@kuhar
kuhar merged commit a7bb0b2 into develop Sep 12, 2026
32 of 33 checks passed
@kuhar
kuhar deleted the users/kuhar/atomic-semantics-and-decode branch September 12, 2026 01:17
@systems-assistant

systems-assistant Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

abchoudh-amd pushed a commit that referenced this pull request Sep 16, 2026
@kuhar @abchoudh-amd
[rocjitsu] Preserve atomic semantics and GLOBAL-only decoding (#11489)
3499270 
Conditional and clamped subtraction were lowered to ordinary subtraction,
causing unsigned underflow. Keep their distinct operations through lowering,
execution and DBT, including returning and non-returning DS forms.
Correct modern min/max NaN payload selection and preserve legacy cache-add
denormal results while flushing their inputs.

The decoder discarded GLOBAL encodings without a FLAT counterpart.
Retain unique entries through profile policy, enforce their segment at decode time,
and preserve implicit operand metadata. Restore GLOBAL ADDTID scalar-address
generation with each profile's signed offset width and payload field.

Swap compare-store data fields when translating between different operand
orders. Require semantic expansion for floating atomics whose NaN or denormal
policies differ; unsupported translations now fail instead of changing results.

Regenerate all AMDGPU targets and translation tables. Add decoded boundary,
segment-rejection and translation regressions, and qualify packed atomics on
older CDNA and RDNA4 targets.
dlamd1dai pushed a commit to dlamd1dai/rocm-systems that referenced this pull request Sep 16, 2026
@kuhar @dlamd1dai
[rocjitsu] Preserve atomic semantics and GLOBAL-only decoding (ROCm#1…
879fe3f 
…1489)

Conditional and clamped subtraction were lowered to ordinary subtraction,
causing unsigned underflow. Keep their distinct operations through lowering,
execution and DBT, including returning and non-returning DS forms.
Correct modern min/max NaN payload selection and preserve legacy cache-add
denormal results while flushing their inputs.

The decoder discarded GLOBAL encodings without a FLAT counterpart.
Retain unique entries through profile policy, enforce their segment at decode time,
and preserve implicit operand metadata. Restore GLOBAL ADDTID scalar-address
generation with each profile's signed offset width and payload field.

Swap compare-store data fields when translating between different operand
orders. Require semantic expansion for floating atomics whose NaN or denormal
policies differ; unsupported translations now fail instead of changing results.

Regenerate all AMDGPU targets and translation tables. Add decoded boundary,
segment-rejection and translation regressions, and qualify packed atomics on
older CDNA and RDNA4 targets.
@yashjha123 yashjha123 mentioned this pull request Oct 26, 2026
Merged
yashjha123 added a commit that referenced this pull request Oct 27, 2026
@yashjha123
[rocjitsu] Match floating compare-swap atomics to hardware (#14276)
fe5fd04 
Fix two critical issues when performing floating-point compare-swap.
These were found when working on a related PR (#13996). Tested against
W7900 (gfx1100), RX 6800 XT (gfx1030), and RX 5700 XT (gfx1010)
captures.

(1) The memory swap was triggered incorrectly, as the floating
comparison was confused with an integer comparison. The wrong fallback
to the integer path was due to a drift in the implementations (the
execution paths now share operand classification and an FP operation
mapping).

(2) The overwritten value didn't match the HW behaviour. The written
values don't flush, but the comparison still flushes them, apparently.
`fp_mode::atomic_scalar` uses the flushed values only to decide
equality, then returns the original replacement or memory bits.

<details>
<summary>More details for agent's context</summary>

### What changes

| Case | develop | This PR | Measured |
|---|---|---|---|
| Translated backend, any `FCMPSWAP` (memory 1.0, compare 1.0,
replacement 2.0) | memory stays `3f800000`, reports success | memory
becomes `40000000` | gfx1100 |
| Flushed denormals, match with a denormal replacement (memory 1.0,
compare 1.0, replacement `0x1`) | stores `0` | stores `0x1` | gfx1100
global and LDS; gfx1030, gfx1010 LDS |
| Flushed denormals, mismatch on denormal memory (memory `0x1`, compare
3.0) | writes `0` | memory untouched | same |
| Flushed denormals, mismatch on negative denormal memory (memory
`0x80000001`, compare 1.0) | writes `0x80000000` | memory untouched |
same |

The 32-bit patterns are shown; the 64-bit LDS forms behave the same way
with `0x0000000000000001` and `0x8000000000000001`. The returned pre-op
value was already correct.

Unchanged and confirmed on hardware: −0 equals +0, NaNs never match
(including identical quiet and signaling payloads), flushed denormals
compare equal to ±0 and to each other, preserved denormals compare by
value, and a signaling-NaN replacement is stored without quieting.

### Hardware evidence

- `global_atomic_cmpswap_f32` and `ds_cmpstore_rtn_f32/f64` on a W7900
(gfx1100); `ds_cmpst_rtn_f32/f64` on an RX 6800 XT (gfx1030) and an RX
5700 XT (gfx1010).
- Inline assembly, so the compiler cannot lower to an integer
compare-swap. 18–26 cases per instruction, each run with MODE.FP_DENORM
preserving and flushing denormals. Every capture was rerun with
identical output, and the gfx1010/gfx1030 LDS results are identical to
gfx1100's.
- The same probe binaries under rocjitsu's gfx1100 config reproduce the
W7900 output byte for byte with this PR; before it, the flushed-denormal
rows above differed.

### Changes

- `atomic_op.h`: `atomic_uses_two_sources()` adds `FCMPSWAP` and `WRAP`
to the translated handler's set. `memory_pipeline.cpp` uses one optional
FP operation mapping for both classification and dispatch across the
cached, translated and LDS paths. Unsupported evaluator inputs now fail
explicitly instead of returning the old value; invalid `ScalarAtomicOp`
values are also rejected instead of falling through to max.
- `fp_mode.h`: `atomic_scalar`'s `FCMPSWAP` case returns
`source_bits`/`old_bits` instead of their flushed copies.

### Tests

- `GpuVmPipeline.FloatCompareSwapMatchesAcrossBackends`: synthetic 32-
and 64-bit compare-swap requests through both backends, denormals
preserved and flushed. The CDNA5 fixture has no decoded `FCMPSWAP`
instruction; this checks shared pipeline behaviour. Lanes cover a match,
a mismatch, −0 against +0, NaN, a denormal against +0, a denormal
mismatch and a denormal replacement; checks memory and the returned
value.
- `FpModePolicyTest.FloatCompareSwapFlushesOnlyTheComparison`: the
measured cases as a table, both widths and both MODE settings.
- `AtomicPolicyExecutionTest.DsFloatCompareStoreEqualityAndDenormals`:
three expectations corrected. They asserted the flushed write introduced
with #11489. RDNA3/3.5 ISA section 13.2 and CDNA2 section 9.5.2 /
CDNA3/4 section 9.2.2 specify flushing the CompareStore result, but the
captures contradict that rule on the measured paths. Code and tests now
state this discrepancy and identify unmeasured targets.
- `AtomicPolicyExecutionTest.MemoryFloatCompareStore`: three rows added
for a matching denormal replacement and positive and negative denormal
mismatches, covering the changed stored bits through the decoded GLOBAL
and BUFFER paths on RDNA1–3.5. Its existing flush-mode row passes under
either policy; each new row fails on all four architectures and both
paths when the selection is reverted to the flushed values.
-
`AtomicSemanticsAndDecode.IntegerEvaluatorRejectsUnsupportedOperations`
and `FpModePolicyTest.ScalarAtomicRejectsUnsupportedOperations` cover
both widths and fail with the original evaluator fallbacks.
- Each original fix checked by reverting only its library change: the
pipeline test fails only on the translated backend for the first, and
only in flush mode (both widths, both backends) for the second; the
policy test fails exactly the three flush-mode rows per width.

### Results (clang Release)

- Before the review fixes, `ctest`: 8,862 of 8,876 passed. The 14
failures (3 `HsaTest`, missing `hsa_amd_queue_create` on the host
runtime; 11 `Gfx1250ExecutionTest.TensorDma*` segfaults) also fail on
unmodified develop.

- At `391d14bb060` (rebased onto `a556b0be0bb`), 521 focused tests pass,
85 are skipped, and none fail. Release Clang 20 linking needed a
build-only explicit instantiation for the existing missing
`ComputeUnitCore::step_impl<true>` symbol; the workaround is not part of
this PR. The full suite and hardware captures have not been rerun for
these review fixes.

- At `05d78d76078`, the focused atomic, compare-swap and pipeline tests
pass (395 passed, 85 skipped), as does `rocjitsu_fp_mode_tests` (35,089
passed).

### Still open

- **Unmeasured paths:** CDNA1–4 LDS, RDNA1/2 cache atomics (32/64-bit
FLAT/global and BUFFER forms), RDNA3.5 LDS/cache, and gfx1100
FLAT/BUFFER F32 inherit the shared policy without equivalent captures.
The gfx1010/gfx1030 captures cover LDS only. In this tree CDNA1–4 reach
`FCMPSWAP` only through LDS; RDNA1/2 implement the 64-bit cache
`fcmpswap_x2` forms. CDNA5/RDNA4 expose no `FCMPSWAP` instructions.
- **Unmeasured MODE settings:** every capture used FP_DENORM 0 or 3,
endpoints that cannot separate input-controlled from output-controlled
comparison flushing. A capture at FP_DENORM 1 or 2 with denormal memory
compared against +0 and a normal replacement would, as this policy swaps
only in mode 2.
- **`WRAP`:** currently only LDS `ds_wrap` on CDNA1–4 and RDNA1–3.5 uses
it, so adding it to the translated handler's two-source set has no
visible effect today.

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

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@sjain-stanford sjain-stanford sjain-stanford approved these changes
@atgutier atgutier Awaiting requested review from atgutier atgutier is a code owner

Assignees

No one assigned

Labels

Not ready to Review organization: ROCm project: rocjitsu

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kuhar @sjain-stanford