[PHIElimination] Account for PHI operands that appear more than once

During lowering, the PHI instruction is detached from the function and
thus it is not considered a user of its operands.
The previous implementation, which removed redundant COPY of COPY
instructions, skipped values which had any uses, because otherwise it
can break the function. However, it had a hidden assumption that there's
a single use for that operand in the detached PHI instruction.
By not taking into account that an operand can be referenced more than
once, it interfered with itself and created IMPLICIT_DEF's for some
incoming values instead of copying the correct ones.

Author: guy-david

llvm/lib/CodeGen/PHIElimination.cpp

@@ -541,6 +541,7 @@ void PHIEliminationImpl::LowerPHINode(MachineBasicBlock &MBB,
   // Now loop over all of the incoming arguments, changing them to copy into the
   // IncomingReg register in the corresponding predecessor basic block.
   SmallPtrSet<MachineBasicBlock *, 8> MBBsInsertedInto;
+  SmallVector<MachineInstr *, 8> InsertedCopies;
   for (int i = NumSrcs - 1; i >= 0; --i) {
     Register SrcReg = MPhi->getOperand(i * 2 + 1).getReg();
     unsigned SrcSubReg = MPhi->getOperand(i * 2 + 1).getSubReg();
@@ -581,20 +582,6 @@ void PHIEliminationImpl::LowerPHINode(MachineBasicBlock &MBB,
       continue;
     }
 
-    // Reuse an existing copy in the block if possible.
-    if (IncomingReg.isVirtual()) {
-      MachineInstr *DefMI = MRI->getUniqueVRegDef(SrcReg);
-      const TargetRegisterClass *SrcRC = MRI->getRegClass(SrcReg);
-      const TargetRegisterClass *IncomingRC = MRI->getRegClass(IncomingReg);
-      if (DefMI && DefMI->isCopy() && DefMI->getParent() == &opBlock &&
-          MRI->use_empty(SrcReg) && IncomingRC->hasSuperClassEq(SrcRC)) {
-        DefMI->getOperand(0).setReg(IncomingReg);
-        if (LV)
-          LV->getVarInfo(SrcReg).AliveBlocks.clear();
-        continue;
-      }
-    }
-
     // Find a safe location to insert the copy, this may be the first terminator
     // in the block (or end()).
     MachineBasicBlock::iterator InsertPos =
@@ -621,6 +608,7 @@ void PHIEliminationImpl::LowerPHINode(MachineBasicBlock &MBB,
         NewSrcInstr = TII->createPHISourceCopy(opBlock, InsertPos, nullptr,
                                                SrcReg, SrcSubReg, IncomingReg);
       }
+      InsertedCopies.emplace_back(NewSrcInstr);
     }
 
     // We only need to update the LiveVariables kill of SrcReg if this was the
@@ -744,6 +732,32 @@ void PHIEliminationImpl::LowerPHINode(MachineBasicBlock &MBB,
     }
   }
 
+  // Remove redundant COPY instructions chains, which were potentially added by
+  // the code above. This can simplify the CFG which later on prevents
+  // suboptimal block layout.
+  for (MachineInstr *NewCopy : InsertedCopies) {
+    if (NewCopy->isImplicitDef())
+      continue;
+    Register IncomingReg = NewCopy->getOperand(0).getReg();
+    if (!IncomingReg.isVirtual())
+      continue;
+    Register SrcReg = NewCopy->getOperand(1).getReg();
+    if (!MRI->hasOneUse(SrcReg))
+      continue;
+    MachineInstr *DefMI = MRI->getUniqueVRegDef(SrcReg);
+    if (!DefMI || !DefMI->isCopy() ||
+        DefMI->getParent() != NewCopy->getParent())
+      continue;
+    const TargetRegisterClass *SrcRC = MRI->getRegClass(SrcReg);
+    const TargetRegisterClass *IncomingRC = MRI->getRegClass(IncomingReg);
+    if (!IncomingRC->hasSuperClassEq(SrcRC))
+      continue;
+    DefMI->getOperand(0).setReg(IncomingReg);
+    NewCopy->removeFromParent();
+    if (LV)
+      LV->getVarInfo(SrcReg).AliveBlocks.clear();
+  }
+
   // Really delete the PHI instruction now, if it is not in the LoweredPHIs map.
   if (EliminateNow) {
     if (LIS)

llvm/test/CodeGen/AArch64/PHIElimination-reuse-copy.mir

@@ -135,14 +135,15 @@ body:             |
   ; CHECK-NEXT:   liveins: $nzcv
   ; CHECK-NEXT: {{  $}}
   ; CHECK-NEXT:   dead [[COPY2:%[0-9]+]]:gpr32 = COPY killed [[COPY1]]
-  ; CHECK-NEXT:   [[COPY1:%[0-9]+]]:gpr32 = COPY [[COPY]]
+  ; CHECK-NEXT:   [[COPY3:%[0-9]+]]:gpr32 = COPY [[COPY]]
+  ; CHECK-NEXT:   [[COPY1:%[0-9]+]]:gpr32 = COPY [[COPY3]]
   ; CHECK-NEXT:   Bcc 1, %bb.1, implicit $nzcv
   ; CHECK-NEXT: {{  $}}
   ; CHECK-NEXT: bb.2:
   ; CHECK-NEXT:   successors: %bb.1(0x80000000)
   ; CHECK-NEXT:   liveins: $nzcv
   ; CHECK-NEXT: {{  $}}
-  ; CHECK-NEXT:   [[COPY1:%[0-9]+]]:gpr32 = IMPLICIT_DEF
+  ; CHECK-NEXT:   [[COPY1:%[0-9]+]]:gpr32 = COPY killed [[COPY3]]
   ; CHECK-NEXT:   B %bb.1
   bb.0:
     successors: %bb.1

llvm/test/CodeGen/PowerPC/vsx.ll

@@ -2487,8 +2487,7 @@ define double @test82(double %a, double %b, double %c, double %d) {
 ; CHECK-FISL-LABEL: test82:
 ; CHECK-FISL:       # %bb.0: # %entry
 ; CHECK-FISL-NEXT:    stfd f2, -16(r1) # 8-byte Folded Spill
-; CHECK-FISL-NEXT:    fmr f2, f1
-; CHECK-FISL-NEXT:    stfd f2, -8(r1) # 8-byte Folded Spill
+; CHECK-FISL-NEXT:    stfd f1, -8(r1) # 8-byte Folded Spill
 ; CHECK-FISL-NEXT:    xscmpudp cr0, f3, f4
 ; CHECK-FISL-NEXT:    beq cr0, .LBB67_2
 ; CHECK-FISL-NEXT:  # %bb.1: # %entry