[PATCH] D76042: [PowerPC] Add the Uses of implicit register for the BCLRn instruction

[Zhang Kang via Phabricator via llvm-commits]

ZhangKang marked an inline comment as done.
ZhangKang added inline comments.


================
Comment at: llvm/test/CodeGen/PowerPC/early-ret.mir:35
   ; CHECK:   renamable $cr0 = FCMPUS killed renamable $f3, killed renamable $f4
-  ; CHECK:   BCLRn $cr0eq, implicit $lr, implicit $rm, implicit killed $v2
+  ; CHECK:   BCLRn $cr0eq, implicit $lr, implicit $rm, implicit $lr, implicit $rm, implicit killed $v2
   ; CHECK: bb.1.entry:
----------------
shchenz wrote:
> The test change is a little confusing for me. Before your code changes, there should be no use of `lr` and `rm`. But as you can see, there is.
> 
> After your change, there are two uses of `lr` and `rm`?
You can see the line 76 of this case:
```
; CHECK:   BCLR $cr0eq, implicit $lr, implicit $rm, implicit $lr, implicit $rm, implicit killed $v2
```

In fact, the line 35 and line 76 have exposed a bug of the pass `ppc-early-ret`.

In the file `llvm/llvm/lib/Target/PowerPC/PPCEarlyReturn.cpp`
```
102           } else if (J->getOpcode() == PPC::BC || J->getOpcode() == PPC::BCn) {
103             if (J->getOperand(1).getMBB() == &ReturnMBB) {
104               // This is a conditional branch to the return. Replace the branch
105               // with a bclr.
106               BuildMI(
107                   **PI, J, J->getDebugLoc(),
108                   TII->get(J->getOpcode() == PPC::BC ? PPC::BCLR : PPC::BCLRn))
109                   .addReg(J->getOperand(0).getReg())
110                   .copyImplicitOps(*I);
111               MachineBasicBlock::iterator K = J--;
112               K->eraseFromParent();
113               BlockChanged = true;
114               ++NumBCLR;
115               continue;
116             }
117           } else if (J->isBranch()) {

This code do the early return for `BC/BCn + BLR =====> BCLR/BCLRn`.
For our test case, the I is `BLR implicit $lr, implicit $rm, implicit killed $v2`.

In the line `110                   .copyImplicitOps(*I);`, 
we copy all implicit operands from BLR instruction to the new BLCRn, namely, we copy `implicit $lr, implicit $rm, implicit killed $v2` to the new BCLRn instruction.

Note that the `BuildMI()` function will add the implicit operands automatically if the instruction definition need use implicit operands in the td file.
.
For this patch, we add `Uses = [LR, RM]` for the `BCLRn` instruction definition. Namely, the `BuildMI()` function will add `implicit $lr, implicit $rm` to the new BCLRn automatically.

So you will see `BCLRn $cr0eq, implicit $lr, implicit $rm, implicit $lr, implicit $rm, implicit killed $v2`. The `implicit $lr, implicit $rm,` is added by `BuildMI`, and the `implicit $rm, implicit $lr, implicit $rm, implicit killed $v2` is `copyImplicitOps` from the BLR instruction. You can see the line 76 of this case to verify this.

To fix the bug of `ppc-early-ret` pass, There are two methods.
1. Don't use `copyImplicitOps` function, just copy the needed implicit form the BLR instruction, not all implicit registers.
2. Use the fuction `removeOperand()` to remove the redundant implicit register.
The bug will be fixed in the other patch, not this one.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76042