[PATCH] D77749: [PowerPC][Future] Remove redundant r2 save and restore for indirect call

Nemanja Ivanovic via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 13 05:52:48 PDT 2020


nemanjai added a comment.

There are 3 places that are concerned with whether or not we need to save/restore the TOC. There is certainly a possibility of these conditions diverging. Can you please write a small function that checks this and then use that in all 3 places?
Maybe something like

  static inline bool isTOCSaveRequired(const PPCSubtarget &Subtarget) {
    return Subtarget.isAIXABI() ||
           (Subtarget.is64BitELFABI() && !Subtarget.isUsingPCRelativeCalls());
  }

I suppose we don't really have to use it in `LowerCall_64SVR4()` since there we have already checked that we are on a 64-bit ELF, but I think it doesn't hurt to just use the same query everywhere.

Also, I think it would be useful to add a test case where the function pointer is passed as a parameter.



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:5425
     // operand: after the chain input but before any other variadic arguments.
-    if (Subtarget.is64BitELFABI() || Subtarget.isAIXABI()) {
+    // For 64-bit ELFv2 ABI PCRel, disable the code as TOC is not used.
+    if ((Subtarget.is64BitELFABI() && !Subtarget.isUsingPCRelativeCalls()) ||
----------------
I don't think we should say "disable the code". Maybe:
`// For ELFv2 ABI with PCRel, do not restore the TOC as it is not saved or used.`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77749





More information about the llvm-commits mailing list