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

[Zhang Kang via Phabricator via llvm-commits]

ZhangKang marked 3 inline comments as done.
ZhangKang added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCExpandISEL.cpp:390
-    // 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
----------------
lkail wrote:
> I notice `computeAndAddLiveIns` will call `stepBackward`, this comment says it will lead to cyclic dependence. Is it still the case for current code?
If the livenins of successors are right, we can using `stepBackward` method to calculate the livenins for this MachineBasicBlock.
 Here, the successors of MBB are equal the to the successors of NewSuccessor, and the liveins of successors are right for MBB. So we can use `stepBackward` method to calculate the liveness for NewSuccessor.


================
Comment at: llvm/lib/Target/PowerPC/PPCExpandISEL.cpp:455
+      LivePhysRegs LPR;
+      TrueBlock->clearLiveIns();
+      computeAndAddLiveIns(LPR, *TrueBlock);
----------------
efriedma wrote:
> Is the clearLiveIns() call necessary?  I would have thought that TrueBlock has no liveins set, since you just created it.
If there are more than one ISEL for BIL, we will get assert error the TrueBlock has the liveins for last ISEL instruction.
For example:
```
bb.0.entry:
    liveins: $x4, $x5, $x6, $x7

    renamable $cr0 = CMPWI renamable $r7, 0
    renamable $r3 = ISEL killed renamable $r7, killed renamable $r4, renamable $cr0gt, implicit $x4, implicit $x7
    renamable $r4 = ISEL killed renamable $r5, killed renamable $r6, killed renamable $cr0gt, implicit $cr0, implicit $x6, implicit $x5
    renamable $r3 = nsw ADD4 killed renamable $r3, killed renamable $r4
    renamable $x3 = EXTSW_32_64 killed renamable $r3
    BLR8 implicit $lr8, implicit $rm, implicit killed $x3
```

We will get:
```
  bb.0.entry:
    successors: %bb.2(0x40000000), %bb.1(0x40000000)
    liveins: $x4, $x5, $x6, $x7

    renamable $cr0 = CMPWI renamable $r7, 0
    BC killed renamable $cr0gt, %bb.2

  bb.1.entry:
    successors: %bb.3(0x80000000)
    liveins: $r6, $r4

    renamable $r3 = ORI killed renamable $r4, 0
    renamable $r4 = ORI killed renamable $r6, 0
    B %bb.3

  bb.2.entry:
    successors: %bb.3(0x80000000)
    liveins: $r5, $r7

    renamable $r3 = ADDI killed renamable $r7, 0
    renamable $r4 = ADDI killed renamable $r5, 0

  bb.3.entry:
    liveins: $r3, $r4

    renamable $r3 = nsw ADD4 killed renamable $r3, killed renamable $r4
    renamable $x3 = EXTSW_32_64 killed renamable $r3
    BLR8 implicit $lr8, implicit $rm, implicit killed $x3
```

But your comment reminder me that I should calculate the liveins out of the for loop. I will update the patch.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78657/new/

https://reviews.llvm.org/D78657