[TwoAddressInstruction][PowerPC] Call `regOverlapsSet` to find out real clobbers and uses

[lkail]

Summary

In rescheduleKillAboveMI, current implementation uses SmallSet to track reg's defs and uses. When comparing, use SmallSet.count to find out if it's clobbered or used. It's not correct if involving subregisters. This patch uses regOverlapsSet already used by rescheduleMIBelowKill to fix the issue.

Fixed https://bugs.llvm.org/show_bug.cgi?id=47707.

[nemanjai]

LGTM. Checking for overlap rather than simply the same register should be the right thing to do. However, I wonder if we might have other places where a similar assumption is made that could become a problem since we presumably only mark the subregister as an implicit def.

llvm/lib/CodeGen/TwoAddressInstructionPass.cpp

rescheduleKillAboveMI(MachineBasicBlock::iterator &mi,
   unsigned DstReg;
   if (isTwoAddrUse(*KillMI, Reg, DstReg))
     return false;
 
   bool SeenStore = true;
   if (!KillMI->isSafeToMove(AA, SeenStore))
     return false;
 
-  SmallSet<unsigned, 2> Uses;
-  SmallSet<unsigned, 2> Kills;
-  SmallSet<unsigned, 2> Defs;
-  SmallSet<unsigned, 2> LiveDefs;
+  SmallVector<unsigned, 2> Uses;
+  SmallVector<unsigned, 2> Kills;
+  SmallVector<unsigned, 2> Defs;
+  SmallVector<unsigned, 2> LiveDefs;
   for (const MachineOperand &MO : KillMI->operands()) {
     if (!MO.isReg())
       continue;
     Register MOReg = MO.getReg();
     if (MO.isUse()) {
       if (!MOReg)
         continue;
       if (isDefTooClose(MOReg, DI->second, MI))
         return false;
       bool isKill = MO.isKill() || (LIS && isPlainlyKilled(KillMI, MOReg, LIS));
       if (MOReg == Reg && !isKill)
         return false;
-      Uses.insert(MOReg);
+      Uses.push_back(MOReg);
       if (isKill && MOReg != Reg)
-        Kills.insert(MOReg);
+        Kills.push_back(MOReg);
     } else if (Register::isPhysicalRegister(MOReg)) {
-      Defs.insert(MOReg);
+      Defs.push_back(MOReg);
       if (!MO.isDead())
-        LiveDefs.insert(MOReg);
+        LiveDefs.push_back(MOReg);
     }
   }
 
   // Check if the reschedule will not break depedencies.
   unsigned NumVisited = 0;
   for (MachineInstr &OtherMI :
        make_range(mi, MachineBasicBlock::iterator(KillMI))) {
     // Debug instructions cannot be counted against the limit.
for (MachineInstr &OtherMI :
     SmallVector<unsigned, 2> OtherDefs;
     for (const MachineOperand &MO : OtherMI.operands()) {
       if (!MO.isReg())
         continue;
       Register MOReg = MO.getReg();
       if (!MOReg)
         continue;
       if (MO.isUse()) {
-        if (Defs.count(MOReg))
+        if (regOverlapsSet(Defs, MOReg, TRI))
           // Moving KillMI can clobber the physical register if the def has
           // not been seen.
           return false;
-        if (Kills.count(MOReg))
+        if (regOverlapsSet(Kills, MOReg, TRI))
           // Don't want to extend other live ranges and update kills.
           return false;
         if (&OtherMI != MI && MOReg == Reg &&
             !(MO.isKill() || (LIS && isPlainlyKilled(&OtherMI, MOReg, LIS))))
           // We can't schedule across a use of the register in question.
           return false;
       } else {
         OtherDefs.push_back(MOReg);
       }
     }
 
     for (unsigned i = 0, e = OtherDefs.size(); i != e; ++i) {
       unsigned MOReg = OtherDefs[i];
-      if (Uses.count(MOReg))
+      if (regOverlapsSet(Uses, MOReg, TRI))
         return false;
-      if (Register::isPhysicalRegister(MOReg) && LiveDefs.count(MOReg))
+      if (Register::isPhysicalRegister(MOReg) &&
+          regOverlapsSet(LiveDefs, MOReg, TRI))
         return false;
       // Physical register def is seen.
-      Defs.erase(MOReg);
+      Defs.erase(std::remove(Defs.begin(), Defs.end(), MOReg), Defs.end());
     }
   }
 
   // Move the old kill above MI, don't forget to move debug info as well.
   MachineBasicBlock::iterator InsertPos = mi;
   while (InsertPos != MBB->begin() && std::prev(InsertPos)->isDebugInstr())
     --InsertPos;
   MachineBasicBlock::iterator From = KillMI;

llvm/test/CodeGen/PowerPC/pr47707.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -simplify-mir -verify-machineinstrs < %s | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-n32:64"
+target triple = "powerpc64le-grtev4-linux-gnu"
+
+define void @foo(i64* %p1, i64 %v1, i8 %v2, i64 %v3) {
+; CHECK-LABEL: foo:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    mr 7, 5
+; CHECK-NEXT:    rldimi. 7, 4, 8, 0
+; CHECK-NEXT:    mcrf 1, 0
+; CHECK-NEXT:    andi. 5, 5, 1
+; CHECK-NEXT:    li 5, 0
+; CHECK-NEXT:    std 5, 0(3)
+; CHECK-NEXT:    crnot 20, 6
+; CHECK-NEXT:    bc 4, 1, .LBB0_2
+; CHECK-NEXT:  # %bb.1: # %bb1
+; CHECK-NEXT:    std 4, 0(3)
+; CHECK-NEXT:  .LBB0_2: # %bb2
+; CHECK-NEXT:    bclr 12, 20, 0
+; CHECK-NEXT:  # %bb.3: # %bb3
+; CHECK-NEXT:    std 6, 0(3)
+; CHECK-NEXT:    blr
+  store i64 0, i64* %p1, align 8
+  %ext = zext i8 %v2 to i64
+  %shift = shl nuw i64 %v1, 8
+  %merge = or i64 %shift, %ext
+  %not0 = icmp ne i64 %merge, 0
+  %bit0 = and i64 %ext, 1                 ; and & icmp instructions can be combined
+  %cond1 = icmp eq i64 %bit0, 0           ; to and. and generates condition code to
+  br i1 %cond1, label %bb2, label %bb1    ; be used by this conditional branch
+
+bb1:
+  store i64 %v1, i64* %p1, align 8
+  br label %bb2
+
+bb2:
+  br i1 %not0, label %exit, label %bb3
+
+bb3:
+  store i64 %v3, i64* %p1, align 8
+  br label %exit
+
+exit:
+  ret void
+}