Skip to content

Commit 6cb509c

Browse files
[AMDGPU] Fix Hot Block Register Renaming assertions on complex IR
Fix two assertions discovered during CI/CT testing with rocBLAS kernels: 1. isVirtRegMovable() crashed on PHI nodes with multiple value definitions. Converted assertions to early-return checks, allowing the pass to skip unmovable registers instead of crashing on legitimate IR patterns. 2. tryMoveValue() assumed LiveIntervalUnion contains only virtual registers, but it can contain physical registers after allocation. Added isVirtual() check before calling VirtRegMap::getPhys() to prevent assertion failures. Both fixes improve robustness without affecting correctness or performance.
1 parent f8a7aa6 commit 6cb509c

File tree

1 file changed

+23
-4
lines changed

1 file changed

+23
-4
lines changed

llvm/lib/Target/AMDGPU/AMDGPUHotBlockRegisterRenaming.cpp

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -377,10 +377,21 @@ bool AMDGPUHotBlockRegisterRenamingImpl::isVirtRegMovable(Register VirtReg,
377377
if (S.start >= BBStart && S.end <= BBEnd)
378378
SegmentCount++;
379379
}
380-
assert(SegmentCount == 1 &&
381-
"isVirtRegMovable expects VirtReg with single segment in BB");
382-
assert(VirtRegLI.getNumValNums() == 1 &&
383-
"isVirtRegMovable expects VirtReg with single value");
380+
381+
// Cannot move registers with multiple segments in BB (e.g., PHI nodes)
382+
if (SegmentCount != 1) {
383+
LLVM_DEBUG(dbgs() << " Cannot move " << printReg(VirtReg, TRI)
384+
<< ": has " << SegmentCount << " segments in BB\n");
385+
return false;
386+
}
387+
388+
// Cannot move registers with multiple definitions (e.g., from PHI merge)
389+
if (VirtRegLI.getNumValNums() != 1) {
390+
LLVM_DEBUG(dbgs() << " Cannot move " << printReg(VirtReg, TRI)
391+
<< ": has " << VirtRegLI.getNumValNums()
392+
<< " value definitions\n");
393+
return false;
394+
}
384395

385396
// Check for tied operands
386397
// A tied operand means the instruction requires source and destination to be
@@ -451,6 +462,14 @@ bool AMDGPUHotBlockRegisterRenamingImpl::tryMoveValue(MCRegister DenseReg,
451462
for (LiveIntervalUnion::SegmentIter SI = LIU.begin(); SI.valid(); ++SI) {
452463
Register VirtReg = SI.value()->reg();
453464

465+
// Skip physical registers (LiveIntervalUnion can contain both)
466+
if (!VirtReg.isVirtual())
467+
continue;
468+
469+
// Skip virtual registers that haven't been allocated yet
470+
if (!VRM->hasPhys(VirtReg))
471+
continue;
472+
454473
// Check if this VirtReg is mapped to DenseReg
455474
// NOTE: This is NOT redundant! We iterate per register unit, and units
456475
// can be shared between aliased registers (e.g., VGPR0 and VGPR0_VGPR1).

0 commit comments

Comments
 (0)