Skip to content

Commit 16814dc

Browse files
committed
[M68k] Fix COPY instruction killing live condition flags
During PHI node elimination, a COPY instruction is inserted that is not known to kill condition flags until after it is lowered by `copyPhysReg()`. To mitigate this, `copyPhysReg()` now has logic to determine whether the CCR is live, and if so, will back it up and restore it after the COPY. I've only seen this occur due to PHI elimination, but if the root cause is that LLVM assumes COPY has no side-effects, then perhaps this would show up elsewhere as well.
1 parent d7f7e1d commit 16814dc

File tree

1 file changed

+58
-28
lines changed

1 file changed

+58
-28
lines changed

llvm/lib/Target/M68k/M68kInstrInfo.cpp

Lines changed: 58 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,6 @@ bool M68kInstrInfo::ExpandCCR(MachineInstrBuilder &MIB, bool IsToCCR) const {
584584
if (IsToCCR)
585585
MIB->setDesc(get(M68k::MOV16cd));
586586
else
587-
// FIXME M68010 or later is required
588587
MIB->setDesc(get(M68k::MOV16dc));
589588

590589
return true;
@@ -718,20 +717,13 @@ void M68kInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
718717
else if (M68k::DR8RegClass.contains(DstReg, SrcReg))
719718
Opc = M68k::MOV8dd;
720719

721-
if (Opc) {
722-
BuildMI(MBB, MI, DL, get(Opc), DstReg)
723-
.addReg(SrcReg, getKillRegState(KillSrc));
724-
return;
725-
}
726-
727720
// Now deal with asymmetrically sized copies. The cases that follow are upcast
728721
// moves.
729722
//
730723
// NOTE
731724
// These moves are not aware of type nature of these values and thus
732725
// won't do any SExt or ZExt and upper bits will basically contain garbage.
733-
MachineInstrBuilder MIB(*MBB.getParent(), MI);
734-
if (M68k::DR8RegClass.contains(SrcReg)) {
726+
else if (M68k::DR8RegClass.contains(SrcReg)) {
735727
if (M68k::XR16RegClass.contains(DstReg))
736728
Opc = M68k::MOVXd16d8;
737729
else if (M68k::XR32RegClass.contains(DstReg))
@@ -740,18 +732,7 @@ void M68kInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
740732
M68k::XR32RegClass.contains(DstReg))
741733
Opc = M68k::MOVXd32d16;
742734

743-
if (Opc) {
744-
BuildMI(MBB, MI, DL, get(Opc), DstReg)
745-
.addReg(SrcReg, getKillRegState(KillSrc));
746-
return;
747-
}
748-
749-
bool FromCCR = SrcReg == M68k::CCR;
750-
bool FromSR = SrcReg == M68k::SR;
751-
bool ToCCR = DstReg == M68k::CCR;
752-
bool ToSR = DstReg == M68k::SR;
753-
754-
if (FromCCR) {
735+
else if (SrcReg == M68k::CCR) {
755736
if (M68k::DR8RegClass.contains(DstReg)) {
756737
Opc = M68k::MOV8dc;
757738
} else if (M68k::DR16RegClass.contains(DstReg)) {
@@ -762,7 +743,7 @@ void M68kInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
762743
LLVM_DEBUG(dbgs() << "Cannot copy CCR to " << RI.getName(DstReg) << '\n');
763744
llvm_unreachable("Invalid register for MOVE from CCR");
764745
}
765-
} else if (ToCCR) {
746+
} else if (DstReg == M68k::CCR) {
766747
if (M68k::DR8RegClass.contains(SrcReg)) {
767748
Opc = M68k::MOV8cd;
768749
} else if (M68k::DR16RegClass.contains(SrcReg)) {
@@ -773,18 +754,67 @@ void M68kInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
773754
LLVM_DEBUG(dbgs() << "Cannot copy " << RI.getName(SrcReg) << " to CCR\n");
774755
llvm_unreachable("Invalid register for MOVE to CCR");
775756
}
776-
} else if (FromSR || ToSR)
757+
} else if (SrcReg == M68k::SR || DstReg == M68k::SR)
777758
llvm_unreachable("Cannot emit SR copy instruction");
778759

779-
if (Opc) {
760+
if (!Opc) {
761+
LLVM_DEBUG(dbgs() << "Cannot copy " << RI.getName(SrcReg) << " to "
762+
<< RI.getName(DstReg) << '\n');
763+
llvm_unreachable("Cannot emit physreg copy instruction");
764+
}
765+
766+
// Create the final MOVE instruction without yet inserting
767+
MachineFunction &MF = *MBB.getParent();
768+
// MachineInstrBuilder MoveMIB = BuildMI(MF, DL, get(Opc), DstReg)
769+
// .addReg(SrcReg, getKillRegState(KillSrc));
770+
771+
// Get the live registers right before the COPY instruction. If CCR is
772+
// live, the MOVE is going to kill it, so we will need to preserve it.
773+
MachineRegisterInfo &MRI = MF.getRegInfo();
774+
LivePhysRegs UsedRegs(RI);
775+
UsedRegs.addLiveOuts(MBB);
776+
auto InstUpToI = MBB.end();
777+
while (InstUpToI != MI)
778+
// The pre-decrement is on purpose here.
779+
// We want to have the liveness right before I.
780+
UsedRegs.stepBackward(*--InstUpToI);
781+
782+
if (UsedRegs.available(MRI, M68k::CCR)) {
780783
BuildMI(MBB, MI, DL, get(Opc), DstReg)
781-
.addReg(SrcReg, getKillRegState(KillSrc));
784+
.addReg(SrcReg, getKillRegState(KillSrc));
782785
return;
783786
}
784787

785-
LLVM_DEBUG(dbgs() << "Cannot copy " << RI.getName(SrcReg) << " to "
786-
<< RI.getName(DstReg) << '\n');
787-
llvm_unreachable("Cannot emit physreg copy instruction");
788+
// CCR is live, so we must restore it after the copy. Prepare push/pop ops.
789+
// 68000 must use MOVE from SR, 68010+ must use MOVE from CCR. In either
790+
// case, MOVE to CCR masks out the upper byte.
791+
const M68kSubtarget &STI = MF.getSubtarget<M68kSubtarget>();
792+
793+
// Look for an available data register for the CCR, or push to stack if
794+
// there are none
795+
BitVector Allocatable = RI.getAllocatableSet(
796+
MF, RI.getRegClass(M68k::DR16RegClassID));
797+
for (Register Reg : Allocatable.set_bits()) {
798+
if (!RI.regsOverlap(DstReg, Reg) &&
799+
(UsedRegs.available(MRI, Reg) || !UsedRegs.contains(Reg))) {
800+
unsigned CCRPushOp = STI.isM68000() ? M68k::MOV16ds : M68k::MOV16dc;
801+
unsigned CCRPopOp = M68k::MOV16cd;
802+
BuildMI(MBB, MI, DL, get(CCRPushOp), Reg);
803+
BuildMI(MBB, MI, DL, get(Opc), DstReg)
804+
.addReg(SrcReg, getKillRegState(KillSrc));
805+
BuildMI(MBB, MI, DL, get(CCRPopOp)).addReg(Reg);
806+
return;
807+
}
808+
}
809+
810+
unsigned CCRPushOp = STI.isM68000() ? M68k::MOV16es : M68k::MOV16ec;
811+
unsigned CCRPopOp = M68k::MOV16co;
812+
813+
BuildMI(MBB, MI, DL, get(CCRPushOp)).addReg(RI.getStackRegister());
814+
BuildMI(MBB, MI, DL, get(Opc), DstReg)
815+
.addReg(SrcReg, getKillRegState(KillSrc));
816+
BuildMI(MBB, MI, DL, get(CCRPopOp)).addReg(RI.getStackRegister());
817+
return;
788818
}
789819

790820
namespace {

0 commit comments

Comments
 (0)