Options for custom CCState, CCAssignFn, and GlobalISel

This question came about through reviewing work from Leslie Zhai on GlobalISel
support for RISC-V, which also motivated me to revisit code which I've always
felt was a bit clunky.

Calling convention lowering in LLVM is typically handled by functions
conforming to the CCAssignFn typedef:

    typedef bool CCAssignFn(unsigned ValNo, MVT ValVT,
                            MVT LocVT, CCValAssign::LocInfo LocInfo,
                            ISD::ArgFlagsTy ArgFlags, CCState &State);

Notably, these functions are called after type legalisation so an argument/ret
has been split to legal value types. In some cases you want more information
than is available through this function interface, which leads to a number of
backends creating their own CCState subclass:

* MipsCCState: adds bool vectors OriginalArgWasF128, OriginalArgWasFloat,
OriginalArgWasFloatVector, OriginalRetWasFloatVector, CallOperandIsFixed. Also
a SpeciallCallingConv field. Provides its own implementation of
AnalyzeFormalArguments etc that fill these vectors.
* 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!].
  * Do any out-of-tree backends have custom calling convention code that
requires more information than original arg type and whether the argument is
fixed or not? In the RISC-V calling convention implementation I'd be happiest
if the calling convention function had access to SubtargetInfo and the
DataLayout, but can probably do without.

Does anyone have views on this, or insight into planned future developments of
calling convention handling in GlobalISel?

Thanks,

Alex

I haven't dug into the GlobalISel calling convention code much but I can comment on the MipsCCState.

This question came about through reviewing work from Leslie Zhai on GlobalISel
support for RISC-V, which also motivated me to revisit code which I've always
felt was a bit clunky.

Calling convention lowering in LLVM is typically handled by functions
conforming to the CCAssignFn typedef:

   typedef bool CCAssignFn(unsigned ValNo, MVT ValVT,
                           MVT LocVT, CCValAssign::LocInfo LocInfo,
                           ISD::ArgFlagsTy ArgFlags, CCState &State);

Notably, these functions are called after type legalisation so an argument/ret
has been split to legal value types. In some cases you want more information
than is available through this function interface, which leads to a number of
backends creating their own CCState subclass:

* 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.

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.

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?

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?

Best,

Alex

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.

Hi Alex,

Thanks for your leading!

I refactory GlobalISel CallLowering handleAssignments https://reviews.llvm.org/D41774

So it is able to override `handleAssignments` in your <Target>CallLowering to initial your <Target>CCState, and it also need to override `assignArg` in your <Target>CallLowering, cast from `CCState` to custom <Target>CCState, then do some target-specific treatment.

Please give me some suggestion, thanks for your teaching!

From: daniel_l_sanders@apple.com [mailto:daniel_l_sanders@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

>
>> 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

Hi Alex,

Please review my patch D41700 again, thanks a lot!

Thanks for LLVM developers' code view, I can rebase my patch based on D40805 .

1. Consider about `IsFixed` and `OrigTy`, but assert as firstly:

assert(IsFixed && "IsFixed support not yet implemented");
assert(OrigTy && "OrigTy support not yet implemented");

The implementation of `RISCVCCState` will be in the next patch, please give me some suggestion about custom `RISCVCCState` inherited from `CCState`, I think, at least, it needs to cover `IsFixed` and `OrigTy`.

2. Remove parameter `DL` for `CC_RISCV`, because it is able to use:

State.getMachineFunction().getDataLayout();

through parameter `State`.

Hi LLVM developers,

Don't be quiet :slight_smile: we need your suggestions for supporting custom CCState, CCAssignFn in D41700.

And also RegisterBank in D41653. because it needs to consider about how to support variable-sized register classes concept implemented in D24631.

And I think you might have same question when porting to GlobalISel for your Targets, so please give us some directions, thanks a lot!

As a starting point. I've put a patch up for comment which adds
IsFixed to ISD::ArgFlagsTy and ensures it is set correctly, as well as
updating backends to use it <https://reviews.llvm.org/D42374>. This
patch allows HexagonCCState to be removed completely. As noted in the
patch description, further improvements could be made such as modify
AArch64 calling conventions to make use of CCIfFixed and so calling
AArch64ISelLowering::CCAssignFnForCall once per function rather than
once per arg.

Comments and review very welcome.

Best,

Alex

Hi LLVM developers,

I am not available from February 11th to February 25th due to Chinese Spring Festival and my sincere thanks goto:

Anna Zaks:

 She lead me to the LLVM family and reviewed my patch for clang analyzer MallocChecker carefully and patiently\. Дуже дякую

Artem Dergachev:

 He reviewed my patches for clang analyzer some Checkers carefully\. Большое спасибо

Dylan McKay:

 He reviewed my patches for AVR Target\. Thanks a lot\!

Rui Ueyama:

 He taught me how to migrate AVR for LLD\. どうもありがとう

Duncan Sands:

 He lead me to the GCC family via migrate DragonEgg to GCC 8\.x and LLVM 7\.x\. Merci beaucoup

Alex Bradbury:

 He is my mentor, he is leading me to contribute to RISCV Target\. Thanks a lot\!

Heiher:

 He reviewed my backport patches for GCC 5\.5, but we are also tracking the issue for gcc\-6/7 branches and gcc\-8 trunk\. 非常感谢!龙芯战士们临近春节还奋战在Mozilla、Linux Kernel、GCC、Binutils,但依然抽出休息时间审核我的工作,非常感激!