[PowerPC] Fix the liveins for ppc-early-ret pass

[ZhangKang]

Summary

The pass ppc-early-ret doesn't update the liveins after doing early return. It will cause assertion error when enable verify-machineinstrs.

Fo below mir:

bb.2:
  successors: %bb.3(0x80000000)
  liveins: $zero8

  renamable $x5 = ADDI8 $zero8, 0

bb.3:
  liveins: $x3, $x4, $x5

  BLR8 implicit $lr8, implicit $rm, implicit killed $x3, implicit killed $x4, implicit killed $x5

After doing ppc-early-ret, we will get:

bb.2:
  liveins: $zero8

  renamable $x5 = ADDI8 $zero8, 0
  BLR8 implicit $lr8, implicit $rm, implicit killed $x3, implicit killed $x4, implicit killed $x5

Above result will cause assertion error: Using an undefined physical register, because we have use undef $x3, $x4, $x5 in bb.2.

The right result should be:

bb.2:
  liveins: $zero8, $x3, $x4, $x5

  renamable $x5 = ADDI8 $zero8, 0
  BLR8 implicit $lr8, implicit $rm, implicit killed $x3, implicit killed $x4, implicit killed $x5

This patch is to fix the liveins for ppc-early-ret pass.

[efriedma]

A physical register is "livein" when on entry to that block, the register contains some meaningful value, and that value is used in that block or one of its successors. Moving an instruction at the end of a block can't affect the liveness at the beginning of that block. If the register didn't contain a meaningful value before, it still won't contain a meaningful value afterwards.

Please take another look at the input and output to ppc-early-ret, and try to figure out what the correct liveness actually looks like. Blindly calling addLiveIn might satisfy the verifier, but it won't make the result correct.

[ZhangKang]

In D78440#1990955, @efriedma wrote:

A physical register is "livein" when on entry to that block, the register contains some meaningful value, and that value is used in that block or one of its successors. Moving an instruction at the end of a block can't affect the liveness at the beginning of that block. If the register didn't contain a meaningful value before, it still won't contain a meaningful value afterwards.

Please take another look at the input and output to ppc-early-ret, and try to figure out what the correct liveness actually looks like. Blindly calling addLiveIn might satisfy the verifier, but it won't make the result correct.

Yes, you are right. The real error is in ppc-expand-isel pass. I have create the patch https://reviews.llvm.org/D78657 for this. So I will close this patch.

[ZhangKang]

The new patch is https://reviews.llvm.org/D78657

llvm/lib/Target/PowerPC/PPCEarlyReturn.cpp

bool processBlock(MachineBasicBlock &ReturnMBB) {
         if ((*PI)->canFallThrough() && (*PI)->isLayoutSuccessor(&ReturnMBB))
           OtherReference = true;
 
         // Predecessors are stored in a vector and can't be removed here.
         if (!OtherReference && BlockChanged) {
           PredToRemove.push_back(*PI);
         }
 
-        if (BlockChanged)
+        if (BlockChanged) {
+          // Update the liveins for *PI.
+          for (const MachineOperand &MO : I->operands()) {
+            if (!MO.isReg())
+              continue;
+            Register Reg = MO.getReg();
+            if (!Reg || !ReturnMBB.isLiveIn(Reg) || (*PI)->isLiveIn(Reg))
+              continue;
+            (*PI)->addLiveIn(Reg);
+          }
+
           Changed = true;
         }
+      }
 
       for (unsigned i = 0, ie = PredToRemove.size(); i != ie; ++i)
         PredToRemove[i]->removeSuccessor(&ReturnMBB, true);
 
       if (Changed && !ReturnMBB.hasAddressTaken()) {
         // We now might be able to merge this blr-only block into its
         // by-layout predecessor.
         if (ReturnMBB.pred_size() == 1) {
           MachineBasicBlock &PrevMBB = **ReturnMBB.pred_begin();
           if (PrevMBB.isLayoutSuccessor(&ReturnMBB) && PrevMBB.canFallThrough()) {
+            // Update the liveins for PrevMBB.
+            for (const MachineOperand &MO : I->operands()) {
+              if (!MO.isReg())
+                continue;
+              Register Reg = MO.getReg();
+              if (!Reg || !ReturnMBB.isLiveIn(Reg) || PrevMBB.isLiveIn(Reg))
+                continue;
+              PrevMBB.addLiveIn(Reg);
+            }
+
             // Move the blr into the preceding block.
             PrevMBB.splice(PrevMBB.end(), &ReturnMBB, I);
             PrevMBB.removeSuccessor(&ReturnMBB, true);
           }
         }
 
         if (ReturnMBB.pred_empty())
           ReturnMBB.eraseFromParent();

llvm/test/CodeGen/PowerPC/early-ret-liveness.mir

+# RUN: llc -mtriple powerpc64le-unknown-linux-gnu -run-pass=ppc-early-ret -o \
+# RUN:   - %s -verify-machineinstrs | FileCheck %s
+
+---
+name:            early-ret-liveness
+tracksRegLiveness: true
+hasWinCFI:       false
+registers:       []
+liveins:
+  - { reg: '$x3', virtual-reg: '' }
+  - { reg: '$x4', virtual-reg: '' }
+  - { reg: '$x5', virtual-reg: '' }
+  - { reg: '$x6', virtual-reg: '' }
+body:             |
+  bb.0:
+    successors: %bb.2(0x40000000), %bb.1(0x40000000)
+    liveins: $x3, $x4, $x5, $x6
+  
+    renamable $x8 = MULLD renamable $x5, renamable $x4
+    renamable $cr5 = CMPDI renamable $x3, 0
+    dead renamable $x9 = MULHDU_rec renamable $x3, renamable $x6, implicit-def $cr0
+    renamable $x3 = MULLD killed renamable $x3, renamable $x6
+    $cr1 = MCRF killed $cr0
+    renamable $x3 = ADD8 killed renamable $x3, killed renamable $x8
+    renamable $cr0 = CMPDI renamable $x5, 0
+    renamable $cr5lt = CRNOR killed renamable $cr0eq, killed renamable $cr5eq, implicit $cr5, implicit $cr0
+    renamable $cr0 = CMPLDI renamable $x3, 0
+    renamable $x8 = MULHDU renamable $x4, renamable $x6
+    renamable $x3 = ADD8 renamable $x8, killed renamable $x3
+    renamable $cr6 = CMPLD renamable $x3, killed renamable $x8
+    renamable $cr5gt = CRANDC killed renamable $cr6lt, killed renamable $cr0eq, implicit $cr0, implicit $cr6
+    renamable $cr5lt = CRORC killed renamable $cr5lt, killed renamable $cr1eq, implicit $cr1
+    renamable $x7 = LI8 1
+    dead renamable $x5 = MULHDU_rec killed renamable $x5, renamable $x4, implicit-def $cr0
+    renamable $cr5lt = CRORC killed renamable $cr5lt, killed renamable $cr0eq, implicit $cr0
+    renamable $cr5lt = CRNOR killed renamable $cr5lt, killed renamable $cr5gt
+    renamable $x4 = MULLD killed renamable $x4, killed renamable $x6
+    BC killed renamable $cr5lt, %bb.2
+  
+  bb.1:
+    successors: %bb.3(0x80000000)
+    liveins: $x7
+  
+    renamable $x5 = ORI8 killed renamable $x7, 0
+    B %bb.3
+  
+  bb.2:
+    successors: %bb.3(0x80000000)
+    liveins: $zero8
+  
+    renamable $x5 = ADDI8 $zero8, 0
+  
+  bb.3:
+    liveins: $x3, $x4, $x5
+  
+    BLR8 implicit $lr8, implicit $rm, implicit killed $x3, implicit killed $x4, implicit killed $x5
+
+  ; CHECK-LABEL: early-ret-liveness
+  ; CHECK: bb.1:
+  ; CHECK:   liveins: $x7, $x3, $x4, $x5
+  ; CHECK:   renamable $x5 = ORI8 killed renamable $x7, 0
+  ; CHECK:   BLR8 implicit $lr8, implicit $rm, implicit $lr8, implicit $rm, implicit killed $x3, implicit killed $x4, implicit killed $x5
+  ; CHECK: bb.2:
+  ; CHECK:   liveins: $zero8, $x3, $x4, $x5
+  ; CHECK:   renamable $x5 = ADDI8 $zero8, 0
+  ; CHECK:   BLR8 implicit $lr8, implicit $rm, implicit killed $x3, implicit killed $x4, implicit killed $x5
+...