[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. [M68k] Fix CCR edge cases in `copyPhysReg()`

Author: dansalvato

llvm/lib/Target/M68k/M68kInstrData.td

@@ -372,7 +372,6 @@ def MOVM32mp_P : MxMOVEM_RM_Pseudo<MxType32r, MxType32.POp>;
 /// | EFFECTIVE ADDRESS
 /// 0 1 0 0 0 1 0 0 1 1 | MODE | REG
 /// --------------------------------------------------
-let Defs = [CCR] in {
 class MxMoveToCCR<MxOperand MEMOp, MxEncMemOp SRC_ENC>
  : MxInst<(outs CCRC:$dst), (ins MEMOp:$src), "move.w\t$src, $dst", []> {
  let Inst = (ascend
@@ -383,7 +382,6 @@ class MxMoveToCCR<MxOperand MEMOp, MxEncMemOp SRC_ENC>
 
 class MxMoveToCCRPseudo<MxOperand MEMOp>
  : MxPseudo<(outs CCRC:$dst), (ins MEMOp:$src)>;
-} // let Defs = [CCR]
 
 let mayLoad = 1 in
 foreach AM = MxMoveSupportedAMs in {
@@ -403,7 +401,6 @@ def MOV8cd : MxMoveToCCRPseudo<MxOp8AddrMode_d.Op>;
 /// | EFFECTIVE ADDRESS
 /// 0 1 0 0 0 0 1 0 1 1 | MODE | REG
 /// --------------------------------------------------
-let Uses = [CCR] in {
 class MxMoveFromCCR_R
  : MxInst<(outs MxDRD16:$dst), (ins CCRC:$src), "move.w\t$src, $dst", []>,
  Requires<[ AtLeastM68010 ]> {
@@ -423,7 +420,6 @@ class MxMoveFromCCRPseudo<MxOperand MEMOp>
  : MxPseudo<(outs), (ins MEMOp:$dst, CCRC:$src)>;
 class MxMoveFromCCR_RPseudo<MxOperand MEMOp>
  : MxPseudo<(outs MEMOp:$dst), (ins CCRC:$src)>;
-} // let Uses = [CCR]
 
 let mayStore = 1 in
 foreach AM = MxMoveSupportedAMs in {
@@ -445,15 +441,13 @@ def MOV8dc : MxMoveFromCCR_RPseudo<MxOp8AddrMode_d.Op>;
 /// | EFFECTIVE ADDRESS
 /// 0 1 0 0 0 1 1 0 1 1 | MODE | REG
 /// --------------------------------------------------
-let Defs = [SR] in {
 class MxMoveToSR<MxOperand MEMOp, MxEncMemOp SRC_ENC>
  : MxInst<(outs SRC:$dst), (ins MEMOp:$src), "move.w\t$src, $dst", []> {
  let Inst = (ascend
  (descend 0b0100011011, SRC_ENC.EA),
  SRC_ENC.Supplement
  );
 }
-} // let Defs = [SR]
 
 let mayLoad = 1 in
 foreach AM = MxMoveSupportedAMs in {
@@ -470,22 +464,20 @@ def MOV16sd : MxMoveToSR<MxOp16AddrMode_d.Op, MxMoveSrcOpEnc_d>;
 /// | EFFECTIVE ADDRESS
 /// 0 1 0 0 0 0 0 0 1 1 | MODE | REG
 /// --------------------------------------------------
-let Uses = [SR] in {
 class MxMoveFromSR_R
  : MxInst<(outs MxDRD16:$dst), (ins SRC:$src), "move.w\t$src, $dst", []>,
- Requires<[ AtLeastM68010 ]> {
+ Requires<[ IsM68000 ]> {
  let Inst = (descend 0b0100000011, MxEncAddrMode_d<"dst">.EA);
 }
 
 class MxMoveFromSR_M<MxOperand MEMOp, MxEncMemOp DST_ENC>
  : MxInst<(outs), (ins MEMOp:$dst, SRC:$src), "move.w\t$src, $dst", []>,
- Requires<[ AtLeastM68010 ]> {
+ Requires<[ IsM68000 ]> {
  let Inst = (ascend
  (descend 0b0100000011, DST_ENC.EA),
  DST_ENC.Supplement
  );
 }
-} // let Uses = [SR]
 
 let mayStore = 1 in
 foreach AM = MxMoveSupportedAMs in {

llvm/lib/Target/M68k/M68kInstrInfo.cpp

@@ -583,9 +583,11 @@ bool M68kInstrInfo::ExpandCCR(MachineInstrBuilder &MIB, bool IsToCCR) const {
  // Replace the pseudo instruction with the real one
  if (IsToCCR)
  MIB->setDesc(get(M68k::MOV16cd));
- else
- // FIXME M68010 or later is required
+ else if (MIB->getParent()->getParent()->getSubtarget<M68kSubtarget>()
+  .atLeastM68010())
  MIB->setDesc(get(M68k::MOV16dc));
+ else
+ MIB->setDesc(get(M68k::MOV16ds));
 
  return true;
 }
@@ -709,6 +711,8 @@ void M68kInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
  Register SrcReg, bool KillSrc,
  bool RenamableDest, bool RenamableSrc) const {
  unsigned Opc = 0;
+ MachineFunction &MF = *MBB.getParent();
+ const M68kSubtarget &STI = MF.getSubtarget<M68kSubtarget>();
 
  // First deal with the normal symmetric copies.
  if (M68k::XR32RegClass.contains(DstReg, SrcReg))
@@ -718,20 +722,13 @@ void M68kInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
  else if (M68k::DR8RegClass.contains(DstReg, SrcReg))
  Opc = M68k::MOV8dd;
 
- if (Opc) {
- BuildMI(MBB, MI, DL, get(Opc), DstReg)
- .addReg(SrcReg, getKillRegState(KillSrc));
- return;
- }
-
  // Now deal with asymmetrically sized copies. The cases that follow are upcast
  // moves.
  //
  // NOTE
  // These moves are not aware of type nature of these values and thus
  // won't do any SExt or ZExt and upper bits will basically contain garbage.
- MachineInstrBuilder MIB(*MBB.getParent(), MI);
- if (M68k::DR8RegClass.contains(SrcReg)) {
+ else if (M68k::DR8RegClass.contains(SrcReg)) {
  if (M68k::XR16RegClass.contains(DstReg))
  Opc = M68k::MOVXd16d8;
  else if (M68k::XR32RegClass.contains(DstReg))
@@ -740,29 +737,18 @@ void M68kInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
  M68k::XR32RegClass.contains(DstReg))
  Opc = M68k::MOVXd32d16;
 
- if (Opc) {
- BuildMI(MBB, MI, DL, get(Opc), DstReg)
- .addReg(SrcReg, getKillRegState(KillSrc));
- return;
- }
-
- bool FromCCR = SrcReg == M68k::CCR;
- bool FromSR = SrcReg == M68k::SR;
- bool ToCCR = DstReg == M68k::CCR;
- bool ToSR = DstReg == M68k::SR;
-
- if (FromCCR) {
+ else if (SrcReg == M68k::CCR) {
  if (M68k::DR8RegClass.contains(DstReg)) {
  Opc = M68k::MOV8dc;
  } else if (M68k::DR16RegClass.contains(DstReg)) {
- Opc = M68k::MOV16dc;
+ Opc = STI.isM68000() ? M68k::MOV16ds : M68k::MOV16dc;
  } else if (M68k::DR32RegClass.contains(DstReg)) {
- Opc = M68k::MOV16dc;
+ Opc = STI.isM68000() ? M68k::MOV16ds : M68k::MOV16dc;
  } else {
  LLVM_DEBUG(dbgs() << "Cannot copy CCR to " << RI.getName(DstReg) << '\n');
  llvm_unreachable("Invalid register for MOVE from CCR");
  }
- } else if (ToCCR) {
+ } else if (DstReg == M68k::CCR) {
  if (M68k::DR8RegClass.contains(SrcReg)) {
  Opc = M68k::MOV8cd;
  } else if (M68k::DR16RegClass.contains(SrcReg)) {
@@ -773,18 +759,69 @@ void M68kInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
  LLVM_DEBUG(dbgs() << "Cannot copy " << RI.getName(SrcReg) << " to CCR\n");
  llvm_unreachable("Invalid register for MOVE to CCR");
  }
- } else if (FromSR || ToSR)
+ } else if (SrcReg == M68k::SR || DstReg == M68k::SR)
  llvm_unreachable("Cannot emit SR copy instruction");
 
- if (Opc) {
+ if (!Opc) {
+ LLVM_DEBUG(dbgs() << "Cannot copy " << RI.getName(SrcReg) << " to "
+ << RI.getName(DstReg) << '\n');
+ llvm_unreachable("Cannot emit physreg copy instruction");
+ }
+
+ unsigned CCRSrcReg = STI.isM68000() ? M68k::SR : M68k::CCR;
+
+ // Get the live registers right before the COPY instruction. If CCR is
+ // live, the MOVE is going to kill it, so we will need to preserve it.
+ LiveRegUnits UsedRegs(RI);
+ UsedRegs.addLiveOuts(MBB);
+ auto InstUpToI = MBB.end();
+ while (InstUpToI != MI) {
+ UsedRegs.stepBackward(*--InstUpToI);
+ }
+
+ if (SrcReg == M68k::CCR) {
+ BuildMI(MBB, MI, DL, get(Opc), DstReg).addReg(CCRSrcReg);
+ return;
+ }
+ if (DstReg == M68k::CCR) {
+ BuildMI(MBB, MI, DL, get(Opc), M68k::CCR)
+ .addReg(SrcReg, getKillRegState(KillSrc));
+ return;
+ }
+ if (UsedRegs.available(M68k::CCR)) {
  BuildMI(MBB, MI, DL, get(Opc), DstReg)
-  .addReg(SrcReg, getKillRegState(KillSrc));
+ .addReg(SrcReg, getKillRegState(KillSrc));
  return;
  }
 
- LLVM_DEBUG(dbgs() << "Cannot copy " << RI.getName(SrcReg) << " to "
- << RI.getName(DstReg) << '\n');
- llvm_unreachable("Cannot emit physreg copy instruction");
+ // CCR is live, so we must restore it after the copy. Prepare push/pop ops.
+ // 68000 must use MOVE from SR, 68010+ must use MOVE from CCR. In either
+ // case, MOVE to CCR masks out the upper byte.
+
+ // Look for an available data register for the CCR, or push to stack if
+ // there are none
+ BitVector Allocatable = RI.getAllocatableSet(
+ MF, RI.getRegClass(M68k::DR16RegClassID));
+ for (Register Reg : Allocatable.set_bits()) {
+ if (!RI.regsOverlap(DstReg, Reg) && (UsedRegs.available(Reg))) {
+ unsigned CCRPushOp = STI.isM68000() ? M68k::MOV16ds : M68k::MOV16dc;
+ unsigned CCRPopOp = M68k::MOV16cd;
+ BuildMI(MBB, MI, DL, get(CCRPushOp), Reg).addReg(CCRSrcReg);
+ BuildMI(MBB, MI, DL, get(Opc), DstReg)
+ .addReg(SrcReg, getKillRegState(KillSrc));
+ BuildMI(MBB, MI, DL, get(CCRPopOp), M68k::CCR).addReg(Reg);
+ return;
+ }
+ }
+
+ unsigned CCRPushOp = STI.isM68000() ? M68k::MOV16es : M68k::MOV16ec;
+ unsigned CCRPopOp = M68k::MOV16co;
+
+ BuildMI(MBB, MI, DL, get(CCRPushOp)).addReg(RI.getStackRegister()).addReg(CCRSrcReg);
+ BuildMI(MBB, MI, DL, get(Opc), DstReg)
+ .addReg(SrcReg, getKillRegState(KillSrc));
+ BuildMI(MBB, MI, DL, get(CCRPopOp), M68k::CCR).addReg(RI.getStackRegister());
+ return;
 }
 
 namespace {

llvm/lib/Target/M68k/M68kInstrInfo.td

@@ -465,6 +465,9 @@ def IsPIC : Predicate<"TM.isPositionIndependent()">;
 def IsNotPIC : Predicate<"!TM.isPositionIndependent()">;
 
 // ISA versions
+def IsM68000 : Predicate<"Subtarget->isM68000()">,
+ AssemblerPredicate<(all_of FeatureISA00)>;
+
 foreach i = [0,1,2,4,6] in
 def AtLeastM680 # i # "0" : Predicate<"Subtarget->atLeastM680"#i#"0()">,
  AssemblerPredicate<(all_of

llvm/lib/Target/M68k/M68kRegisterInfo.td

@@ -87,7 +87,7 @@ class MxPseudoReg<string N, list<Register> SUBREGS = [], list<SubRegIndex> SUBID
  : MxReg<N, 0, SUBREGS, SUBIDX>;
 
 def CCR : MxPseudoReg<"ccr">;
-def SR : MxPseudoReg<"sr">;
+def SR : MxPseudoReg<"sr", [CCR], [MxSubRegIndex8Lo]>;
 
 def PC : MxPseudoReg<"pc">;
 

llvm/lib/Target/M68k/M68kSubtarget.h

@@ -82,6 +82,7 @@ class M68kSubtarget : public M68kGenSubtargetInfo {
  /// of function is auto generated by tblgen.
  void ParseSubtargetFeatures(StringRef CPU, StringRef TuneCPU, StringRef FS);
 
+ bool isM68000() const { return SubtargetKind == M00; }
  bool atLeastM68000() const { return SubtargetKind >= M00; }
  bool atLeastM68010() const { return SubtargetKind >= M10; }
  bool atLeastM68020() const { return SubtargetKind >= M20; }