[PATCH] D65814: [PowerPC] Add combined ELF ABI and 32/64 bit queries to the Subtarget [NFC]
Nemanja Ivanovic via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 21 00:59:34 PDT 2019
nemanjai accepted this revision.
nemanjai added a comment.
This revision is now accepted and ready to land.
LGTM other than a couple of minor nits. Thanks for cleaning this up.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:3389
+
+ // We are using this for both AIX and Darwin. If this is intentional then we
+ // should rename it appropriately.
----------------
Yes, this is a temporary situation as we work towards a functioning AIX compiler. It may be more appropriate to convert this comment to a FIXME.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:4931
bool isSVR4ABI = Subtarget.isSVR4ABI();
- bool isELFv2ABI = Subtarget.isELFv2ABI();
+ bool isELFv1ABI = isPPC64 && isSVR4ABI && !Subtarget.isELFv2ABI();
bool isAIXABI = Subtarget.isAIXABI();
----------------
I think the 64-bit matters here and the name should reflect it. Perhaps `is64BitELFv1ABI`?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65814/new/
https://reviews.llvm.org/D65814
More information about the llvm-commits
mailing list