[PATCH] D81126: [PowerPC] Fix for PC Relative call protocol

Mon Jun 29 09:10:17 PDT 2020

sfertile added inline comments.

================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:4723

+  // If the callee is not dso-local then they caller and callee might not share
+  // toc-bases.
----------------
I think we can pare down this comment. 

The example does a  good job at showing why its the callees pre-emptability that matters rather then if the caller/callee are in the same section. I'm not sure that in itself is useful enough to warrant a long comment in the code though. I think the relationship is straightforward enough: 

```If the callee is preemptable, then the static linker will use a plt-stub which saves the toc to the stack, and needs a nop after the call instruction to convert to a toc-restore. ```

In this context the given example needing a toc-restore or being un-tail-callable is obvious. I think we should replace the existing comment with something similar to what I used above.

================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:4757
+  // the TOC.
+  if (!F && !Alias)
+    return false;
----------------
This check is redundant.

================
Comment at: llvm/test/CodeGen/PowerPC/func-alias.ll:4
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:   -mcpu=pwr9 -ppc-asm-full-reg-names < %s | FileCheck %s --check-prefix=P9
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
----------------
Minor nit: I think having P9 and P10 cases is enough coverage.

================
Comment at: llvm/test/CodeGen/PowerPC/ifunc.ll:7
+; RUN:   -verify-machineinstrs | FileCheck --check-prefix=LEP8 %s
+; RUN: llc %s -o - -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr9 \
+; RUN:   -verify-machineinstrs | FileCheck --check-prefix=LEP9 %s
----------------
Minor nit: I don't think we need a P9 run. P8 and P10 are enough.

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

https://reviews.llvm.org/D81126