[PATCH] D63547: [AIX]Global Address Lowering
Xiangling Liao via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 9 12:56:03 PDT 2019
Xiangling_L marked 3 inline comments as done.
Xiangling_L added inline comments.
================
Comment at: llvm/lib/Target/PowerPC/PPCTOCRegDeps.cpp:118
+ const bool isAIXABI = Subtarget.isAIXABI();
+ assert(((isPPC64 && isSVR4ABI) || isAIXABI) &&
+ "Only supported for 64-bit SVR4 ABI and AIX ABI under large code "
----------------
sfertile wrote:
> Xiangling_L wrote:
> > sfertile wrote:
> > > Does it help readability if we add some extra helpers to the subtarget combining ABI and pointer width checks?
> > > eg `is64BitELF()`, `is32BitAIX()` etc?
> > or we can do?:
> >
> > ```
> > assert(((isPPC64 && isTargetELF()) || isAIXABI) &&...
> > ```
> > and
> > ```
> > const bool is32BitAIX = !isPPC64 && isAIXABI;
> > const unsigned TOCReg = (is32BitAIX) ? PPC::X2 : PPC::R2;
> > ```
> Sure that works as well, stick with that for now. I think we already do a lot of checks like `isPPC64() && isSVR4ABI()` and with the addition of an AIX target we are going to end up with many more of these. I can post an NFC patch adding my suggestions and we evaluate if it helps or not in that context.
Thank you for doing that, though I just realized that I shouldn't have checked `isPPC64()` here, because there does exist 32bit+non-PIC situation[eg. LowerConstantPool] will take advantage of this function. But it seems weird to me that we use R2 for 32-bit. But I guess it's better to keep it that way and make changes for AIX only?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63547/new/
https://reviews.llvm.org/D63547
More information about the llvm-commits
mailing list