[Bugfix][Dialect] Reject vector operands in atomic copy atoms - #918
Open
AiyyappanMR wants to merge 2 commits into
Open
[Bugfix][Dialect] Reject vector operands in atomic copy atoms#918AiyyappanMR wants to merge 2 commits into
AiyyappanMR wants to merge 2 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Passing a vector-typed value to an atomic-add copy atom currently generates illegal LLVM IR (
llvm.atomicrmw faddwith avector<Nxf32>operand), which is rejected by LLVM's IR verifier and, on AMDGPU, eventually surfaces as a crypticunsupported cmpxchgfailure deep in backend codegen with no useful diagnostic pointing back to the actual source of the problem (see issue #732, case 1). This PR adds a check that catches this earlier and reports it as a clear compile-time error instead.Technical Details
UniversalAtomicAdd(and atomic copy atoms in general) only support scalar operands on this hardware — there's no native vectorized atomic-add instruction, andllvm.atomicrmw faddonly accepts a scalar floating-point operand, not a vector type.Added a check in
FlyUniversalOps.cppthat rejects a vector-typed operand being routed into an atomic copy atom, emitting: "vectorized atomic ops are unsupported; use a scalar-sized copy atom" instead of silently generating illegal IR that only fails much later duringgpu-module-to-binary/backend codegen.Test Plan
Reproduced the original crash using the example from issue #732 (case 1:
sum_kernelwithUniversalCopy128bload atom +UniversalAtomicAddstore atom, no retile). Confirmed the new check triggers a clear compile-time diagnostic instead of the previous downstreamunsupported cmpxchgcrash.Test Result
Before: kernel compilation proceeded past MLIR lowering and failed with
unsupported cmpxchgduringgpu-module-to-binary, with no indication of the actual cause.After: compilation fails immediately with a clear error pointing at the vector-operand/atomic-atom mismatch.
Submission Checklist