From 9b21b0268a85711946145817e05f6581964ceeac Mon Sep 17 00:00:00 2001 From: Philip Reames Date: Mon, 6 Oct 2025 08:28:49 -0700 Subject: [PATCH 1/2] [LRE] Adjust order of cases in eliminateDeadDefs If we have a rematerializable instruction without live virtual register uses (that we didn't heuristically decide to shrink), but with a physreg use, we were creating a kill to keep the physreg operand liveness unchanged. This meant we *weren't* keeping around the remat instruction, and thus inhibiting future remat within the original live interval. If we keep the remat instruction, that *also* keeps the physreg use, and thus we can achieve both objectives at once. Noticed this via inspection, and we don't currently seem to have any rematerializable instructions which could observe the difference. This could in theory happen for both trivial and non-trivial remat, but requires the rematerialization of an instruction with a physreg use. --- llvm/lib/CodeGen/LiveRangeEdit.cpp | 73 +++++++++++++++--------------- 1 file changed, 37 insertions(+), 36 deletions(-) diff --git a/llvm/lib/CodeGen/LiveRangeEdit.cpp b/llvm/lib/CodeGen/LiveRangeEdit.cpp index 59bc82dc267b5..b2474c4c29bc8 100644 --- a/llvm/lib/CodeGen/LiveRangeEdit.cpp +++ b/llvm/lib/CodeGen/LiveRangeEdit.cpp @@ -303,6 +303,37 @@ void LiveRangeEdit::eliminateDeadDef(MachineInstr *MI, ToShrinkSet &ToShrink) { } } + // If the dest of MI is an original reg and MI is reMaterializable, + // don't delete the inst. Replace the dest with a new reg, and keep + // the inst for remat of other siblings. The inst is saved in + // LiveRangeEdit::DeadRemats and will be deleted after all the + // allocations of the func are done. Note that if we keep the + // instruction with the original operands, that handles the physreg + // operand case (described just below) as well. + // However, immediately delete instructions which have unshrunk virtual + // register uses. That may provoke RA to split an interval at the KILL + // and later result in an invalid live segment end. + if (isOrigDef && DeadRemats && !HasLiveVRegUses && + TII.isReMaterializable(*MI)) { + LiveInterval &NewLI = createEmptyIntervalFrom(Dest, false); + VNInfo::Allocator &Alloc = LIS.getVNInfoAllocator(); + VNInfo *VNI = NewLI.getNextValue(Idx, Alloc); + NewLI.addSegment(LiveInterval::Segment(Idx, Idx.getDeadSlot(), VNI)); + + if (DestSubReg) { + const TargetRegisterInfo *TRI = MRI.getTargetRegisterInfo(); + auto *SR = NewLI.createSubRange( + Alloc, TRI->getSubRegIndexLaneMask(DestSubReg)); + SR->addSegment(LiveInterval::Segment(Idx, Idx.getDeadSlot(), + SR->getNextValue(Idx, Alloc))); + } + + pop_back(); + DeadRemats->insert(MI); + const TargetRegisterInfo &TRI = *MRI.getTargetRegisterInfo(); + MI->substituteRegister(Dest, NewLI.reg(), 0, TRI); + assert(MI->registerDefIsDead(NewLI.reg(), &TRI)); + } // Currently, we don't support DCE of physreg live ranges. If MI reads // any unreserved physregs, don't erase the instruction, but turn it into // a KILL instead. This way, the physreg live ranges don't end up @@ -310,7 +341,7 @@ void LiveRangeEdit::eliminateDeadDef(MachineInstr *MI, ToShrinkSet &ToShrink) { // FIXME: It would be better to have something like shrinkToUses() for // physregs. That could potentially enable more DCE and it would free up // the physreg. It would not happen often, though. - if (ReadsPhysRegs) { + else if (ReadsPhysRegs) { MI->setDesc(TII.get(TargetOpcode::KILL)); // Remove all operands that aren't physregs. for (unsigned i = MI->getNumOperands(); i; --i) { @@ -322,41 +353,11 @@ void LiveRangeEdit::eliminateDeadDef(MachineInstr *MI, ToShrinkSet &ToShrink) { MI->dropMemRefs(*MI->getMF()); LLVM_DEBUG(dbgs() << "Converted physregs to:\t" << *MI); } else { - // If the dest of MI is an original reg and MI is reMaterializable, - // don't delete the inst. Replace the dest with a new reg, and keep - // the inst for remat of other siblings. The inst is saved in - // LiveRangeEdit::DeadRemats and will be deleted after all the - // allocations of the func are done. - // However, immediately delete instructions which have unshrunk virtual - // register uses. That may provoke RA to split an interval at the KILL - // and later result in an invalid live segment end. - if (isOrigDef && DeadRemats && !HasLiveVRegUses && - TII.isReMaterializable(*MI)) { - LiveInterval &NewLI = createEmptyIntervalFrom(Dest, false); - VNInfo::Allocator &Alloc = LIS.getVNInfoAllocator(); - VNInfo *VNI = NewLI.getNextValue(Idx, Alloc); - NewLI.addSegment(LiveInterval::Segment(Idx, Idx.getDeadSlot(), VNI)); - - if (DestSubReg) { - const TargetRegisterInfo *TRI = MRI.getTargetRegisterInfo(); - auto *SR = NewLI.createSubRange( - Alloc, TRI->getSubRegIndexLaneMask(DestSubReg)); - SR->addSegment(LiveInterval::Segment(Idx, Idx.getDeadSlot(), - SR->getNextValue(Idx, Alloc))); - } - - pop_back(); - DeadRemats->insert(MI); - const TargetRegisterInfo &TRI = *MRI.getTargetRegisterInfo(); - MI->substituteRegister(Dest, NewLI.reg(), 0, TRI); - assert(MI->registerDefIsDead(NewLI.reg(), &TRI)); - } else { - if (TheDelegate) - TheDelegate->LRE_WillEraseInstruction(MI); - LIS.RemoveMachineInstrFromMaps(*MI); - MI->eraseFromParent(); - ++NumDCEDeleted; - } + if (TheDelegate) + TheDelegate->LRE_WillEraseInstruction(MI); + LIS.RemoveMachineInstrFromMaps(*MI); + MI->eraseFromParent(); + ++NumDCEDeleted; } // Erase any virtregs that are now empty and unused. There may be From 30a741adf078415767123872ce99b4f6bb350882 Mon Sep 17 00:00:00 2001 From: Philip Reames Date: Mon, 6 Oct 2025 10:09:24 -0700 Subject: [PATCH 2/2] clang-format --- llvm/lib/CodeGen/LiveRangeEdit.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/lib/CodeGen/LiveRangeEdit.cpp b/llvm/lib/CodeGen/LiveRangeEdit.cpp index b2474c4c29bc8..e79dafef8e0ea 100644 --- a/llvm/lib/CodeGen/LiveRangeEdit.cpp +++ b/llvm/lib/CodeGen/LiveRangeEdit.cpp @@ -322,8 +322,8 @@ void LiveRangeEdit::eliminateDeadDef(MachineInstr *MI, ToShrinkSet &ToShrink) { if (DestSubReg) { const TargetRegisterInfo *TRI = MRI.getTargetRegisterInfo(); - auto *SR = NewLI.createSubRange( - Alloc, TRI->getSubRegIndexLaneMask(DestSubReg)); + auto *SR = + NewLI.createSubRange(Alloc, TRI->getSubRegIndexLaneMask(DestSubReg)); SR->addSegment(LiveInterval::Segment(Idx, Idx.getDeadSlot(), SR->getNextValue(Idx, Alloc))); }