[ARM] Replace TransferImpOps with copyImplicitOps

In most places where TransferImpOps is currently used we just have one
machine instruction, so it's doing the same thing as copyImplicitOps
anyway. In those cases where we have more than one machine
instruction the destination is written to in each instruction so any
implicit defs should appear on all of them (and we shouldn't see any
implicit refs as these pseudo-instruction don't have any register
inputs), meaning the current use of TransferImpOps is incorrect and
we should be using copyImplicitOps on all of the generated
instructions.

Differential Revision: https://reviews.llvm.org/D155301

diff --git a/llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp b/llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp
index f628a72..1adaa6a 100644 (file)
--- a/llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp
+++ b/llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp
@@ -60,8 +60,6 @@ namespace {
     }
 
   private:
-    void TransferImpOps(MachineInstr &OldMI,
-                        MachineInstrBuilder &UseMI, MachineInstrBuilder &DefMI);
     bool ExpandMI(MachineBasicBlock &MBB,
                   MachineBasicBlock::iterator MBBI,
                   MachineBasicBlock::iterator &NextMBBI);
@@ -123,22 +121,6 @@ namespace {
 INITIALIZE_PASS(ARMExpandPseudo, DEBUG_TYPE, ARM_EXPAND_PSEUDO_NAME, false,
                 false)
 
-/// TransferImpOps - Transfer implicit operands on the pseudo instruction to
-/// the instructions created from the expansion.
-void ARMExpandPseudo::TransferImpOps(MachineInstr &OldMI,
-                                     MachineInstrBuilder &UseMI,
-                                     MachineInstrBuilder &DefMI) {
-  const MCInstrDesc &Desc = OldMI.getDesc();
-  for (const MachineOperand &MO :
-       llvm::drop_begin(OldMI.operands(), Desc.getNumOperands())) {
-    assert(MO.isReg() && MO.getReg());
-    if (MO.isUse())
-      UseMI.add(MO);
-    else
-      DefMI.add(MO);
-  }
-}
-
 namespace {
   // Constants for register spacing in NEON load/store instructions.
   // For quad-register load-lane and store-lane pseudo instructors, the
@@ -678,7 +660,7 @@ void ARMExpandPseudo::ExpandVLD(MachineBasicBlock::iterator &MBBI) {
   }
   // Add an implicit def for the super-register.
   MIB.addReg(DstReg, RegState::ImplicitDefine | getDeadRegState(DstIsDead));
-  TransferImpOps(MI, MIB, MIB);
+  MIB.copyImplicitOps(MI);
 
   // Transfer memoperands.
   MIB.cloneMemRefs(MI);
@@ -754,7 +736,7 @@ void ARMExpandPseudo::ExpandVST(MachineBasicBlock::iterator &MBBI) {
     MIB->addRegisterKilled(SrcReg, TRI, true);
   else if (!SrcIsUndef)
     MIB.addReg(SrcReg, RegState::Implicit); // Add implicit uses for src reg.
-  TransferImpOps(MI, MIB, MIB);
+  MIB.copyImplicitOps(MI);
 
   // Transfer memoperands.
   MIB.cloneMemRefs(MI);
@@ -846,7 +828,7 @@ void ARMExpandPseudo::ExpandLaneOp(MachineBasicBlock::iterator &MBBI) {
   if (TableEntry->IsLoad)
     // Add an implicit def for the super-register.
     MIB.addReg(DstReg, RegState::ImplicitDefine | getDeadRegState(DstIsDead));
-  TransferImpOps(MI, MIB, MIB);
+  MIB.copyImplicitOps(MI);
   // Transfer memoperands.
   MIB.cloneMemRefs(MI);
   MI.eraseFromParent();
@@ -886,7 +868,7 @@ void ARMExpandPseudo::ExpandVTBL(MachineBasicBlock::iterator &MBBI,
 
   // Add an implicit kill and use for the super-reg.
   MIB.addReg(SrcReg, RegState::Implicit | getKillRegState(SrcIsKill));
-  TransferImpOps(MI, MIB, MIB);
+  MIB.copyImplicitOps(MI);
   MI.eraseFromParent();
   LLVM_DEBUG(dbgs() << "To:        "; MIB.getInstr()->dump(););
 }
@@ -923,7 +905,7 @@ void ARMExpandPseudo::ExpandMQQPRLoadStore(MachineBasicBlock::iterator &MBBI) {
   if (NewOpc == ARM::VSTMDIA)
     MIB.addReg(SrcReg, RegState::Implicit);
 
-  TransferImpOps(MI, MIB, MIB);
+  MIB.copyImplicitOps(MI);
   MIB.cloneMemRefs(MI);
   MI.eraseFromParent();
 }
@@ -1132,7 +1114,8 @@ void ARMExpandPseudo::ExpandMOV32BitImm(MachineBasicBlock &MBB,
     HI16.addImm(Pred).addReg(PredReg).add(condCodeOp());
     if (isCC)
       LO16.add(makeImplicit(MI.getOperand(1)));
-    TransferImpOps(MI, LO16, HI16);
+    LO16.copyImplicitOps(MI);
+    HI16.copyImplicitOps(MI);
     MI.eraseFromParent();
     return;
   }
@@ -1191,7 +1174,8 @@ void ARMExpandPseudo::ExpandMOV32BitImm(MachineBasicBlock &MBB,
 
   if (isCC)
     LO16.add(makeImplicit(MI.getOperand(1)));
-  TransferImpOps(MI, LO16, HI16);
+  LO16.copyImplicitOps(MI);
+  HI16.copyImplicitOps(MI);
   MI.eraseFromParent();
   LLVM_DEBUG(dbgs() << "To:        "; LO16.getInstr()->dump(););
   LLVM_DEBUG(dbgs() << "And:       "; HI16.getInstr()->dump(););
@@ -2584,14 +2568,13 @@ bool ARMExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
     }
     case ARM::RRX: {
       // This encodes as "MOVs Rd, Rm, rrx
-      MachineInstrBuilder MIB =
-          BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(ARM::MOVsi),
-                  MI.getOperand(0).getReg())
-              .add(MI.getOperand(1))
-              .addImm(ARM_AM::getSORegOpc(ARM_AM::rrx, 0))
-              .add(predOps(ARMCC::AL))
-              .add(condCodeOp());
-      TransferImpOps(MI, MIB, MIB);
+      BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(ARM::MOVsi),
+              MI.getOperand(0).getReg())
+          .add(MI.getOperand(1))
+          .addImm(ARM_AM::getSORegOpc(ARM_AM::rrx, 0))
+          .add(predOps(ARMCC::AL))
+          .add(condCodeOp())
+          .copyImplicitOps(MI);
       MI.eraseFromParent();
       return true;
     }
@@ -2631,7 +2614,7 @@ bool ARMExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
       }
 
       MIB.cloneMemRefs(MI);
-      TransferImpOps(MI, MIB, MIB);
+      MIB.copyImplicitOps(MI);
       // Update the call site info.
       if (MI.isCandidateForCallSiteEntry())
         MF->moveCallSiteInfo(&MI, &*MIB);
@@ -2644,17 +2627,16 @@ bool ARMExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
         ? ARM::tLDRpci : ARM::t2LDRpci;
       Register DstReg = MI.getOperand(0).getReg();
       bool DstIsDead = MI.getOperand(0).isDead();
-      MachineInstrBuilder MIB1 =
-          BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(NewLdOpc), DstReg)
-              .add(MI.getOperand(1))
-              .add(predOps(ARMCC::AL));
-      MIB1.cloneMemRefs(MI);
-      MachineInstrBuilder MIB2 =
-          BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(ARM::tPICADD))
-              .addReg(DstReg, RegState::Define | getDeadRegState(DstIsDead))
-              .addReg(DstReg)
-              .add(MI.getOperand(2));
-      TransferImpOps(MI, MIB1, MIB2);
+      BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(NewLdOpc), DstReg)
+          .add(MI.getOperand(1))
+          .add(predOps(ARMCC::AL))
+          .cloneMemRefs(MI)
+          .copyImplicitOps(MI);
+      BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(ARM::tPICADD))
+          .addReg(DstReg, RegState::Define | getDeadRegState(DstIsDead))
+          .addReg(DstReg)
+          .add(MI.getOperand(2))
+          .copyImplicitOps(MI);
       MI.eraseFromParent();
       return true;
     }
@@ -2739,15 +2721,16 @@ bool ARMExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
       unsigned PICAddOpc = isARM
         ? (Opcode == ARM::MOV_ga_pcrel_ldr ? ARM::PICLDR : ARM::PICADD)
         : ARM::tPICADD;
-      MachineInstrBuilder MIB1 = BuildMI(MBB, MBBI, MI.getDebugLoc(),
-                                         TII->get(LO16Opc), DstReg)
-        .addGlobalAddress(GV, MO1.getOffset(), TF | LO16TF)
-        .addImm(LabelId);
+      BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(LO16Opc), DstReg)
+          .addGlobalAddress(GV, MO1.getOffset(), TF | LO16TF)
+          .addImm(LabelId)
+          .copyImplicitOps(MI);
 
       BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(HI16Opc), DstReg)
-        .addReg(DstReg)
-        .addGlobalAddress(GV, MO1.getOffset(), TF | HI16TF)
-        .addImm(LabelId);
+          .addReg(DstReg)
+          .addGlobalAddress(GV, MO1.getOffset(), TF | HI16TF)
+          .addImm(LabelId)
+          .copyImplicitOps(MI);
 
       MachineInstrBuilder MIB3 = BuildMI(MBB, MBBI, MI.getDebugLoc(),
                                          TII->get(PICAddOpc))
@@ -2758,7 +2741,7 @@ bool ARMExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
         if (Opcode == ARM::MOV_ga_pcrel_ldr)
           MIB3.cloneMemRefs(MI);
       }
-      TransferImpOps(MI, MIB1, MIB3);
+      MIB3.copyImplicitOps(MI);
       MI.eraseFromParent();
       return true;
     }
@@ -2786,14 +2769,13 @@ bool ARMExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
       return true;
 
     case ARM::SUBS_PC_LR: {
-      MachineInstrBuilder MIB =
-          BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(ARM::SUBri), ARM::PC)
-              .addReg(ARM::LR)
-              .add(MI.getOperand(0))
-              .add(MI.getOperand(1))
-              .add(MI.getOperand(2))
-              .addReg(ARM::CPSR, RegState::Undef);
-      TransferImpOps(MI, MIB, MIB);
+      BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(ARM::SUBri), ARM::PC)
+          .addReg(ARM::LR)
+          .add(MI.getOperand(0))
+          .add(MI.getOperand(1))
+          .add(MI.getOperand(2))
+          .addReg(ARM::CPSR, RegState::Undef)
+          .copyImplicitOps(MI);
       MI.eraseFromParent();
       return true;
     }
@@ -2822,7 +2804,7 @@ bool ARMExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
 
       // Add an implicit def for the super-register.
       MIB.addReg(DstReg, RegState::ImplicitDefine | getDeadRegState(DstIsDead));
-      TransferImpOps(MI, MIB, MIB);
+      MIB.copyImplicitOps(MI);
       MIB.cloneMemRefs(MI);
       MI.eraseFromParent();
       return true;
@@ -2855,7 +2837,7 @@ bool ARMExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
       if (SrcIsKill)      // Add an implicit kill for the Q register.
         MIB->addRegisterKilled(SrcReg, TRI, true);
 
-      TransferImpOps(MI, MIB, MIB);
+      MIB.copyImplicitOps(MI);
       MIB.cloneMemRefs(MI);
       MI.eraseFromParent();
       return true;

diff --git a/llvm/test/CodeGen/ARM/expand-pseudos.mir b/llvm/test/CodeGen/ARM/expand-pseudos.mir
index 9ec0d97..8aada54 100644 (file)
--- a/llvm/test/CodeGen/ARM/expand-pseudos.mir
+++ b/llvm/test/CodeGen/ARM/expand-pseudos.mir
@@ -15,6 +15,15 @@
   entry:
     unreachable
   }
+  define i32 @test4(i32 %x) {
+  entry:
+    unreachable
+  }
+  @var = global i32 0
+  define i32 @test5(i32 %x) {
+  entry:
+    unreachable
+  }
 ...
 ---
 name:            test1
@@ -28,11 +37,12 @@ body:             |
 
     ; CHECK-LABEL: name: test1
     ; CHECK: liveins: $r0
-    ; CHECK: $r1 = MOVi 2, 14 /* CC::al */, $noreg, $noreg
-    ; CHECK: CMPri killed $r0, 0, 14 /* CC::al */, $noreg, implicit-def $cpsr
-    ; CHECK: $r1 = MOVi16 500, 0 /* CC::eq */, killed $cpsr, implicit killed $r1
-    ; CHECK: $r0 = MOVr killed $r1, 14 /* CC::al */, $noreg, $noreg
-    ; CHECK: BX_RET 14 /* CC::al */, $noreg, implicit $r0
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: $r1 = MOVi 2, 14 /* CC::al */, $noreg, $noreg
+    ; CHECK-NEXT: CMPri killed $r0, 0, 14 /* CC::al */, $noreg, implicit-def $cpsr
+    ; CHECK-NEXT: $r1 = MOVi16 500, 0 /* CC::eq */, killed $cpsr, implicit killed $r1
+    ; CHECK-NEXT: $r0 = MOVr killed $r1, 14 /* CC::al */, $noreg, $noreg
+    ; CHECK-NEXT: BX_RET 14 /* CC::al */, $noreg, implicit $r0
     $r1 = MOVi 2, 14, $noreg, $noreg
     CMPri killed $r0, 0, 14, $noreg, implicit-def $cpsr
     $r1 = MOVCCi16 killed $r1, 500, 0, killed $cpsr
@@ -52,12 +62,13 @@ body:             |
 
     ; CHECK-LABEL: name: test2
     ; CHECK: liveins: $r0
-    ; CHECK: $r1 = MOVi 2, 14 /* CC::al */, $noreg, $noreg
-    ; CHECK: CMPri killed $r0, 0, 14 /* CC::al */, $noreg, implicit-def $cpsr
-    ; CHECK: $r1 = MOVi16 2068, 0 /* CC::eq */, $cpsr, implicit killed $r1
-    ; CHECK: $r1 = MOVTi16 $r1, 7637, 0 /* CC::eq */, $cpsr
-    ; CHECK: $r0 = MOVr killed $r1, 14 /* CC::al */, $noreg, $noreg
-    ; CHECK: BX_RET 14 /* CC::al */, $noreg, implicit $r0
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: $r1 = MOVi 2, 14 /* CC::al */, $noreg, $noreg
+    ; CHECK-NEXT: CMPri killed $r0, 0, 14 /* CC::al */, $noreg, implicit-def $cpsr
+    ; CHECK-NEXT: $r1 = MOVi16 2068, 0 /* CC::eq */, $cpsr, implicit killed $r1
+    ; CHECK-NEXT: $r1 = MOVTi16 $r1, 7637, 0 /* CC::eq */, $cpsr
+    ; CHECK-NEXT: $r0 = MOVr killed $r1, 14 /* CC::al */, $noreg, $noreg
+    ; CHECK-NEXT: BX_RET 14 /* CC::al */, $noreg, implicit $r0
     $r1 = MOVi 2, 14, $noreg, $noreg
     CMPri killed $r0, 0, 14, $noreg, implicit-def $cpsr
     $r1 = MOVCCi32imm killed $r1, 500500500, 0, killed $cpsr
@@ -78,11 +89,55 @@ body:             |
 
     ; CHECK-LABEL: name: test3
     ; CHECK: liveins: $r0, $r1
-    ; CHECK: CMPri $r1, 500, 14 /* CC::al */, $noreg, implicit-def $cpsr
-    ; CHECK: $r0 = MOVr killed $r1, 12 /* CC::gt */, killed $cpsr, $noreg, implicit killed $r0
-    ; CHECK: BX_RET 14 /* CC::al */, $noreg, implicit $r0
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: CMPri $r1, 500, 14 /* CC::al */, $noreg, implicit-def $cpsr
+    ; CHECK-NEXT: $r0 = MOVr killed $r1, 12 /* CC::gt */, killed $cpsr, $noreg, implicit killed $r0
+    ; CHECK-NEXT: BX_RET 14 /* CC::al */, $noreg, implicit $r0
     CMPri $r1, 500, 14, $noreg, implicit-def $cpsr
     $r0 = MOVCCr killed $r0, killed $r1, 12, killed $cpsr
     BX_RET 14, $noreg, implicit $r0
 
 ...
+---
+name:            test4
+alignment:       4
+tracksRegLiveness: true
+liveins:
+  - { reg: '$r0', virtual-reg: '' }
+  - { reg: '$r0_r1', virtual-reg: '' }
+body:             |
+  bb.0.entry:
+    liveins: $r0, $r0_r1
+
+    ; CHECK-LABEL: name: test4
+    ; CHECK: liveins: $r0, $r0_r1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: $r0 = MOVi16 51712, 14 /* CC::al */, $noreg, implicit-def $r0_r1
+    ; CHECK-NEXT: $r0 = MOVTi16 $r0, 15258, 14 /* CC::al */, $noreg, implicit-def $r0_r1
+    ; CHECK-NEXT: BX_RET 14 /* CC::al */, $noreg, implicit $r0
+    $r0 = MOVi32imm 1000000000, implicit-def $r0_r1
+    BX_RET 14, $noreg, implicit $r0
+
+...
+---
+name:            test5
+alignment:       4
+tracksRegLiveness: true
+liveins:
+  - { reg: '$r0', virtual-reg: '' }
+  - { reg: '$r0_r1', virtual-reg: '' }
+body:             |
+  bb.0.entry:
+    liveins: $r0, $r0_r1
+
+    ; CHECK-LABEL: name: test5
+    ; CHECK: liveins: $r0, $r0_r1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: $r0 = MOVi16_ga_pcrel target-flags(arm-lo16) @var, 0, implicit-def $r0_r1
+    ; CHECK-NEXT: $r0 = MOVTi16_ga_pcrel $r0, target-flags(arm-hi16) @var, 0, implicit-def $r0_r1
+    ; CHECK-NEXT: $r0 = PICLDR $r0, 0, 14 /* CC::al */, $noreg, implicit-def $r0_r1
+    ; CHECK-NEXT: BX_RET 14 /* CC::al */, $noreg, implicit $r0
+    $r0 = MOV_ga_pcrel_ldr @var, implicit-def $r0_r1
+    BX_RET 14, $noreg, implicit $r0
+
+...

diff --git a/llvm/test/CodeGen/Thumb2/expand-pseudos.mir b/llvm/test/CodeGen/Thumb2/expand-pseudos.mir
new file mode 100644 (file)
index 0000000..e1f326a
--- /dev/null
+++ b/llvm/test/CodeGen/Thumb2/expand-pseudos.mir
@@ -0,0 +1,32 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -run-pass=arm-pseudo -verify-machineinstrs %s -o - | FileCheck %s
+--- |
+  target triple = "thumbv7---gnueabi"
+
+  @var = global i32 0
+  define i32 @test1(i32 %x) {
+  entry:
+    unreachable
+  }
+...
+---
+name:            test1
+alignment:       4
+tracksRegLiveness: true
+liveins:
+  - { reg: '$r0', virtual-reg: '' }
+  - { reg: '$r0_r1', virtual-reg: '' }
+body:             |
+  bb.0.entry:
+    liveins: $r0, $r0_r1
+
+    ; CHECK-LABEL: name: test1
+    ; CHECK: liveins: $r0, $r0_r1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: $r0 = t2LDRpci @var, 14 /* CC::al */, $noreg, implicit-def $r0_r1
+    ; CHECK-NEXT: $r0 = tPICADD $r0, 0, implicit-def $r0_r1
+    ; CHECK-NEXT: BX_RET 14 /* CC::al */, $noreg, implicit $r0
+    $r0 = t2LDRpci_pic @var, 0, implicit-def $r0_r1
+    BX_RET 14, $noreg, implicit $r0
+
+...