[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