[AMDGPU] Use syncscope("agent") for atomix xor to avoid CAS livelock

[hughperkins]

The AMDGPU LLVM backend refuses to lower system-scope atomicrmw to its native single-instruction atomics (global_atomic_xor, flat_atomic_add_f32, ...) and falls back to a flat_atomic_cmpswap retry loop, because system scope demands mid-kernel coherence with the host CPU. Under high contention that CAS loop livelocks for non-converging ops like xor — test_reduction_single_i32[arch=amdgpu-5] hangs indefinitely on ~1M atomic_xor's into the same address.

Quadrants kernels never need mid-kernel host-CPU visibility (the host only reads results after kernel completion, which forces a full cache flush anyway), so the largest scope we ever need is agent. Setting it explicitly on AMDGPU lets the backend emit the native atomics. CPU and CUDA already single-instruction-lower seq_cst atomics at system scope, so they keep the LLVM default; SPIR-V already spells Device scope explicitly. Internal runtime atomics that touch pinned host memory (adstack overflow flag) are intentionally left at system scope.

Add a per-backend scope summary to atomics.md.

Issue: #

Brief Summary

copilot:summary

Walkthrough

copilot:walkthrough

[hughperkins]

doc seems ok

[github-actions]

Total: 1 file(s) changed, +12 -3 code lines.

[github-actions]

Total: 3 file(s) changed, +21 -4 code lines.

[github-actions]

Diff coverage: 0% · 0 lines, 0 missing

[duburcqa]

Not sure that

"Choosing device scope (rather than the strongest "system" scope) lets every backend lower the op to a single hardware atomic instruction instead of a software CAS retry loop, which matters for correctness as much as for speed: under heavy contention, a CAS loop on a non-converging op like atomic_xor can livelock."

is really relevant for the end user. But as you wish.

[hughperkins]

If the end user is @alanray-tech , I think it might matter.

[alanray-tech]

I think it's necessary to keep these details for advanced users

[duburcqa]

Similarly, this seems a bit too low-level for the end-user. Clarifying that setting syncscope("agent") is essentially to unlocks the native flat_atomic_xor / global_atomic_xor is great typically, but

". AMDGPU's LLVM backend, in contrast, refuses to use its native single-instruction atomics at system scope (it would have to add cache-flush instructions that don't exist for that op), and silently falls back to a CAS loop;"

seems unnecessary here.

[hughperkins]

I think we've agreed that @alanray-tech will review these docs? Let's leave such decisions to him, since he is ultimately the end-user of these docs, at least in the short-term. And in the medium to long-term, I feel he is a reasonable representative of other users.

[alanray-tech]

I prefer to keep the details, from my side it's good to have some "evidence&reason" to support the design decision, which makes it more logical and trustworthy.

[alanray-tech]

Nit / follow-up: AtomicOpType::min and AtomicOpType::max for f32/f64 are not covered by this switch — they fall through to default: break; and land in the call("atomic_min_f32", ...) path below (lines 1366–1373), which dispatches to the runtime-bitcode CAS loops in runtime_module/atomic.h (DEFINE_ATOMIC_OP_COMP_EXCH). Those CAS loops compile __atomic_compare_exchange(..., seq_cst, seq_cst) to a system-scope cmpxchg — the same scope this PR fixes everywhere else. On AMDGPU the per-iteration cache flushes still apply to those CAS loops.

Easy to fix in a follow-up — add two cases to this switch, mirroring the FAdd case:

    case AtomicOpType::min:
      return builder->CreateAtomicRMW(llvm::AtomicRMWInst::FMin,
          llvm_val[stmt->dest], llvm_val[stmt->val],
          llvm::MaybeAlign(0),
          llvm::AtomicOrdering::SequentiallyConsistent,
          kernel_atomic_syncscope(llvm_context, current_arch()));
    case AtomicOpType::max:
      return builder->CreateAtomicRMW(llvm::AtomicRMWInst::FMax,
          llvm_val[stmt->dest], llvm_val[stmt->val],
          llvm::MaybeAlign(0),
          llvm::AtomicOrdering::SequentiallyConsistent,
          kernel_atomic_syncscope(llvm_context, current_arch()));

Why this is safe / sources:

• AtomicRMWInst::FMin/FMax landed in LLVM 15 via D127041 (commit 1023ddaf, Jul 6 2022; branch release/15.x cut Jul 26 2022). Confirmed in the LLVM 15.0.0 release notes, "Changes to the LLVM IR" section. CI uses ROCm 6.4 (.github/workflows/test_gpu.yml:199), which is based on LLVM 19.0.0.

• GFX10+ (RDNA1) has native global_atomic_fmin/global_atomic_fmax for f32 — support was added to the LLVM AMDGPU backend in commit e381833. GFX9 (Vega) does not have native f32 fmin/fmax (only gfx90a/gfx940 got f64 versions; see PR #95592 for the full per-generation breakdown). On GFX9 LLVM will still expand to a CAS loop, but the cmpxchg inside that loop now uses agent scope, avoiding the per-iteration cache-flush overhead.

• Bonus: atomicrmw fmin/fmax match llvm.minnum/llvm.maxnum semantics (IEEE 754-2008 minNum/maxNum — see D127041 review discussion and LLVM 15 LangRef). This aligns f32/f64 with the f16 path which already uses CreateMinNum/CreateMaxNum (codegen_llvm.cpp:1342–1346), fixing the NaN inconsistency the docs note (current f32/f64 runtime CAS uses b > a ? a : b which returns NaN when one input is NaN, while minNum returns the non-NaN value).

The call("atomic_min_f32", ...) block below and the corresponding DEFINE_ATOMIC_OP_COMP_EXCH(min, f32) / (max, f32) / (min, f64) / (max, f64) entries in atomic.h become dead code and can be removed.

[github-actions]

Total: 3 file(s) changed, +39 -17 code lines.

[github-actions]

Diff coverage: 0% · 0 lines, 0 missing

[hughperkins]

Thanks! 🙌

[github-actions]

Total: 3 file(s) changed, +39 -17 code lines.

[github-actions]

Diff coverage: 0% · 0 lines, 0 missing