[PATCH] D61417: [NFC] Refactor TargetCallingConv so calling conventions can see which parameters are varargs

Wed May 1 18:12:02 PDT 2019

comex created this revision.
comex added reviewers: chandlerc, t.p.northover.
Herald added subscribers: llvm-commits, jsji, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, atanasyan, edward-jones, zzheng, jrtc27, niosHD, sabuasal, apazos, simoncook, johnrusso, rbar, asb, fedor.sergeev, kbarton, aheejin, kristof.beyls, jgravelle-google, arichardson, sbc100, javed.absar, nemanjai, sdardis, jyknight, dschuff.
Herald added a project: LLVM.

Currently, the OutputArg struct has a flag IsFixed, where "fixed" means "not a variable argument".  But the value of this flag is not provided to calling convention functions.  As a result, 3 different targets (Mips, SystemZ, Hexagon) define their own "CCIfArgIsVarArg" or equivalent, of which 2 work by maintaining a separate std::vector of which arguments are variable, while the other uses a subclass of CCState.  In addition, at least AArch64 avoids using CCState::AnalyzeCallOperands entirely, seemingly because of this issue; instead, it does its own loop over the arguments, choosing a calling convention for each argument based on IsFixed (as well as other checks that don't vary by argument; see AArch64TargetLowering::CCAssignFnForCall).

As far as I can tell, there's no good reason for things to be this way.  OutputArg has another field Flags, of type ArgFlagsTy, which already tracks a wide variety of information about each argument, and which *is* passed to calling convention functions.  This patch simply moves IsFixed to be a flag within ArgFlagsTy (renaming it IsVarArg for consistency).  Then it adds non-target-specific CC interfaces `CCIfArgIsVarArg` and `CCIfArgNotVarArg`, which can now just check `ArgFlags.isVarArg()` rather than doing anything fancy, and removes the aforementioned target-specific definitions in favor of them.

Why the verbose names?  Because there are already interfaces `CCIfVarArg` and `CCIfNotVarArg`, which test whether the *function* is declared as receiving variable arguments, as opposed to the current argument being variable.  To avoid confusion, this patch also renames those to `CCIfFuncIsVarArg` and `CCIfFuncNotVarArg`.

One caveat is that ArgFlagsTy is also used for input arguments, which cannot be varargs, but that's no big deal since isVarArg() will just return false.

I also needed to refactor some code in the Mips backend: it was relying on the fact that InputArg and OutputArg happened to have the same type signature, even though the meaning of the parameters was different!  Specifically, InputArg has a bool parameter Used, while OutputArg had a bool parameter IsFixed in the same place.  With IsFixed removed, the code broke.  I refactored it to use a callback instead of a template: a bit ugly, but so was the old version.

However, this patch doesn't fix all the hacks I noticed.  MipsCCState.h and SystemZCallingConv.h both have comments claiming AnalyzeCallOperands "is not usable since we must provide a means of accessing" IsFixed, yet the replacement functions defined in those files tracked other things in addition to IsFixed.  I've removed the CallOperandIsFixed vector from both, but I haven't investigated whether the other vectors can be removed too and the code refactored to use AnalyzeCallOperands, so instead I added TODO comments.  I also haven't refactored the AArch64 code to use AnalyzeCallOperands.

Repository:
  rL LLVM

https://reviews.llvm.org/D61417

Files:
  docs/WritingAnLLVMBackend.rst
  include/llvm/CodeGen/TargetCallingConv.h
  include/llvm/Target/TargetCallingConv.td
  lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  lib/CodeGen/TargetLoweringBase.cpp
  lib/Target/AArch64/AArch64ISelLowering.cpp
  lib/Target/Hexagon/HexagonCallingConv.td
  lib/Target/Hexagon/HexagonISelLowering.cpp
  lib/Target/Lanai/LanaiCallingConv.td
  lib/Target/Mips/MipsCCState.cpp
  lib/Target/Mips/MipsCCState.h
  lib/Target/Mips/MipsCallLowering.cpp
  lib/Target/Mips/MipsCallLowering.h
  lib/Target/Mips/MipsCallingConv.td
  lib/Target/PowerPC/PPCISelLowering.cpp
  lib/Target/RISCV/RISCVISelLowering.cpp
  lib/Target/Sparc/SparcISelLowering.cpp
  lib/Target/SystemZ/SystemZCallingConv.h
  lib/Target/SystemZ/SystemZCallingConv.td
  lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  lib/Target/X86/X86CallingConv.td

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D61417.197683.patch
Type: text/x-patch
Size: 29952 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190502/6a5f965d/attachment-0001.bin>