[PATCH] D78657: [PowerPC] Fix the liveins for ppc-expand-isel pass

[Zhang Kang via Phabricator via llvm-commits]

ZhangKang created this revision.
ZhangKang added reviewers: jsji, nemanjai, hfinkel, efriedma, PowerPC.
ZhangKang added a project: LLVM.
Herald added subscribers: shchenz, wuzish, hiraditya.

In the `ppc-expand-isel` pass, we use `stepForward()` to update the liveness, this function is not recommended, because it need the accurate kill info.

In fact, we have the function `computeAndAddLiveIns()`, it will use stepBackward() to update the liveness. It's the recommended method to update the liveins.

I have use the option machine-verifier-strict-livein provided in https://reviews.llvm.org/D78586/ to test this patch. 
In the patch D78586 <https://reviews.llvm.org/D78586>, there are 57 PowerPC cases failed, after using this patch, 49 cases can pass, and the left 8 cases should be the cases issue.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78657

Files:
  llvm/lib/Target/PowerPC/PPCExpandISEL.cpp
  llvm/test/CodeGen/PowerPC/expand-isel-liveness.mir


Index: llvm/test/CodeGen/PowerPC/expand-isel-liveness.mir
===================================================================
--- llvm/test/CodeGen/PowerPC/expand-isel-liveness.mir
+++ llvm/test/CodeGen/PowerPC/expand-isel-liveness.mir
@@ -38,13 +38,13 @@
 
   ; CHECK-LABEL: expand_isel_liveness
   ; CHECK: bb.1:
-  ; CHECK:   liveins: $x7
+  ; CHECK:   liveins: $x3, $x4, $x7
   ; CHECK:   renamable $x5 = ORI8 killed renamable $x7, 0
   ; CHECK:   B %bb.3
   ; CHECK: bb.2:
-  ; CHECK:   liveins: $zero8
+  ; CHECK:   liveins: $x3, $x4
   ; CHECK:   renamable $x5 = ADDI8 $zero8, 0
   ; CHECK: bb.3:
-  ; CHECK:   liveins: $x3, $x4, $x5, $x6, $cr1lt, $cr1gt, $x3, $cr6lt, $cr0eq, $r3, $cr5un, $cr1eq, $cr1un, $cr6un, $cr0lt, $cr0gt, $cr6gt, $cr0un, $cr1, $cr6, $cr5eq, $x8, $r8, $cr6eq, $x4, $r4, $cr0, $cr5gt, $cr5, $cr5lt, $x7, $r7, $x5, $r5, $x5, $zero8, $x7, $cr5lt
+  ; CHECK:   liveins: $x3, $x4, $x5
   ; CHECK:   BLR8 implicit $lr8, implicit $rm, implicit killed $x3, implicit killed $x4, implicit killed $x5
 ...
Index: llvm/lib/Target/PowerPC/PPCExpandISEL.cpp
===================================================================
--- llvm/lib/Target/PowerPC/PPCExpandISEL.cpp
+++ llvm/lib/Target/PowerPC/PPCExpandISEL.cpp
@@ -381,21 +381,10 @@
                          MBB->end());
     NewSuccessor->transferSuccessorsAndUpdatePHIs(MBB);
 
-    // Copy the original liveIns of MBB to NewSuccessor.
-    for (auto &LI : MBB->liveins())
-      NewSuccessor->addLiveIn(LI);
-
-    // After splitting the NewSuccessor block, Regs defined but not killed
-    // in MBB should be treated as liveins of NewSuccessor.
-    // Note: Cannot use stepBackward instead since we are using the Reg
-    // liveness state at the end of MBB (liveOut of MBB) as the liveIn for
-    // NewSuccessor. Otherwise, will cause cyclic dependence.
-    LivePhysRegs LPR(*MF->getSubtarget<PPCSubtarget>().getRegisterInfo());
-    SmallVector<std::pair<MCPhysReg, const MachineOperand *>, 2> Clobbers;
-    for (MachineInstr &MI : *MBB)
-      LPR.stepForward(MI, Clobbers);
-    for (auto &LI : LPR)
-      NewSuccessor->addLiveIn(LI);
+    // Update the liveins for NewSuccessor.
+    LivePhysRegs LPR;
+    computeAndAddLiveIns(LPR, *NewSuccessor);
+
   } else {
     // Remove successor from MBB.
     MBB->removeSuccessor(Successor);
@@ -461,31 +450,26 @@
           .add(TrueValue)
           .add(MachineOperand::CreateImm(0));
 
-      // Add the LiveIn registers required by true block.
-      TrueBlock->addLiveIn(TrueValue.getReg());
+      // Update the liveins for TrueBlock.
+      LivePhysRegs LPR;
+      TrueBlock->clearLiveIns();
+      computeAndAddLiveIns(LPR, *TrueBlock);
     }
 
     if (IsORIInstRequired) {
-      // Add the LiveIn registers required by false block.
-      FalseBlock->addLiveIn(FalseValue.getReg());
-    }
-
-    if (NewSuccessor) {
-      // Add the LiveIn registers required by NewSuccessor block.
-      NewSuccessor->addLiveIn(Dest.getReg());
-      NewSuccessor->addLiveIn(TrueValue.getReg());
-      NewSuccessor->addLiveIn(FalseValue.getReg());
-      NewSuccessor->addLiveIn(ConditionRegister.getReg());
-    }
-
-    // Copy the value into the destination if the condition is false.
-    if (IsORIInstRequired)
+      // Copy the value into the destination.
       BuildMI(*FalseBlock, FalseBlockI, dl,
               TII->get(isISEL8(*MI) ? PPC::ORI8 : PPC::ORI))
           .add(Dest)
           .add(FalseValue)
           .add(MachineOperand::CreateImm(0));
 
+      // Update the liveins for FalseBlock.
+      LivePhysRegs LPR;
+      FalseBlock->clearLiveIns();
+      computeAndAddLiveIns(LPR, *FalseBlock);
+    }
+
     MI->eraseFromParent(); // Remove the ISEL instruction.
 
     NumExpanded++;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D78657.259339.patch
Type: text/x-patch
Size: 3748 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200422/00002e91/attachment-0001.bin>