[SystemZ] Improve emitSelect()

Merge more Select pseudo instructions in emitSelect() by allowing other instructions between them as long as they do not clobber CC. Debug value instructions are now moved down to below the new PHIs instead of erasing them. Review: Ulrich Weigand https://reviews.llvm.org/D67619 llvm-svn: 372873
2026-01-31 01:35:20 +01:00 · 2019-09-25 14:00:33 +00:00
parent c64a68b123
commit bf5edcf100
5 changed files with 150 additions and 103 deletions
--- a/lib/Target/SystemZ/SystemZISelLowering.cpp
+++ b/lib/Target/SystemZ/SystemZISelLowering.cpp
@@ -6563,19 +6563,17 @@ static bool isSelectPseudo(MachineInstr &MI) {

 // Helper function, which inserts PHI functions into SinkMBB:
 //   %Result(i) = phi [ %FalseValue(i), FalseMBB ], [ %TrueValue(i), TrueMBB ],
-// where %FalseValue(i) and %TrueValue(i) are taken from the consequent Selects
-// in [MIItBegin, MIItEnd) range.
-static void createPHIsForSelects(MachineBasicBlock::iterator MIItBegin,
-                                 MachineBasicBlock::iterator MIItEnd,
+// where %FalseValue(i) and %TrueValue(i) are taken from Selects.
+static void createPHIsForSelects(SmallVector<MachineInstr*, 8> &Selects,
                                 MachineBasicBlock *TrueMBB,
                                 MachineBasicBlock *FalseMBB,
                                 MachineBasicBlock *SinkMBB) {
  MachineFunction *MF = TrueMBB->getParent();
  const TargetInstrInfo *TII = MF->getSubtarget().getInstrInfo();

-  unsigned CCValid = MIItBegin->getOperand(3).getImm();
-  unsigned CCMask = MIItBegin->getOperand(4).getImm();
-  DebugLoc DL = MIItBegin->getDebugLoc();
+  MachineInstr *FirstMI = Selects.front();
+  unsigned CCValid = FirstMI->getOperand(3).getImm();
+  unsigned CCMask = FirstMI->getOperand(4).getImm();

  MachineBasicBlock::iterator SinkInsertionPoint = SinkMBB->begin();

@@ -6587,16 +6585,15 @@ static void createPHIsForSelects(MachineBasicBlock::iterator MIItBegin,
  // destination registers, and the registers that went into the PHI.
  DenseMap<unsigned, std::pair<unsigned, unsigned>> RegRewriteTable;

-  for (MachineBasicBlock::iterator MIIt = MIItBegin; MIIt != MIItEnd;
-       MIIt = skipDebugInstructionsForward(++MIIt, MIItEnd)) {
-    Register DestReg = MIIt->getOperand(0).getReg();
-    Register TrueReg = MIIt->getOperand(1).getReg();
-    Register FalseReg = MIIt->getOperand(2).getReg();
+  for (auto MI : Selects) {
+    Register DestReg = MI->getOperand(0).getReg();
+    Register TrueReg = MI->getOperand(1).getReg();
+    Register FalseReg = MI->getOperand(2).getReg();

    // If this Select we are generating is the opposite condition from
    // the jump we generated, then we have to swap the operands for the
    // PHI that is going to be generated.
-    if (MIIt->getOperand(4).getImm() == (CCValid ^ CCMask))
+    if (MI->getOperand(4).getImm() == (CCValid ^ CCMask))
      std::swap(TrueReg, FalseReg);

    if (RegRewriteTable.find(TrueReg) != RegRewriteTable.end())
@@ -6605,6 +6602,7 @@ static void createPHIsForSelects(MachineBasicBlock::iterator MIItBegin,
    if (RegRewriteTable.find(FalseReg) != RegRewriteTable.end())
      FalseReg = RegRewriteTable[FalseReg].second;

+    DebugLoc DL = MI->getDebugLoc();
    BuildMI(*SinkMBB, SinkInsertionPoint, DL, TII->get(SystemZ::PHI), DestReg)
      .addReg(TrueReg).addMBB(TrueMBB)
      .addReg(FalseReg).addMBB(FalseMBB);
@@ -6620,36 +6618,61 @@ static void createPHIsForSelects(MachineBasicBlock::iterator MIItBegin,
 MachineBasicBlock *
 SystemZTargetLowering::emitSelect(MachineInstr &MI,
                                  MachineBasicBlock *MBB) const {
+  assert(isSelectPseudo(MI) && "Bad call to emitSelect()");
  const SystemZInstrInfo *TII =
      static_cast<const SystemZInstrInfo *>(Subtarget.getInstrInfo());

  unsigned CCValid = MI.getOperand(3).getImm();
  unsigned CCMask = MI.getOperand(4).getImm();
-  DebugLoc DL = MI.getDebugLoc();

  // If we have a sequence of Select* pseudo instructions using the
  // same condition code value, we want to expand all of them into
  // a single pair of basic blocks using the same condition.
-  MachineInstr *LastMI = &MI;
-  MachineBasicBlock::iterator NextMIIt = skipDebugInstructionsForward(
-      std::next(MachineBasicBlock::iterator(MI)), MBB->end());
-
-  if (isSelectPseudo(MI))
-    while (NextMIIt != MBB->end() && isSelectPseudo(*NextMIIt) &&
-           NextMIIt->getOperand(3).getImm() == CCValid &&
-           (NextMIIt->getOperand(4).getImm() == CCMask ||
-            NextMIIt->getOperand(4).getImm() == (CCValid ^ CCMask))) {
-      LastMI = &*NextMIIt;
-      NextMIIt = skipDebugInstructionsForward(++NextMIIt, MBB->end());
+  SmallVector<MachineInstr*, 8> Selects;
+  SmallVector<MachineInstr*, 8> DbgValues;
+  Selects.push_back(&MI);
+  unsigned Count = 0;
+  for (MachineBasicBlock::iterator NextMIIt =
+         std::next(MachineBasicBlock::iterator(MI));
+       NextMIIt != MBB->end(); ++NextMIIt) {
+    if (NextMIIt->definesRegister(SystemZ::CC))
+      break;
+    if (isSelectPseudo(*NextMIIt)) {
+      assert(NextMIIt->getOperand(3).getImm() == CCValid &&
+             "Bad CCValid operands since CC was not redefined.");
+      if (NextMIIt->getOperand(4).getImm() == CCMask ||
+          NextMIIt->getOperand(4).getImm() == (CCValid ^ CCMask)) {
+        Selects.push_back(&*NextMIIt);
+        continue;
+      }
+      break;
    }
+    bool User = false;
+    for (auto SelMI : Selects)
+      if (NextMIIt->readsVirtualRegister(SelMI->getOperand(0).getReg())) {
+        User = true;
+        break;
+      }
+    if (NextMIIt->isDebugInstr()) {
+      if (User) {
+        assert(NextMIIt->isDebugValue() && "Unhandled debug opcode.");
+        DbgValues.push_back(&*NextMIIt);
+      }
+    }
+    else if (User || ++Count > 20)
+      break;
+  }

+  MachineInstr *LastMI = Selects.back();
+  bool CCKilled =
+      (LastMI->killsRegister(SystemZ::CC) || checkCCKill(*LastMI, MBB));
  MachineBasicBlock *StartMBB = MBB;
-  MachineBasicBlock *JoinMBB  = splitBlockBefore(MI, MBB);
+  MachineBasicBlock *JoinMBB  = splitBlockAfter(LastMI, MBB);
  MachineBasicBlock *FalseMBB = emitBlockAfter(StartMBB);

  // Unless CC was killed in the last Select instruction, mark it as
  // live-in to both FalseMBB and JoinMBB.
-  if (!LastMI->killsRegister(SystemZ::CC) && !checkCCKill(*LastMI, JoinMBB)) {
+  if (!CCKilled) {
    FalseMBB->addLiveIn(SystemZ::CC);
    JoinMBB->addLiveIn(SystemZ::CC);
  }
@@ -6658,7 +6681,7 @@ SystemZTargetLowering::emitSelect(MachineInstr &MI,
  //   BRC CCMask, JoinMBB
  //   # fallthrough to FalseMBB
  MBB = StartMBB;
-  BuildMI(MBB, DL, TII->get(SystemZ::BRC))
+  BuildMI(MBB, MI.getDebugLoc(), TII->get(SystemZ::BRC))
    .addImm(CCValid).addImm(CCMask).addMBB(JoinMBB);
  MBB->addSuccessor(JoinMBB);
  MBB->addSuccessor(FalseMBB);
@@ -6672,12 +6695,14 @@ SystemZTargetLowering::emitSelect(MachineInstr &MI,
  //   %Result = phi [ %FalseReg, FalseMBB ], [ %TrueReg, StartMBB ]
  //  ...
  MBB = JoinMBB;
-  MachineBasicBlock::iterator MIItBegin = MachineBasicBlock::iterator(MI);
-  MachineBasicBlock::iterator MIItEnd = skipDebugInstructionsForward(
-      std::next(MachineBasicBlock::iterator(LastMI)), MBB->end());
-  createPHIsForSelects(MIItBegin, MIItEnd, StartMBB, FalseMBB, MBB);
+  createPHIsForSelects(Selects, StartMBB, FalseMBB, MBB);
+  for (auto SelMI : Selects)
+    SelMI->eraseFromParent();
+
+  MachineBasicBlock::iterator InsertPos = MBB->getFirstNonPHI();
+  for (auto DbgMI : DbgValues)
+    MBB->splice(InsertPos, StartMBB, DbgMI);

-  MBB->erase(MIItBegin, MIItEnd);
  return JoinMBB;
 }

--- a/test/CodeGen/SystemZ/debuginstr-02.mir
+++ b/test/CodeGen/SystemZ/debuginstr-02.mir
@@ -1,13 +1,16 @@
 # Check that the backend can handle consecutive select instructions also in
-# the presence of DEBUG_VALUE machine instructions.
+# the presence of DEBUG_VALUE machine instructions, which should be moved.
 #
-# RUN: llc %s -verify-machineinstrs -mtriple=s390x-linux-gnu -mcpu=z13 \
-# RUN:   -start-before=finalize-isel -o - 2>&1 | FileCheck %s
+# RUN: llc %s -mtriple=s390x-linux-gnu -mcpu=z13 -run-pass=finalize-isel \
+# RUN:   -o - 2>&1 | FileCheck %s
 #
-# CHECK-LABEL: %bb.1:
-# CHECK:       ldr
-# CHECK-NEXT:  ldr
-# CHECK-NEXT:  ldr
+# CHECK-LABEL: bb.1 (%ir-block.0):
+# CHECK-NEXT: %5:fp32bit = PHI %1, %bb.0, %2, %bb.2
+# CHECK-NEXT: %6:fp32bit = PHI %3, %bb.0, %4, %bb.2
+# CHECK-NEXT: %7:fp32bit = PHI %1, %bb.0, %4, %bb.2
+# CHECK-NEXT: DBG_VALUE %5, $noreg, !5, !DIExpression(), debug-location !9
+# CHECK-NEXT: DBG_VALUE %6, $noreg, !5, !DIExpression(), debug-location !9
+# CHECK-NEXT: %8:fp32bit = AEBR %5, killed %6, implicit-def dead $cc, implicit $fpc

 --- |
  ; ModuleID = 'tc.ll'
--- a/test/CodeGen/SystemZ/fp-strict-conv-10.ll
+++ b/test/CodeGen/SystemZ/fp-strict-conv-10.ll
@@ -18,19 +18,14 @@ define i32 @f1(float %f) {
 ; CHECK-LABEL: f1:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    larl %r1, .LCPI0_0
-; CHECK-NEXT:    le %f2, 0(%r1)
-; CHECK-NEXT:    ler %f1, %f0
-; CHECK-NEXT:    sebr %f1, %f2
-; CHECK-NEXT:    cebr %f0, %f2
+; CHECK-NEXT:    le %f1, 0(%r1)
+; CHECK-NEXT:    cebr %f0, %f1
+; CHECK-NEXT:    lhi %r0, 0
 ; CHECK-NEXT:    jl .LBB0_2
 ; CHECK-NEXT:  # %bb.1:
-; CHECK-NEXT:    ler %f0, %f1
-; CHECK-NEXT:  .LBB0_2:
-; CHECK-NEXT:    lhi %r0, 0
-; CHECK-NEXT:    jl .LBB0_4
-; CHECK-NEXT:  # %bb.3:
+; CHECK-NEXT:    sebr %f0, %f1
 ; CHECK-NEXT:    llilh %r0, 32768
-; CHECK-NEXT:  .LBB0_4:
+; CHECK-NEXT:  .LBB0_2:
 ; CHECK-NEXT:    cfebr %r2, 5, %f0
 ; CHECK-NEXT:    xr %r2, %r0
 ; CHECK-NEXT:    br %r14
@@ -44,19 +39,14 @@ define i32 @f2(double %f) {
 ; CHECK-LABEL: f2:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    larl %r1, .LCPI1_0
-; CHECK-NEXT:    ldeb %f2, 0(%r1)
-; CHECK-NEXT:    ldr %f1, %f0
-; CHECK-NEXT:    sdbr %f1, %f2
-; CHECK-NEXT:    cdbr %f0, %f2
+; CHECK-NEXT:    ldeb %f1, 0(%r1)
+; CHECK-NEXT:    cdbr %f0, %f1
+; CHECK-NEXT:    lhi %r0, 0
 ; CHECK-NEXT:    jl .LBB1_2
 ; CHECK-NEXT:  # %bb.1:
-; CHECK-NEXT:    ldr %f0, %f1
-; CHECK-NEXT:  .LBB1_2:
-; CHECK-NEXT:    lhi %r0, 0
-; CHECK-NEXT:    jl .LBB1_4
-; CHECK-NEXT:  # %bb.3:
+; CHECK-NEXT:    sdbr %f0, %f1
 ; CHECK-NEXT:    llilh %r0, 32768
-; CHECK-NEXT:  .LBB1_4:
+; CHECK-NEXT:  .LBB1_2:
 ; CHECK-NEXT:    cfdbr %r2, 5, %f0
 ; CHECK-NEXT:    xr %r2, %r0
 ; CHECK-NEXT:    br %r14
@@ -72,19 +62,14 @@ define i32 @f3(fp128 *%src) {
 ; CHECK-NEXT:    ld %f0, 0(%r2)
 ; CHECK-NEXT:    ld %f2, 8(%r2)
 ; CHECK-NEXT:    larl %r1, .LCPI2_0
-; CHECK-NEXT:    lxeb %f4, 0(%r1)
-; CHECK-NEXT:    lxr %f1, %f0
-; CHECK-NEXT:    sxbr %f1, %f4
-; CHECK-NEXT:    cxbr %f0, %f4
+; CHECK-NEXT:    lxeb %f1, 0(%r1)
+; CHECK-NEXT:    cxbr %f0, %f1
+; CHECK-NEXT:    lhi %r0, 0
 ; CHECK-NEXT:    jl .LBB2_2
 ; CHECK-NEXT:  # %bb.1:
-; CHECK-NEXT:    lxr %f0, %f1
-; CHECK-NEXT:  .LBB2_2:
-; CHECK-NEXT:    lhi %r0, 0
-; CHECK-NEXT:    jl .LBB2_4
-; CHECK-NEXT:  # %bb.3:
+; CHECK-NEXT:    sxbr %f0, %f1
 ; CHECK-NEXT:    llilh %r0, 32768
-; CHECK-NEXT:  .LBB2_4:
+; CHECK-NEXT:  .LBB2_2:
 ; CHECK-NEXT:    cfxbr %r2, 5, %f0
 ; CHECK-NEXT:    xr %r2, %r0
 ; CHECK-NEXT:    br %r14
--- a/test/CodeGen/SystemZ/fp-strict-conv-12.ll
+++ b/test/CodeGen/SystemZ/fp-strict-conv-12.ll
@@ -17,19 +17,14 @@ define i64 @f1(float %f) {
 ; CHECK-LABEL: f1:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    larl %r1, .LCPI0_0
-; CHECK-NEXT:    le %f2, 0(%r1)
-; CHECK-NEXT:    ler %f1, %f0
-; CHECK-NEXT:    sebr %f1, %f2
-; CHECK-NEXT:    cebr %f0, %f2
+; CHECK-NEXT:    le %f1, 0(%r1)
+; CHECK-NEXT:    cebr %f0, %f1
+; CHECK-NEXT:    lghi %r0, 0
 ; CHECK-NEXT:    jl .LBB0_2
 ; CHECK-NEXT:  # %bb.1:
-; CHECK-NEXT:    ler %f0, %f1
-; CHECK-NEXT:  .LBB0_2:
-; CHECK-NEXT:    lghi %r0, 0
-; CHECK-NEXT:    jl .LBB0_4
-; CHECK-NEXT:  # %bb.3:
+; CHECK-NEXT:    sebr %f0, %f1
 ; CHECK-NEXT:    llihh %r0, 32768
-; CHECK-NEXT:  .LBB0_4:
+; CHECK-NEXT:  .LBB0_2:
 ; CHECK-NEXT:    cgebr %r2, 5, %f0
 ; CHECK-NEXT:    xgr %r2, %r0
 ; CHECK-NEXT:    br %r14
@@ -43,19 +38,14 @@ define i64 @f2(double %f) {
 ; CHECK-LABEL: f2:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    larl %r1, .LCPI1_0
-; CHECK-NEXT:    ldeb %f2, 0(%r1)
-; CHECK-NEXT:    ldr %f1, %f0
-; CHECK-NEXT:    sdbr %f1, %f2
-; CHECK-NEXT:    cdbr %f0, %f2
+; CHECK-NEXT:    ldeb %f1, 0(%r1)
+; CHECK-NEXT:    cdbr %f0, %f1
+; CHECK-NEXT:    lghi %r0, 0
 ; CHECK-NEXT:    jl .LBB1_2
 ; CHECK-NEXT:  # %bb.1:
-; CHECK-NEXT:    ldr %f0, %f1
-; CHECK-NEXT:  .LBB1_2:
-; CHECK-NEXT:    lghi %r0, 0
-; CHECK-NEXT:    jl .LBB1_4
-; CHECK-NEXT:  # %bb.3:
+; CHECK-NEXT:    sdbr %f0, %f1
 ; CHECK-NEXT:    llihh %r0, 32768
-; CHECK-NEXT:  .LBB1_4:
+; CHECK-NEXT:  .LBB1_2:
 ; CHECK-NEXT:    cgdbr %r2, 5, %f0
 ; CHECK-NEXT:    xgr %r2, %r0
 ; CHECK-NEXT:    br %r14
@@ -71,19 +61,14 @@ define i64 @f3(fp128 *%src) {
 ; CHECK-NEXT:    ld %f0, 0(%r2)
 ; CHECK-NEXT:    ld %f2, 8(%r2)
 ; CHECK-NEXT:    larl %r1, .LCPI2_0
-; CHECK-NEXT:    lxeb %f4, 0(%r1)
-; CHECK-NEXT:    lxr %f1, %f0
-; CHECK-NEXT:    sxbr %f1, %f4
-; CHECK-NEXT:    cxbr %f0, %f4
+; CHECK-NEXT:    lxeb %f1, 0(%r1)
+; CHECK-NEXT:    cxbr %f0, %f1
+; CHECK-NEXT:    lghi %r0, 0
 ; CHECK-NEXT:    jl .LBB2_2
 ; CHECK-NEXT:  # %bb.1:
-; CHECK-NEXT:    lxr %f0, %f1
-; CHECK-NEXT:  .LBB2_2:
-; CHECK-NEXT:    lghi %r0, 0
-; CHECK-NEXT:    jl .LBB2_4
-; CHECK-NEXT:  # %bb.3:
+; CHECK-NEXT:    sxbr %f0, %f1
 ; CHECK-NEXT:    llihh %r0, 32768
-; CHECK-NEXT:  .LBB2_4:
+; CHECK-NEXT:  .LBB2_2:
 ; CHECK-NEXT:    cgxbr %r2, 5, %f0
 ; CHECK-NEXT:    xgr %r2, %r0
 ; CHECK-NEXT:    br %r14
--- a/test/CodeGen/SystemZ/multiselect.ll
+++ b/test/CodeGen/SystemZ/multiselect.ll
@@ -1,10 +1,11 @@
 ; Test that multiple select statements using the same condition are expanded
-; into a single conditional branch.
+; into a single conditional branch when possible.
 ;
-; RUN: llc < %s -mtriple=s390x-linux-gnu | FileCheck %s
+; RUN: llc < %s -mtriple=s390x-linux-gnu -disable-block-placement | FileCheck %s

-define void @test(i32 signext %positive, double %base, double %offset, double* %rmin, double* %rmax) {
+define void @test0(i32 signext %positive, double %base, double %offset, double* %rmin, double* %rmax) {
 entry:
+; CHECK-LABEL: test0
 ; CHECK: cijlh %r2, 0,
 ; CHECK-NOT: cij
 ; CHECK-NOT: je
@@ -19,3 +20,51 @@ entry:
  ret void
 }

+; Two selects with an intervening instruction that doesn't clobber CC can
+; still be merged.
+define double @test1(i32 signext %positive, double %A, double %B, double %C) {
+entry:
+; CHECK-LABEL: test1
+; CHECK: cijhe {{.*}}LBB1_2
+; CHECK-NOT: cij
+; CHECK: br %r14
+
+  %tobool = icmp slt i32 %positive, 0
+  %s1  = select i1 %tobool, double %A, double %B
+  %mul = fmul double %A, %B
+  %s2  = select i1 %tobool, double %B, double %C
+  %add = fadd double %s1, %s2
+  %add2 = fadd double %add, %mul
+  ret double %add2
+}
+
+; Two selects with an intervening user of the first select can't be merged.
+define double @test2(i32 signext %positive, double %A, double %B) {
+entry:
+; CHECK-LABEL: test2
+; CHECK: cije {{.*}}LBB2_2
+; CHECK: cibe {{.*}}%r14
+; CHECK: br %r14
+
+  %tobool = icmp eq i32 %positive, 0
+  %s1  = select i1 %tobool, double %A, double %B
+  %add = fadd double %A, %s1
+  %s2  = select i1 %tobool, double %A, double %add
+  ret double %s2
+}
+
+; Two selects with different conditions can't be merged
+define double @test3(i32 signext %positive, double %A, double %B, double %C) {
+entry:
+; CHECK-LABEL: test3
+; CHECK: cijl {{.*}}LBB3_2
+; CHECK: cijl {{.*}}LBB3_4
+; CHECK: br %r14
+
+  %tobool = icmp slt i32 %positive, 0
+  %s1  = select i1 %tobool, double %A, double %B
+  %tobool2 = icmp slt i32 %positive, 2
+  %s2  = select i1 %tobool2, double %B, double %C
+  %add = fadd double %s1, %s2
+  ret double %add
+}