[llvm] [SystemZ] Improve shouldCoalesce() for i128. (PR #74942)

Jonas Paulsson via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 13 14:23:45 PST 2023


https://github.com/JonPsson1 updated https://github.com/llvm/llvm-project/pull/74942

>From a15e99f858bd06c8ec1eaacbd71338205f95da3d Mon Sep 17 00:00:00 2001
From: Jonas Paulsson <paulson1 at linux.ibm.com>
Date: Sat, 9 Dec 2023 10:25:10 -0600
Subject: [PATCH 1/2] Improve for PAIR128

---
 llvm/lib/Target/SystemZ/SystemZRegisterInfo.cpp | 11 ++++++++++-
 llvm/test/CodeGen/SystemZ/atomicrmw-ops-i128.ll |  4 +---
 llvm/test/CodeGen/SystemZ/atomicrmw-xchg-07.ll  |  3 +--
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/llvm/lib/Target/SystemZ/SystemZRegisterInfo.cpp b/llvm/lib/Target/SystemZ/SystemZRegisterInfo.cpp
index 4d6b94da3a2754..e822dbf586ab73 100644
--- a/llvm/lib/Target/SystemZ/SystemZRegisterInfo.cpp
+++ b/llvm/lib/Target/SystemZ/SystemZRegisterInfo.cpp
@@ -384,6 +384,7 @@ bool SystemZRegisterInfo::shouldCoalesce(MachineInstr *MI,
                                   const TargetRegisterClass *NewRC,
                                   LiveIntervals &LIS) const {
   assert (MI->isCopy() && "Only expecting COPY instructions");
+  const MachineRegisterInfo *MRI = &MI->getMF()->getRegInfo();
 
   // Coalesce anything which is not a COPY involving a subreg to/from GR128.
   if (!(NewRC->hasSuperClassEq(&SystemZ::GR128BitRegClass) &&
@@ -400,7 +401,6 @@ bool SystemZRegisterInfo::shouldCoalesce(MachineInstr *MI,
   LiveInterval &IntGR128 = LIS.getInterval(GR128Reg);
   LiveInterval &IntGRNar = LIS.getInterval(GRNarReg);
 
-  // Check that the two virtual registers are local to MBB.
   MachineBasicBlock *MBB = MI->getParent();
   MachineInstr *FirstMI_GR128 =
     LIS.getInstructionFromIndex(IntGR128.beginIndex());
@@ -408,6 +408,15 @@ bool SystemZRegisterInfo::shouldCoalesce(MachineInstr *MI,
     LIS.getInstructionFromIndex(IntGRNar.beginIndex());
   MachineInstr *LastMI_GR128 = LIS.getInstructionFromIndex(IntGR128.endIndex());
   MachineInstr *LastMI_GRNar = LIS.getInstructionFromIndex(IntGRNar.endIndex());
+
+  // Allow cases (like PAIR128) where there is just one use of the narrow
+  // source reg defined close to MI.
+  if (WideOpNo == 0 && MRI->hasOneUse(GRNarReg) &&
+      FirstMI_GRNar && FirstMI_GRNar->getParent() == MBB &&
+      std::distance(FirstMI_GRNar->getIterator(), MI->getIterator()) < 5)
+    return true;
+
+  // Check that the two virtual registers are local to MBB.
   if ((!FirstMI_GR128 || FirstMI_GR128->getParent() != MBB) ||
       (!FirstMI_GRNar || FirstMI_GRNar->getParent() != MBB) ||
       (!LastMI_GR128 || LastMI_GR128->getParent() != MBB) ||
diff --git a/llvm/test/CodeGen/SystemZ/atomicrmw-ops-i128.ll b/llvm/test/CodeGen/SystemZ/atomicrmw-ops-i128.ll
index 0e8f0446802220..41b0964be05e56 100644
--- a/llvm/test/CodeGen/SystemZ/atomicrmw-ops-i128.ll
+++ b/llvm/test/CodeGen/SystemZ/atomicrmw-ops-i128.ll
@@ -14,13 +14,11 @@ define i128 @atomicrmw_xchg(ptr %src, i128 %b) {
 ; CHECK-NEXT:    stmg %r12, %r15, 96(%r15)
 ; CHECK-NEXT:    .cfi_offset %r12, -64
 ; CHECK-NEXT:    .cfi_offset %r13, -56
-; CHECK-NEXT:    .cfi_offset %r14, -48
 ; CHECK-NEXT:    .cfi_offset %r15, -40
-; CHECK-NEXT:    lg %r14, 8(%r4)
+; CHECK-NEXT:    lg %r1, 8(%r4)
 ; CHECK-NEXT:    lg %r0, 0(%r4)
 ; CHECK-NEXT:    lg %r4, 8(%r3)
 ; CHECK-NEXT:    lg %r5, 0(%r3)
-; CHECK-NEXT:    lgr %r1, %r14
 ; CHECK-NEXT:  .LBB0_1: # %atomicrmw.start
 ; CHECK-NEXT:    # =>This Inner Loop Header: Depth=1
 ; CHECK-NEXT:    lgr %r12, %r5
diff --git a/llvm/test/CodeGen/SystemZ/atomicrmw-xchg-07.ll b/llvm/test/CodeGen/SystemZ/atomicrmw-xchg-07.ll
index b9e29599af7ee7..18fa89e6ca6cb8 100644
--- a/llvm/test/CodeGen/SystemZ/atomicrmw-xchg-07.ll
+++ b/llvm/test/CodeGen/SystemZ/atomicrmw-xchg-07.ll
@@ -4,11 +4,10 @@
 
 define void @f1(ptr align 16 %ret, ptr align 16 %src, ptr align 16 %b) {
 ; CHECK-LABEL: f1:
-; CHECK:       lg      %r14, 8(%r4)
+; CHECK:       lg      %r1, 8(%r4)
 ; CHECK-NEXT:  lg      %r0, 0(%r4)
 ; CHECK-NEXT:  lg      %r4, 8(%r3)
 ; CHECK-NEXT:  lg      %r5, 0(%r3)
-; CHECK-NEXT:  lgr     %r1, %r14
 ; CHECK-NEXT:.LBB0_1:                          # %atomicrmw.start
 ; CHECK-NEXT:                                  # =>This Inner Loop Header: Depth=1
 ; CHECK-NEXT:  lgr     %r12, %r5

>From 52bb695262fefdd7849206462f48b77d75b8c9b8 Mon Sep 17 00:00:00 2001
From: Jonas Paulsson <paulson1 at linux.ibm.com>
Date: Tue, 12 Dec 2023 09:35:22 -0600
Subject: [PATCH 2/2] Only check the subreg

---
 .../Target/SystemZ/SystemZRegisterInfo.cpp    | 64 ++++++-------------
 1 file changed, 18 insertions(+), 46 deletions(-)

diff --git a/llvm/lib/Target/SystemZ/SystemZRegisterInfo.cpp b/llvm/lib/Target/SystemZ/SystemZRegisterInfo.cpp
index e822dbf586ab73..d5313acd87856c 100644
--- a/llvm/lib/Target/SystemZ/SystemZRegisterInfo.cpp
+++ b/llvm/lib/Target/SystemZ/SystemZRegisterInfo.cpp
@@ -377,66 +377,39 @@ SystemZRegisterInfo::eliminateFrameIndex(MachineBasicBlock::iterator MI,
 }
 
 bool SystemZRegisterInfo::shouldCoalesce(MachineInstr *MI,
-                                  const TargetRegisterClass *SrcRC,
-                                  unsigned SubReg,
-                                  const TargetRegisterClass *DstRC,
-                                  unsigned DstSubReg,
-                                  const TargetRegisterClass *NewRC,
-                                  LiveIntervals &LIS) const {
+                                         const TargetRegisterClass *SrcRC,
+                                         unsigned SubReg,
+                                         const TargetRegisterClass *DstRC,
+                                         unsigned DstSubReg,
+                                         const TargetRegisterClass *NewRC,
+                                         LiveIntervals &LIS) const {
   assert (MI->isCopy() && "Only expecting COPY instructions");
-  const MachineRegisterInfo *MRI = &MI->getMF()->getRegInfo();
 
   // Coalesce anything which is not a COPY involving a subreg to/from GR128.
   if (!(NewRC->hasSuperClassEq(&SystemZ::GR128BitRegClass) &&
         (getRegSizeInBits(*SrcRC) <= 64 || getRegSizeInBits(*DstRC) <= 64)))
     return true;
 
-  // Allow coalescing of a GR128 subreg COPY only if the live ranges are small
-  // and local to one MBB with not too much interferring registers. Otherwise
+  // Allow coalescing of a GR128 subreg COPY only if the subreg liverange is
+  // local to one MBB with not too many interferring physreg clobbers. Otherwise
   // regalloc may run out of registers.
+  unsigned SubregOpIdx = getRegSizeInBits(*SrcRC) == 128 ? 0 : 1;
+  LiveInterval &LI = LIS.getInterval(MI->getOperand(SubregOpIdx).getReg());
 
-  unsigned WideOpNo = (getRegSizeInBits(*SrcRC) == 128 ? 1 : 0);
-  Register GR128Reg = MI->getOperand(WideOpNo).getReg();
-  Register GRNarReg = MI->getOperand((WideOpNo == 1) ? 0 : 1).getReg();
-  LiveInterval &IntGR128 = LIS.getInterval(GR128Reg);
-  LiveInterval &IntGRNar = LIS.getInterval(GRNarReg);
-
+  // Check that the subreg is local to MBB.
   MachineBasicBlock *MBB = MI->getParent();
-  MachineInstr *FirstMI_GR128 =
-    LIS.getInstructionFromIndex(IntGR128.beginIndex());
-  MachineInstr *FirstMI_GRNar =
-    LIS.getInstructionFromIndex(IntGRNar.beginIndex());
-  MachineInstr *LastMI_GR128 = LIS.getInstructionFromIndex(IntGR128.endIndex());
-  MachineInstr *LastMI_GRNar = LIS.getInstructionFromIndex(IntGRNar.endIndex());
-
-  // Allow cases (like PAIR128) where there is just one use of the narrow
-  // source reg defined close to MI.
-  if (WideOpNo == 0 && MRI->hasOneUse(GRNarReg) &&
-      FirstMI_GRNar && FirstMI_GRNar->getParent() == MBB &&
-      std::distance(FirstMI_GRNar->getIterator(), MI->getIterator()) < 5)
-    return true;
-
-  // Check that the two virtual registers are local to MBB.
-  if ((!FirstMI_GR128 || FirstMI_GR128->getParent() != MBB) ||
-      (!FirstMI_GRNar || FirstMI_GRNar->getParent() != MBB) ||
-      (!LastMI_GR128 || LastMI_GR128->getParent() != MBB) ||
-      (!LastMI_GRNar || LastMI_GRNar->getParent() != MBB))
+  MachineInstr *FirstMI = LIS.getInstructionFromIndex(LI.beginIndex());
+  MachineInstr *LastMI = LIS.getInstructionFromIndex(LI.endIndex());
+  if (!FirstMI || FirstMI->getParent() != MBB ||
+      !LastMI || LastMI->getParent() != MBB)
     return false;
 
-  MachineBasicBlock::iterator MII = nullptr, MEE = nullptr;
-  if (WideOpNo == 1) {
-    MII = FirstMI_GR128;
-    MEE = LastMI_GRNar;
-  } else {
-    MII = FirstMI_GRNar;
-    MEE = LastMI_GR128;
-  }
-
   // Check if coalescing seems safe by finding the set of clobbered physreg
   // pairs in the region.
   BitVector PhysClobbered(getNumRegs());
-  MEE++;
-  for (; MII != MEE; ++MII) {
+  for (MachineBasicBlock::iterator MII = FirstMI,
+                                   MEE = std::next(LastMI->getIterator());
+       MII != MEE; ++MII)
     for (const MachineOperand &MO : MII->operands())
       if (MO.isReg() && MO.getReg().isPhysical()) {
         for (MCPhysReg SI : superregs_inclusive(MO.getReg()))
@@ -445,7 +418,6 @@ bool SystemZRegisterInfo::shouldCoalesce(MachineInstr *MI,
             break;
           }
       }
-  }
 
   // Demand an arbitrary margin of free regs.
   unsigned const DemandedFreeGR128 = 3;



More information about the llvm-commits mailing list