[AMDGPU] Add force_inline loop_config hint for parallel range-for bodies - #844
[AMDGPU] Add force_inline loop_config hint for parallel range-for bodies#844paveltc wants to merge 2 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c79dc69864
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
qd.loop_config(force_inline=True/False/None) sets a tri-state per-loop hint,
plumbed frontend -> RangeForStmt (lower_ast) -> OffloadedStmt (offload) and
consumed in the AMDGPU range-for codegen:
True -> mark the loop body AlwaysInline, removing a per-thread call boundary
in hot loops whose small bodies the AMDGPU inliner cost model
otherwise declines to inline.
False -> mark the body NoInline, keeping it a separate call for
register-pressure-sensitive bodies where inlining hurts occupancy.
None -> default; LLVM's inliner cost model decides.
The hint is AMDGPU-only (a no-op on other backends); mesh-for does not carry it.
De-bundled from ROCm/quadrants Genesis-Embodied-AI#15, deliberately excluding that commit's
launch-overhead rework, the default_gpu_block_dim change, and the warp-shuffle
changes (f32 XOR shuffle already works on AMDGPU upstream via the subgroup path).
Co-authored-by: Cursor <cursoragent@cursor.com>
- gen_offline_cache_key.cpp: emit FrontendForStmt::force_inline into the AST offline-cache key alongside the other loop-config fields, so two otherwise-identical kernels that differ only by qd.loop_config(force_inline=True/False/None) no longer collide in the offline cache and inherit the wrong AlwaysInline/NoInline attribute. - docs: document the new AMDGPU-only qd.loop_config(force_inline=...) keyword (True/False/None semantics) in the optimization-passes guide, per AGENTS.md's requirement to keep user-facing docs in sync with public API changes. Co-authored-by: Cursor <cursoragent@cursor.com>
4a08595 to
66ded2a
Compare
|
Closing this in favor of #775. Root cause recap: the parallel range-for body was left out-of-line not because of an LLVM cost/size decision, but because of a hard inline barrier — AMDGPU's areInlineCompatible refuses to inline when caller and callee disagree on amdgpu-ieee / amdgpu-dx10-clamp. The kernel carried these attributes; the body did not. This blocks inlining regardless of body size (even a 2-instruction body stayed out-of-line; a real Genesis run left 81/82 kernels' bodies out-of-line for this same reason). Why #844 is redundant: #844 adds an env-gated force-inline knob (QD_AMDGPU_RANGE_FOR_INLINE_MAX_INSTS). But #775 removes the attribute mismatch at the source by propagating amdgpu-ieee/amdgpu-dx10-clamp to all functions, after which LLVM inlines the body on its own (via the existing "last call to internal" bonus). I verified this empirically: with #775's two-attribute change alone (fast-math removed, flat-work-group-size / wave-limiter untouched), the optimized IR shows the body fully inlined — 0 out-of-line defines, 0 out-of-line call sites. Ordering note: #844 actually depends on #775 to work reliably — without the compatibility fix, an alwaysinline hint can still be overridden by the same barrier. So the knob is strictly downstream of the change that already solves the problem. The only scenario where a force-inline knob would matter is a body large enough that LLVM declines on genuine cost grounds even with compatible attributes (we saw bodies up to ~51k instructions). Force-inlining those is usually the wrong call for occupancy, and it's a niche opt-in tuning concern rather than a default-build need — not enough to justify a public/env knob. Closing as redundant with #775. Happy to reopen as an explicit, documented performance-tuning escape hatch if a concrete large-body case shows a win. |
2 participants
Summary
Adds a tri-state
qd.loop_config(force_inline=True/False/None)hint controlling whether an AMDGPU parallel range-for body is inlined into the launcher trampoline:True→ mark the bodyalwaysinline(removes a per-thread call boundary in hot loops whose small bodies the AMDGPU inliner cost model otherwise declines to inline — the body function carries a differentamdgpu-flat-work-group-sizethan the kernel, which skews the cost model).False→ mark the bodynoinline(keep it a separate call for register-pressure-sensitive bodies where inlining hurts occupancy).None(default) → unchanged; LLVM's inliner decides.The hint is plumbed front-end →
FrontendForStmt→RangeForStmt(lower_ast) →OffloadedStmt(offload) → consumed incodegen_amdgpu.cpp. It is AMDGPU-only and a no-op on other backends; mesh-for does not carry it. 9 files, +54, all additive.Testing
Built and validated on gfx942 (MI300X),
arch=amdgpu:f32range-for kernel produces identical, correct results withforce_inlineset toNone,True, andFalse.alwaysinlineunderTrue,noinlineunderFalse, and neither underNone— confirming the hint reaches codegen and takes effect.Note: the inlining win is workload-dependent; this PR verifies the mechanism (attribute + correctness), not a specific speedup number.
Made with Cursor