[MachineCopyPropagation] Fix D125335 accidentally change control flow.

[HanKuanChen]

Add tag and more information in commit message.

[HanKuanChen]

Update test.

[adriantong1024]

llvm/lib/CodeGen/MachineCopyPropagation.cpp

663–686

Is this not the same as what is here already ?

933–935

Same here. Is this not the same as what is here already ?

[HanKuanChen]

llvm/lib/CodeGen/MachineCopyPropagation.cpp

663–686

No. The line 658 and 736 are included by the same loop (line 655).

933–935

Same. The line 929 and 949 are included by the same loop (line 926).

[adriantong1024]

LGTM

llvm/lib/CodeGen/MachineCopyPropagation.cpp

void MachineCopyPropagation::ForwardCopyPropagateBlock(MachineBasicBlock &MBB) {
   for (MachineInstr &MI : llvm::make_early_inc_range(MBB)) {
     // Analyze copies (which don't overlap themselves).
     Optional<DestSourcePair> CopyOperands = isCopyInstr(MI, *TII, UseCopyInstr);
     if (CopyOperands) {
 
       Register RegSrc = CopyOperands->Source->getReg();
       Register RegDef = CopyOperands->Destination->getReg();
 
-      if (TRI->regsOverlap(RegDef, RegSrc))
-        continue;
-
+      if (!TRI->regsOverlap(RegDef, RegSrc)) {
         assert(RegDef.isPhysical() && RegSrc.isPhysical() &&
               "MachineCopyPropagation should be run after register allocation!");
 
         MCRegister Def = RegDef.asMCReg();
         MCRegister Src = RegSrc.asMCReg();
 
         // The two copies cancel out and the source of the first copy
         // hasn't been overridden, eliminate the second one. e.g.
         //  %ecx = COPY %eax
         //  ... nothing clobbered eax.
         //  %eax = COPY %ecx
         // =>
         //  %ecx = COPY %eax
         //
         // or
         //
         //  %ecx = COPY %eax
         //  ... nothing clobbered eax.
         //  %ecx = COPY %eax
         // =>
         //  %ecx = COPY %eax
         if (eraseIfRedundant(MI, Def, Src) || eraseIfRedundant(MI, Src, Def))
           continue;
 
         forwardUses(MI);
 
         // Src may have been changed by forwardUses()
         CopyOperands = isCopyInstr(MI, *TII, UseCopyInstr);
         Src = CopyOperands->Source->getReg().asMCReg();
 
         // If Src is defined by a previous copy, the previous copy cannot be
         // eliminated.
         ReadRegister(Src, MI, RegularUse);
         for (const MachineOperand &MO : MI.implicit_operands()) {
           if (!MO.isReg() || !MO.readsReg())
             continue;
           MCRegister Reg = MO.getReg().asMCReg();
           if (!Reg)
             continue;
           ReadRegister(Reg, MI, RegularUse);
         }
 
         LLVM_DEBUG(dbgs() << "MCP: Copy is a deletion candidate: "; MI.dump());
 
         // Copy is now a candidate for deletion.
         if (!MRI->isReserved(Def))
           MaybeDeadCopies.insert(&MI);
 
         // If 'Def' is previously source of another copy, then this earlier copy's
         // source is no longer available. e.g.
         // %xmm9 = copy %xmm2
         // ...
         // %xmm2 = copy %xmm0
         // ...
         // %xmm2 = copy %xmm9
         Tracker.clobberRegister(Def, *TRI, *TII, UseCopyInstr);
         for (const MachineOperand &MO : MI.implicit_operands()) {
           if (!MO.isReg() || !MO.isDef())
             continue;
           MCRegister Reg = MO.getReg().asMCReg();
           if (!Reg)
             continue;
           Tracker.clobberRegister(Reg, *TRI, *TII, UseCopyInstr);
         }
 
         Tracker.trackCopy(&MI, *TRI, *TII, UseCopyInstr);
 
         continue;
       }
+    }
 
     // Clobber any earlyclobber regs first.
     for (const MachineOperand &MO : MI.operands())
       if (MO.isReg() && MO.isEarlyClobber()) {
         MCRegister Reg = MO.getReg().asMCReg();
         // If we have a tied earlyclobber, that means it is also read by this
         // instruction, so we need to make sure we don't remove it as dead
         // later.

 void MachineCopyPropagation::BackwardCopyPropagateBlock(
     MachineBasicBlock &MBB) {
   LLVM_DEBUG(dbgs() << "MCP: BackwardCopyPropagateBlock " << MBB.getName()
                     << "\n");
 
   for (MachineInstr &MI : llvm::make_early_inc_range(llvm::reverse(MBB))) {
     // Ignore non-trivial COPYs.
     Optional<DestSourcePair> CopyOperands = isCopyInstr(MI, *TII, UseCopyInstr);
-    if (CopyOperands) {
+    if (CopyOperands && MI.getNumOperands() == 2) {
       Register DefReg = CopyOperands->Destination->getReg();
       Register SrcReg = CopyOperands->Source->getReg();
 
-      if (TRI->regsOverlap(DefReg, SrcReg))
-        continue;
-
+      if (!TRI->regsOverlap(DefReg, SrcReg)) {
         MCRegister Def = DefReg.asMCReg();
         MCRegister Src = SrcReg.asMCReg();
 
         // Unlike forward cp, we don't invoke propagateDefs here,
         // just let forward cp do COPY-to-COPY propagation.
         if (isBackwardPropagatableCopy(MI, *MRI, *TII, UseCopyInstr)) {
           Tracker.invalidateRegister(Src, *TRI, *TII, UseCopyInstr);
           Tracker.invalidateRegister(Def, *TRI, *TII, UseCopyInstr);
           Tracker.trackCopy(&MI, *TRI, *TII, UseCopyInstr);
           continue;
         }
       }
+    }
 
     // Invalidate any earlyclobber regs first.
     for (const MachineOperand &MO : MI.operands())
       if (MO.isReg() && MO.isEarlyClobber()) {
         MCRegister Reg = MO.getReg().asMCReg();
         if (!Reg)
           continue;
         Tracker.invalidateRegister(Reg, *TRI, *TII, UseCopyInstr);

llvm/test/CodeGen/RISCV/machine-cp.mir

 ---
 name:            foo
 body:             |
   bb.0.entry:
     liveins: $v28_v29_v30, $v8_v9, $v1
     ; RV32-LABEL: name: foo
     ; RV32: liveins: $v28_v29_v30, $v8_v9, $v1
     ; RV32-NEXT: {{  $}}
-    ; RV32-NEXT: renamable $v4_v5_v6_v7_v8_v9_v10_v11 = COPY renamable $v0_v1_v2_v3_v4_v5_v6_v7
-    ; RV32-NEXT: renamable $v28 = COPY $v1, implicit killed $v28_v29_v30, implicit-def $v28_v29_v30
+    ; RV32-NEXT: renamable $v4_v5_v6_v7_v8_v9_v10_v11 = COPY killed renamable $v0_v1_v2_v3_v4_v5_v6_v7
+    ; RV32-NEXT: renamable $v28 = COPY renamable $v8, implicit killed $v28_v29_v30, implicit-def $v28_v29_v30
     ; RV32-NEXT: PseudoRET implicit $v28
     ; RV64-LABEL: name: foo
     ; RV64: liveins: $v28_v29_v30, $v8_v9, $v1
     ; RV64-NEXT: {{  $}}
-    ; RV64-NEXT: renamable $v4_v5_v6_v7_v8_v9_v10_v11 = COPY renamable $v0_v1_v2_v3_v4_v5_v6_v7
-    ; RV64-NEXT: renamable $v28 = COPY $v1, implicit killed $v28_v29_v30, implicit-def $v28_v29_v30
+    ; RV64-NEXT: renamable $v4_v5_v6_v7_v8_v9_v10_v11 = COPY killed renamable $v0_v1_v2_v3_v4_v5_v6_v7
+    ; RV64-NEXT: renamable $v28 = COPY renamable $v8, implicit killed $v28_v29_v30, implicit-def $v28_v29_v30
     ; RV64-NEXT: PseudoRET implicit $v28
     renamable $v8 = COPY renamable $v1, implicit killed $v8_v9, implicit-def $v8_v9
     renamable $v4_v5_v6_v7_v8_v9_v10_v11 = COPY killed renamable $v0_v1_v2_v3_v4_v5_v6_v7
     renamable $v28 = COPY renamable $v8, implicit killed $v28_v29_v30, implicit-def $v28_v29_v30
     PseudoRET implicit $v28
 ...