[llvm] [AMDGPU] Fix waterfall inreg call args on gfx90a: hoist defs + avoid AGPR sources (PR #194890)
Amit Tiwari via llvm-commits
llvm-commits at lists.llvm.org
Fri May 1 12:51:14 PDT 2026
================
@@ -7828,6 +7851,38 @@ void SIInstrInfo::createWaterFallForSiCall(MachineInstr *MI,
MI->definesRegister(End->getOperand(1).getReg(), &RI))
++End;
+ MachineRegisterInfo &MRI = MBB.getParent()->getRegInfo();
----------------
amitamd7 wrote:
Explaining the entire case so that we both are on the same page. On waterfall-call-inreg-agpr.ll, the problematic COPYs are inserted by AMDGPU instruction selection itself. Right after `amdgpu-isel` and before `si-fix-sgpr-copies` (which is where `createWaterFallForSiCall` lives), the IR already looks like:
```
bb.0:
%25:av_64_align2 = FLAT_LOAD_DWORDX2 ...
%27:av_32 = COPY %25.sub1
%30:av_64_align2 = FLAT_LOAD_DWORDX2 ...
%32:av_32 = COPY %30.sub1
%35:av_64_align2 = FLAT_LOAD_DWORDX2 ...
%37:av_32 = COPY %35.sub1
...
ADJCALLSTACKUP 0, 0, ...
%46:av_32 = COPY %35.sub0
%47:av_32 = COPY %30.sub0
%48:av_32 = COPY %25.sub0
...
ADJCALLSTACKDOWN ...
```
We load three 64-bit pointer pairs: %25, %30, %35 before the call. When these are split into 32-bit halves:
The .sub1 copies are placed before ADJCALLSTACKUP.
The .sub0 copies end up between ADJCALLSTACKUP and ADJCALLSTACKDOWN (inside call frame).
This breaks later even though both halves come from values loaded before the call. Later, waterfall change splices [ADJCALLSTACKUP, ADJCALLSTACKDOWN) into BodyBB, but the `readfirstlane for these operands is emitted in LoopBB. Now, those instructions in LoopBB try to use the .sub0 values before they’re defined (because their defs got moved into BodyBB).
With the only single change (hoisting disabled):
```
`$ llc -mtriple=amdgcn -mcpu=gfx90a -verify-machineinstrs \
< llvm/test/CodeGen/AMDGPU/waterfall-call-inreg-agpr.ll
*** Bad machine code: Virtual register defs don't dominate all uses. ***
- function: chain_preserve_caller
- v. register: %46
*** Bad machine code: Virtual register defs don't dominate all uses. ***
- function: chain_preserve_caller
- v. register: %48
LLVM ERROR: Found 2 machine code errors. (Aborted, exit 134)
```
Here, %46 %48 is what the minimal patch hoists to preserve breaking; causing zero regressions. Fixing it in isel would mean changing `LowerCall` for the chain-preserve family -- much wider change and outside the scope of this PR. Happy to file a follow-up issue related to the isel side change.
https://github.com/llvm/llvm-project/pull/194890
More information about the llvm-commits
mailing list