[AMDGPU] Fix: propagate amdgpu-ieee and amdgpu-dx10-clamp to all functions#775
Closed
paveltc wants to merge 3 commits into
Closed
[AMDGPU] Fix: propagate amdgpu-ieee and amdgpu-dx10-clamp to all functions#775paveltc wants to merge 3 commits into
paveltc wants to merge 3 commits into
Conversation
…I#11) Lets users override AMDGPU codegen attributes per kernel without editing the JIT pipeline. Attributes must be pre-registered in quadrants/program/fn_attrs_registry.h; unknown backend or attribute names raise QuadrantsSyntaxError at decoration time. Currently registered: amdgpu-max-num-workgroups, amdgpu-agpr-alloc, amdgpu-waves-per-eu, amdgpu-flat-work-group-size. Examples: ```python @qd.kernel(fn_attrs={"amdgpu": {"amdgpu-max-num-workgroups": "128,1,1"}}) def k(...): ... @qd.kernel(fn_attrs={"amdgpu": {"amdgpu-waves-per-eu": "1,2"}}) def k(...): ... ``` Plumbed Python decorator -> Kernel.fn_attrs -> set_fn_attrs pybind -> codegen_llvm.cpp addFnAttr -> jit_amdgpu.cpp (defaults gated by hasFnAttribute so user values win). Included in both fastcache and frontend offline cache keys so changing fn_attrs forces a rebuild.
…s PR Four fixes from Codex review of PR Genesis-Embodied-AI#774: 1. Gate unsafe-fp-math on config_.fast_math Previously unconditional in jit_amdgpu.cpp; now matches the TargetOptions block below it. qd.init(fast_math=False) now correctly preserves IEEE semantics on AMDGPU. 2. Fix block_dim clamping in mark_function_as_amdgpu_kernel Prior impl clamped both min and max to max(block_dim, 64), so block_dim=1 produced "64,64" instead of "1,1". Now uses block_dim as-is so the IR accurately reflects the actual dispatch size. 3. Make cuda_graph a deprecated alias for graph cuda_graph=True was silently ignored because the launch path only checks use_graph. Now _kernel_impl treats cuda_graph=True as graph=True with a DeprecationWarning, and removes the now-unused use_cuda_graph field from Kernel. 4. Add fn_attrs docs to optimization_passes.md Per AGENTS.md, public API changes need docs/ updates. Adds a "Per-kernel LLVM function attributes" section documenting fn_attrs, the supported amdgpu attributes, and cache interaction. Co-authored-by: Cursor <cursoragent@cursor.com>
…ions
These attributes were only set on AMDGPU_KERNEL functions, creating an
attribute mismatch with internal runtime device functions (e.g.
gpu_parallel_range_for). LLVM's inliner refuses to inline a callee into a
caller when they carry mismatching target-specific attributes.
Without that inlining, InferAddressSpaces cannot see the full pointer chain
from kernel params to field data and cannot promote flat_load/store/atomic
to global_*, causing a ~4% throughput regression (301k vs 314k) and
flat-atomic coherency issues on MI300X.
Moving the two attributes to the all-functions loop restores inlining and
allows InferAddressSpaces to emit global_load/global_store/global_atomic.
The hasFnAttribute guards are preserved so user-supplied fn_attrs values
(set via @qd.kernel(fn_attrs={...})) still win over these defaults.
Backport of ROCm/quadrants@50d5a0fad.
Co-authored-by: Cursor <cursoragent@cursor.com>
paveltc
changed the title
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
3 participants
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.
Summary
Backport of ROCm/quadrants@50d5a0fad.
`amdgpu-ieee=false` and `amdgpu-dx10-clamp=false` were only being set on
`AMDGPU_KERNEL` functions. LLVM's inliner refuses to inline a callee into a
caller when they carry mismatching target-specific attributes, so keeping these
on kernels only blocks runtime device functions (e.g. `gpu_parallel_range_for`)
from being inlined into kernels.
Without that inlining, `InferAddressSpaces` cannot see the full pointer chain
from kernel params to field data and cannot promote `flat_load`/`flat_store`/
`flat_atomic` to `global_*`, causing:
Moving the two attributes to the outer all-functions loop restores inlining and
allows `InferAddressSpaces` to emit `global_load`/`global_store`/`global_atomic`.
The `hasFnAttribute` guards are preserved so values set via
`@qd.kernel(fn_attrs={"amdgpu": {"amdgpu-ieee": ...}})` still win.
Changes
`amdgpu-dx10-clamp` `addFnAttr` calls from the `AMDGPU_KERNEL`-only block
into the outer all-functions loop. 1 file, +16/−6 lines.
Test results
Full suite run in Docker image `quadrants:amd-upstream-pr4` with
`QD_AMDGPU_FORCE_PERMLANE64_FALLBACK=1`:
```
20 failed, 5097 passed, 142 skipped, 28 xfailed (1:37:00)
```
All 20 failures are pre-existing baseline flaky tests
(`test_exclusive_scan_composition`, `test_block_inclusive`). Zero new regressions.
Note on stacking
This PR builds on #774 (per-kernel LLVM function attributes) which introduced
the all-functions attribute loop. If #774 merges first, this PR's base can be
retargeted to `main` with no changes to the commit itself.