[PATCH] D101657: [PowerPC] Handle inline assembly clobber of link regsiter

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added inline comments.


================
Comment at: llvm/test/CodeGen/PowerPC/ppc64-inlineasm-clobber.ll:76-94
+; PPC64LE-LABEL: ClobberLR_BR:
+; PPC64LE:       # %bb.0: # %entry
+; PPC64LE-NEXT:    #APP
+; PPC64LE-NEXT:    nop
+; PPC64LE-NEXT:    #NO_APP
+; PPC64LE-NEXT:  # %bb.1: # %return
+; PPC64LE-NEXT:    extsw r3, r3
----------------
nickdesaulniers wrote:
> @jyknight points out in review of D115688 that something looks fishy with this `callbr` unit test, and I think he's right (and I'm sorry I didn't catch this earlier in code review).
> 
> Let's say the asm string actually writes to `lr` rather than being simply a `nop` in this example, hence why it's in the clobber list.  In that case, branching to the lr (returning) will not branch to the restored value of `lr`, but the one updated from the inline asm.  I don't think that's correct.
> 
> Should the `mflr`/`mtlr` "surround" the user supplied asm? Otherwise this looks like a pointless spill+reload on the indirect branch.
Looks like `prologepilog` insertion is where things go wrong, perhaps?

That said, running just this IR function through:

```
$ llc -mtriple=powerpc64le-unknown-linux-unknown -verify-machineinstrs /android0/llvm-project/llvm/test/CodeGen/PowerPC/ppc64-inlineasm-clobber.ll  -ppc-asm-full-reg-names -stop-before=prologepilog -o callbr.mir
$ llc -run-pass=prologepilog callbr.mir -o -
```
Doesn't produce the same output as
```
$ llc -mtriple=powerpc64le-unknown-linux-unknown -verify-machineinstrs /android0/llvm-project/llvm/test/CodeGen/PowerPC/ppc64-inlineasm-clobber.ll  -ppc-asm-full-reg-names -stop-after=prologepilog -o -
```
does (when observing the results of `prologepilog`)...I must be holding MIR wrong. Though it looks like shrink-wrap may also be involved, IIUC?

```
diff --git a/llvm/lib/CodeGen/ShrinkWrap.cpp b/llvm/lib/CodeGen/ShrinkWrap.cpp
index f89069e9f728..67d6bc249ef9 100644
--- a/llvm/lib/CodeGen/ShrinkWrap.cpp
+++ b/llvm/lib/CodeGen/ShrinkWrap.cpp
@@ -504,6 +504,10 @@ bool ShrinkWrap::runOnMachineFunction(MachineFunction &MF) {
     }
 
     for (const MachineInstr &MI : MBB) {
+      if (MI.getOpcode() == TargetOpcode::INLINEASM_BR) {
+        LLVM_DEBUG(dbgs() << "inlineasm_br prevents shrink-wrapping\n");
+        return false;
+      }
       if (!useOrDefCSROrFI(MI, RS.get()))
         continue;
       // Save (resp. restore) point must dominate (resp. post dominate)
```
seems to fix this, but I suspect it's overly conservative? Let's discuss in D116424?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101657