[llvm-dev] Options for custom CCState, CCAssignFn, and GlobalISel

[Simon Dardis via llvm-dev]

> -----Original Message-----
> From: daniel_l_sanders at apple.com [mailto:daniel_l_sanders at apple.com]
> Sent: 05 January 2018 15:57
> To: Alex Bradbury
> Cc: llvm-dev; Leslie Zhai; Simon Dardis
> Subject: Re: [llvm-dev] Options for custom CCState, CCAssignFn, and
> GlobalISel
> 
> 
> 
> > On 4 Jan 2018, at 10:51, Alex Bradbury <asb at lowrisc.org> wrote:
> >
> > On 4 January 2018 at 17:10, Daniel Sanders via llvm-dev
> > <llvm-dev at lists.llvm.org> wrote:
> >>> On 3 Jan 2018, at 14:00, Alex Bradbury via llvm-dev <llvm-
> dev at lists.llvm.org> wrote:
> >> I haven't dug into the GlobalISel calling convention code much but I can
> comment on the MipsCCState.
> >
> > Thanks for the insight Daniel, much appreciated.
> >
> >>> * MipsCCState: adds bool vectors OriginalArgWasF128,
> >>> OriginalArgWasFloat, OriginalArgWasFloatVector,
> >>> OriginalRetWasFloatVector, CallOperandIsFixed. Also a
> >>> SpeciallCallingConv field. Provides its own implementation of
> AnalyzeFormalArguments etc that fill these vectors.
> >>
> >> CallOperandIsFixed was needed because the CCIf* classes could tell
> whether the argument list was variable or not, but couldn't tell whether a
> particular argument was part of the variable portion of the argument list. At
> the time, it struck me as odd that this information wasn't part of ArgFlagsTy
> but I didn't want to add it there because it appeared to be full and I didn't
> want to make everyone have a 65-bit ArgFlagsTy just because of a Mips
> quirk. I see Reid Kleckner changed this object last year and found that it
> wasn't actually full so maybe we should move OutputArg::IsFixed into
> ArgFlagsTy now.
> >
> > AArch64, Hexagon, RISCV, and SystemZ all have the same requirement.
> > AArch64 works around it by calling its CCAssignFnForCall helper for
> > every argument (and passing IsFixed through to that). For GISel, it
> > overrides assignArg so a different function can be called for varargs.
> >
> > I'd be in favour of adding IsFixed to ArgFlagsTy even if it did
> > "overflow" ArgFlagsTy. I know there's an argument about "death by a
> > thousand papercuts", but is the size of ArgFlagsTy really critical
> > anyway?
> 
> Probably not, but someone had gone to a lot of trouble to manually pack it
> into 64-bits so I assumed there was a reason I couldn't see.
> 
> >> I think SpeciallCallingConv ought to be eliminated and Mips16's calling
> convention be given it's own CallingConv def instead. If I remember rightly, I
> left it there because I didn't want to change Mips16 at the time.
> >>
> >> I'll comment on the OriginalArg* family below.
> >>
> >>> * HexagonCCState: adds a single extra field - NumNamedVarArgParams.
> >>> * PPCCCState: adds an OriginalArgWasPPCF128 bool vector. Arguably
> >>> reduces boilerplate vs MipsCCState by just having PPCISelLowering
> >>> call PPCCCState::PreAnalyzeCallOperands directly.
> >>> * SystemZCCState: has bool vectors ArgIsFixed and ArgIsShortVector,
> >>> works similarly to MipsCCState or PPCCCState.
> >>>
> >>> The above works, but it isn't directly usable in the current
> >>> GlobalISel implementation. Very sensibly, GISel tries to both reuse
> >>> existing calling convention implementations and to reduce duplicated
> code as much as possible.
> >>> To this end, CallLowering::handleAssignments will create a CCState
> >>> and use ValueHandler::assignArg to call a function of type CCAssignFn
> type.
> >>>
> >>> I see a couple of options:
> >>> 1) Creating a new virtual method in GISel CallLowering that creates
> >>> and initialises a CCState or custom subclass. Use that to support
> >>> target-specific CCStates.
> >>> 2) Try to remove the need for custom CCState altogether. In
> >>> <https://reviews.llvm.org/D38894>, Shiva Chen adds an OrigVT field
> >>> to ISD::ArgFlagsTy which seems much cleaner. It's not immediately
> >>> obvious to me if the in-tree users that currently track Type rather
> >>> than EVT could always make do with an EVT instead. [Input welcome!].
> >>
> >> Unfortunately, this solution wouldn't work for Mips since the 'Original*'
> vectors provide information from the frontend before the type was lowered
> to a VT. For example, OriginalArgWasF128[I] can be true when the VT before
> legalization was not f128. One of these cases is a structure containing a 128-
> bit float which the frontend lowers to i128. The other is an i128 containing a
> softfloat representation of a 128-bit float. If I remember correctly, the need
> for the latter is caused by a gcc bug that became a de-facto part of the ABI by
> going unnoticed for ~10 years until we tried to link clang-compiled and gcc-
> compiled objects together and found it didn't work.
> >
> > The question then is whether the backends that currently are happy
> > with EVT (SystemZ, PPC) might be just has happy with Type. Thanks for
> > pointing out that MipsCCState::originalTypeIsF128 is slightly more
> > involved than other similar functions (checking for f128 libcall
> > names).
> >
> > It seems Mips has the most fiddly CCState, so perhaps the key question
> > is: has anyone given any thought to supporting Mips in GlobalISel?
> 
> I'm trying to keep it in mind when we design things so we don't do anything
> that's too painful for Mips but I'm sure I'll miss things.

Getting the original Type information would be ideal for Mips. The Original*
vectors that deal with vector types were to deal with vector arguments/returns
on MIPS. The particular quirk there was that vector arguments always go
through the integer register set, even if that vector is a legal type. 

I haven't had the bandwidth to track development in that area of GlobalISel.

Thanks.
Simon

> 
> > Best,
> >
> > Alex