[PATCH] D41700: [RISCV] Refactory the existing CC_RISCV32 function to conform to the CCAssignFn type

Alex Bradbury via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 3 07:21:48 PST 2018


asb added a comment.

Thanks for the patch Leslie. My main concern with this approach is it will fail to work when VarArg support is added (https://reviews.llvm.org/D40805). Conforming to the vararg ABI requires 1) knowing which arguments are fixed vs non-fixed (I currently just have an extra parameter for this in CC_RISCV, Mips handles this with MipsCCState), 2) knowing the original type of an argument. Both issues could be addressed by introducing a RISCVCCState (where I'd probably keep a Datalayout and a SubtargetInfo reference), but looking at GlobalISel that wouldn't actually help without further changes - CallLowering::handleAssignments creates a CCState directly, meaning you'd need to make further changes to support a custom CCState (as used in Hexagon, SystemZ, Mips and other backends).

I think some more GlobalISel design work needs to be done here really. It looks like splitting a smaller `assignArgs` function out of `handleAssignments` and making it virtual would help, but still wouldn't provide complete drop-in compatibility with custom CCState subclasses.


Repository:
  rL LLVM

https://reviews.llvm.org/D41700





More information about the llvm-commits mailing list