[PATCH] D130903: [AArch64][GlobalISel] Lower formal arguments of AAPCS & ms_abi variadic functions.
Jessica Paquette via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 1 10:34:57 PDT 2022
paquette added inline comments.
================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp:553
+
+ static const MCPhysReg GPRArgRegs[] = {AArch64::X0, AArch64::X1, AArch64::X2,
+ AArch64::X3, AArch64::X4, AArch64::X5,
----------------
It looks like these arrays already exist in AArch64CallingConvention.cpp; is it possible to use them here? (Or should we?)
================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp:564
+ if (IsWin64CC) {
+ GPRIdx = MFI.CreateFixedObject(GPRSaveSize, -(int)GPRSaveSize, false);
+ if (GPRSaveSize & 15)
----------------
nit: can you use `static_cast` for greppability?
================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp:565
+ GPRIdx = MFI.CreateFixedObject(GPRSaveSize, -(int)GPRSaveSize, false);
+ if (GPRSaveSize & 15)
+ // The extra size here, if triggered, will always be 8.
----------------
Can you explain why this is the case in a comment? Even referencing some win64 doc would be useful?
================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp:568
+ MFI.CreateFixedObject(16 - (GPRSaveSize & 15),
+ -(int)alignTo(GPRSaveSize, 16), false);
+ } else
----------------
nit: `static_cast`?
================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp:572
+
+ auto FIN = MIRBuilder.buildFrameIndex(LLT::pointer(0, 64), GPRIdx);
+ auto Offset = MIRBuilder.buildConstant(
----------------
can you pull `LLT::pointer(0, 64)` out into like
```
const LLT p0 = LLT::pointer(0, 64);
```
and use that everywhere?
================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp:574
+ auto Offset = MIRBuilder.buildConstant(
+ MRI.createGenericVirtualRegister(LLT::scalar(64)), 8);
+
----------------
similar to above, can you create a
```
LLT s64 = LLT::scalar(64);
```
and use that everywhere?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130903/new/
https://reviews.llvm.org/D130903
More information about the llvm-commits
mailing list