Skip to content

[EXTERNAL][AMDGPU] Fold constant offset into SV form addressing in SVS - #2435

Open
bogdan-petkovic wants to merge 6 commits into
developfrom
users/bpetkovi/external-amdgpu-svs-fi-fix
Open

[EXTERNAL][AMDGPU] Fold constant offset into SV form addressing in SVS#2435
bogdan-petkovic wants to merge 6 commits into
developfrom
users/bpetkovi/external-amdgpu-svs-fi-fix

Conversation

@bogdan-petkovic

@bogdan-petkovic bogdan-petkovic commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Motivation

Padded attention kernels that use the double-buffer direct-to-LDS schedule produce NaNs on gfx950. The root cause is an AMDGPU backend miscompile, not an MLIR codegen issue. This PR backports the upstream LLVM fix so those configurations are numerically correct on gfx950 and no MLIR-side workaround (gating double-buffer to single-buffer when padding is present) is needed.

Technical Details

Clean backport of two upstream llvm/llvm-project commits (external subtree only):

  • [AMDGPU] Fold constant offset into SV form addressing in SVS frame index fallback llvm/llvm-project#2086295f371b2add3e [AMDGPU] Fold constant offset into SV form addressing in SVS (the fix)
  • [NFC][AMDGPU] Simplify scratch address materialization in SVS frame index fallback llvm/llvm-project#2086246ce86d47ca09 [NFC][AMDGPU] Simplify scratch address materialization in SVS (prerequisite refactor + test)
    Under extreme VGPR pressure (~512), the double-buffer direct-to-LDS schedule hit a bad frame-index elimination for the flat-scratch SVS (scalar+vector+scalar) addressing form: the constant offset was not correctly folded/materialized into the scavenged vaddr VGPR, producing wrong scratch addresses and NaNs. The fix folds as much of the constant offset as possible into the instruction's immediate offset field and materializes the remainder (plus the frame register, if any) into the scavenged VGPR.
    Files touched: external/llvm-project/llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp and the added upstream regression test external/llvm-project/llvm/test/CodeGen/AMDGPU/eliminate-frame-index-flat-scratch-svs.mir.

Test Plan

Built a standalone O0-vs-O3 reproducer of the failing gfx950 padded-attention kernel (extracted LLVM IR + HIP harness), run against the June LLVM bump with and without this patch. The upstream .mir regression test (eliminate-frame-index-flat-scratch-svs.mir) is also included and runs as part of the LLVM test suite.

Test Result

  • Without the patch the O3 build diverges from the O0 baseline and emits NaNs (FAIL). With the patch applied, O3 matches O0 and the reproducer passes 3/3. The added upstream .mir test passes.
  • Weekly CI (parameterSweeps)
  • Nightly CI
  • PR CI

Submission Checklist

@bogdan-petkovic bogdan-petkovic self-assigned this Jul 23, 2026
@bogdan-petkovic
bogdan-petkovic requested a review from causten as a code owner July 23, 2026 14:15
@bogdan-petkovic
bogdan-petkovic force-pushed the users/bpetkovi/external-amdgpu-svs-fi-fix branch from ec1955f to 55e56d1 Compare July 24, 2026 07:05
Backport of two upstream llvm/llvm-project commits to fix an AMDGPU
frame-index elimination miscompile that shows up on gfx950:

  * 5f371b2add3e [AMDGPU] Fold constant offset into SV form addressing
    in SVS  (llvm/llvm-project#208629)
  * 6ce86d47ca09 [NFC][AMDGPU] Simplify scratch address materialization
    in SVS  (llvm/llvm-project#208624)

Under extreme VGPR pressure (~512) the double-buffer direct-to-LDS
schedule used by padded attention kernels hit a bad frame-index
elimination for the flat-scratch SVS (scalar+vector+scalar) addressing
form: the constant offset was not correctly folded/materialized into the
scavenged vaddr VGPR, producing wrong scratch addresses and NaNs in the
kernel output.

The fix folds as much of the constant offset as possible into the
instruction's immediate offset field and materializes the remainder
(plus the frame register, if any) into the scavenged VGPR. The upstream
regression test (eliminate-frame-index-flat-scratch-svs.mir) is included.

Adapted to develop's LLVM base: the flat variant is passed as
SIInstrFlags::FlatScratch (upstream later renamed this to
AMDGPU::FlatAddrSpace::FlatScratch).

Verified against the June LLVM bump: an O0-vs-O3 reproducer of the
gfx950 padded-attention kernel goes from FAIL to PASS (3/3) with this
patch applied.
@bogdan-petkovic
bogdan-petkovic force-pushed the users/bpetkovi/external-amdgpu-svs-fi-fix branch from 55e56d1 to bfff468 Compare July 24, 2026 07:54
@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2435      +/-   ##
===========================================
+ Coverage    82.57%   83.65%   +1.08%     
===========================================
  Files          120      120              
  Lines        42852    42933      +81     
  Branches      7110     7139      +29     
===========================================
+ Hits         35381    35912     +531     
+ Misses        4815     4487     -328     
+ Partials      2656     2534     -122     
Flag Coverage Δ
gfx120x 83.46% <ø> (+0.94%) ⬆️
gfx950 83.36% <ø> (+1.02%) ⬆️
mfma 83.37% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.
see 33 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@umangyadav

umangyadav commented Jul 24, 2026

Copy link
Copy Markdown
Member

Make sure to run nightly CI too

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.

2 participants