[Doc] Fix subgroup md doc check#781
Merged
Merged
Conversation
Address the "Check doc quality" reviewer flags on subgroup.md: define IR at first use, and drop internal C++ symbols, the fe-ll cache key, the segmented_reduce bitmask-path walkthrough, and the ballot_first_n LLVM isel-bug rationale from user-facing sections, keeping the user-observable behaviour and cost.
Give shuffle / shuffle_up / shuffle_down / broadcast / invocation_id / sync / mem_fence proper docstrings (behaviour, dtype rules, backend lowering, caller contract), so per-method documentation lives on the function itself and is rendered by autoapi. Preparatory step for slimming subgroup.md down to a high-level overview.
Move the per-method reference detail (semantics, backend lowering, caller contracts) into the function docstrings, where it renders in the autoapi API reference. Rewrite subgroup.md as a high-level tour: capability tables, the tiled-variant model, subgroup-size table, examples, and performance notes, with a pointer to the generated API reference. Drops the deep AMDGPU wave64 cross-half ISA walkthrough from user-facing docs (it lives in the runtime.cpp code comments).
Doc-quality review flagged PTX, RDNA/CDNA, LDS, permlane64, ds_permute, and "trace/codegen time" as undefined terms for the end-user audience. Reword the NaN note to use the CUDA backend name, simplify the AMDGPU shuffle performance bullet to cycle-level guidance, and describe compile-time failures in plain language. ds_bpermute stays (glossed at first use).
Collaborator
Author
|
@codex review |
hughperkins
added
awaiting-ci-to-pass
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
…uage CI doc-quality review (stricter than expected) flagged "lowering"/"lower" as compiler jargon and rejected ds_bpermute even when glossed. Replace "lower" with "compile to" / "support", describe AMDGPU shuffles as "a slower cross-lane path" instead of naming ds_bpermute, drop the CUDA __shfl_* intrinsic names, and reword "butterfly variant" to name the op directly.
hughperkins
added
awaiting review pass
Contributor
|
ok to merge |
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
2 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.
Issue: #
Brief Summary
... and also address #746 (comment)
copilot:summary
Walkthrough
copilot:walkthrough