[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