Skip to content

[Refactor] Align RMSNorm with current FlyDSL kernel style#902

Open
jhinpan wants to merge 4 commits into
ROCm:mainfrom
jhinpan:perf/rmsnorm-fx-cleanup
Open

[Refactor] Align RMSNorm with current FlyDSL kernel style#902
jhinpan wants to merge 4 commits into
ROCm:mainfrom
jhinpan:perf/rmsnorm-fx-cleanup

Conversation

@jhinpan

@jhinpan jhinpan commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

This is a behavior-preserving cleanup of the RMSNorm forward/backward implementation using the repository's kernel-code-cleanup guidance.

  • align plain and fused forward launch caching with the shared _run_compiled / raw-stream pattern already used by maintained FlyDSL kernels
  • remove redundant fx.Int32 / fx.Float32 wrapping and use Python literals where the typed operand already determines the runtime type
  • replace legacy full(...) calls with the canonical fx.Vector.filled(...) surface and remove duplicate store-dtype plumbing from the RMSNorm scalar-store helper
  • route quantization-scale writes through that shared scalar-store helper instead of maintaining an equivalent local implementation
  • replace stale legacy terminology and verbose section banners with concise comments that preserve only path choices and invariants
  • retain the tuned backward atomic/two-stage split and genuine single-atom copy helpers; those are intentional lower-level primitives, not legacy migration targets

The goal is coding consistency and maintainability rather than a shape-specific speedup. Kernel math, memory layouts, and backward dispatch policy are unchanged.

Test plan

  • bash scripts/check_python_style.sh --base upstream/main
  • HIP_VISIBLE_DEVICES=7 FLYDSL_RUNTIME_ENABLE_CACHE=0 python3 -m pytest tests/kernels/test_rmsnorm.py -q -m 'not benchmark and not large_shape and not multi_gpu' (30 passed, 6 deselected)
  • HIP_VISIBLE_DEVICES=6,7 FLYDSL_RUNTIME_ENABLE_CACHE=0 python3 -m pytest tests/kernels/test_rmsnorm.py::test_rmsnorm_multi_gpu tests/kernels/test_rmsnorm.py::test_fused_add_rmsnorm_device_mismatch -q (2 passed)
  • plain and fused forward launcher-reuse regression tests (2 passed)

jhinpan and others added 2 commits July 24, 2026 19:04
Cache JIT launchers so plain and fused forward hot paths can use the existing raw-stream fast dispatcher without rebuilding stream wrappers or device guards.

Co-authored-by: Cursor <[email protected]>
Keep only path and invariant comments so the forward and backward implementations reflect the current layout APIs and dispatch behavior.

Co-authored-by: Cursor <[email protected]>
Copilot AI review requested due to automatic review settings July 24, 2026 20:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@jhinpan jhinpan changed the title [Perf] Reduce RMSNorm forward launch overhead [Refactor] Align RMSNorm with current FlyDSL kernel style Jul 24, 2026
Remove redundant numeric wrappers and duplicate store dtype plumbing, and use the canonical Vector.filled surface throughout the RMSNorm kernels.

Co-authored-by: Cursor <[email protected]>
@jhinpan
jhinpan requested review from coderfeli and zhiding512 July 24, 2026 20:21
Route quantization scale writes through the shared typed scalar-store primitive instead of maintaining an equivalent local implementation.

Co-authored-by: Cursor <[email protected]>
store_rstd=store_rstd,
eps=eps,
weight_dtype_str=weight_dtype_str,
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why remove flyc.compile

compiled(x, residual, weight, out, residual_out, rstd, M, stream)
else:
compiled(x, residual, weight, out, residual_out, M, stream)
launcher = _get_fused_add_fwd_launcher(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why remove flyc.compile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants