[PATCH] D108643: Introduce _BitInt, deprecate _ExtInt

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D108643#3057417 <https://reviews.llvm.org/D108643#3057417>, @rjmccall wrote:

> In D108643#3055244 <https://reviews.llvm.org/D108643#3055244>, @erichkeane wrote:
>
>>>> ! In D108643#3054443 <https://reviews.llvm.org/D108643#3054443>, @rjmccall wrote:
>>>>
>>>>> ! In D108643#3027473 <https://reviews.llvm.org/D108643#3027473>, @erichkeane wrote:
>>>>>
>>>>>> ! In D108643#3026550 <https://reviews.llvm.org/D108643#3026550>, @rjmccall wrote:
>>>>>
>>>>> I think it would be better to provide a generic ABI specification that is designed to "lower" `_BitInt` types into more basic concepts that ABIs typically already have rules for.  It would be appropriate to note at the top that this is only a generic specification and that the rules for any particular platform should be codified in a platform-specific ABI.  But we should make that easy so that platform owners who don't have strong opinions about the ABI can just say "as described in version 1.4 of the generic ABI at https://clang.llvm.org/..."
>>>>>
>>>>> Now, that is all independent of what we actually decide the ABI should be.  But I do think that if we're going to make this available on all targets, we should strongly consider doing a "legalizing" lowering in the frontend: at ABI-exposed points (call arguments/parameters and memory accesses), `_BitInt` types should be translated into types that a naive backend can be trusted to handle in a way that matches the documented ABI.  Individual targets can then opt in to using the natural lowerings if they promise to match the ABI, but that becomes a decision that they make to enable better optimization and code generation, not something that's necessary for correctness.
>>>>
>>>> Don't we already have that?  We work the same way a 'bool' does, that is, we zero/sign extend for parameters and return values?  The backends then reliably up-scale these to the power-of-two/multiple-of-pointer.
>>>
>>> There's two levels of a "legalizing" lowering.
>>>
>>> On the specification level, we'd be formally saying that the ABI matches the ABI of some other C construct.  For example, we might say that a `signed _BitInt(14)` argument is treated exactly as if the argument type was instead `signed short` (with whatever restriction we do or do not impose on the range).  This is very nice on this level because, first, it's a rule that works generically for all targets without needing to care about the details of how arguments are assigned to registers or stack slots or whatever; and second, it means you're never adding new cases to the ABI that e.g. libffi would need to care about; and third, you can reliably match the ABI with manually-lowered C code from a compiler that doesn't even support `_BitInt`s.
>>>
>>> On the implementation level, we'd have to actually emit IR which will turn into code which matches that ABI.  I think you are right that small integers already reliably work that way in LLVM, so that if you pass an `i11 sext` it'll get sign-extended to some width — what width exactly, I don't know, but probably the same width that `i16 sext` would be.  Larger integers, I'm not sure.  A legalizing lowering of big `_BitInt` argument would presumably say either that it's exactly the same as a struct containing all the chunks or that it's exactly the same as a series of arguments for each of the chunks.  Those are two very different rules!  Most ABIs won't "straddle" a single argument between registers and the stack, so e.g. if you'd need two registers and you only have one left then you exhaust the registers and go fully on the stack; but obviously you can get straddling if the lowering expands the sequence.  Similarly, some ABIs will go indirect if the struct gets big enough, but you won't get that if the lowering expands.  But on the other hand, if the rule is really that it's like a struct, well, some ABIs don't pass small structs in registers.  Anyway, my point is that what exactly LLVM targets do if you give them an `i117` might not match either of these possibilities reliably across targets.  In any case, since we currently have to count registers in the frontend for the register-passing ABIs, we'll need to actually know what the targets do.
>>
>> Hmm... now that I think I understand the concern, I'm not sure we're able to take on this level of commitment. The Clang ABI code is a bit of a mess in general, plus each target has its own code, right (In CodeGen/TargetInfo.cpp)?
>
> Ideally we would have a generic handling of it that most ABI code would simply trigger.
>
>> Additionally, that would require breaking the Microsoft ABI again, and I thought we'd discussed trying not to make changes until Microsoft defined that ABI, right?
>
> I didn't realize we were matching any other compilers at the moment.  Or is that not what you mean?

Sorry, no, not what I meant, but I see how it is misleading.  We currently have an ABI for _ExtInt (admittedly experimental as you said) on Windows that we will have to break at least 1 more time (when MS decides on the ABI).  I thought at one point in this discussion we'd decided to keep us from breaking OUR experimental MS ABI as much as possible, so we only broke it 1x.

> I think we need to consider the ABI for this experimental and unstable on all platforms.
>
>> I would like us to be able to find a way forward, and hope you can help make a suggestion on how to do so.  We'd love to get Clang to support this C23 feature at least on some targets as there are a good amount of users of this feature already as `_ExtInt`, and we would really want to get them to the standards version ASAP.  Do you have any suggestions on how we can move forward?
>
> I think the immediate way forward is that we can continue to document this as an experimental feature with an unstable ABI, just with a new name.  We do not need to tie these things together as long as we agree that the feature is experimental and the ABI might change.  Perhaps it should be enabled by an experimental flag until the ABI is official?

I don't know if we should continue to document this as experimental.  This is a language-standardized type that we'd ultimately like to be able to claim support for. I'm not sure on the progress of the ABI going 'official', but I think it has already been published, or nearly so?  We've been engaging with HJ, who owns the Itanium ABI.

>> PS: The current state of the things for the 4 platforms I deal with (and of course the most complicated ones) is:
>
> Ah, thank you for this.
>
>> |                                                 | <32 bit    | < 64 bit                     | < 128 bit                                                           | >128 bit          |
>> | Linux x86_64                                    | s/zext     | always pass as a coerced i64 | Pass as 2 i64s 'coerced' while in registers, then Ptr to iN (byval) | Ptr to iN (Byval) |
>> | Windows x86_64                                  | pass as iN | pass as iN                   | Ptr to iN                                                           | Ptr to iN         |
>> | Linux i386                                      | s/zext     | pass as iN                   | Ptr to iN                                                           | Ptr to iN         |
>> | Windows i386                                    | s/zext     | pass as iN                   | Ptr to iN                                                           | Ptr to iN         |
>> | + Your suggestion for ?all? as I understand it? | s/zext     | s/zext                       | Split into i64s?                                                    | Split into i64s?  |
>> |
>
> Lowering semantics would use the platform's normal ABI rules for the lowered type.  For example, if we lowered larger-than-fundamental `_BitInt`s to a struct containing the chunks, then `unsigned _BitInt(192)` would become `struct { uint32_t a, b, c, d, e, f; }` on 32-bit (which IIUC on both Linux and Windows x86 would be passed directly on the stack, not by reference) and `struct { uint64_t a, b, c; }` on 64-bit (which IIUC on both Linux and Windows  x86_64 would be passed by reference).  The target lowering code would recognize `_BitInt` types and ask the normal code to handle the lowered type instead.

I think I have a better understanding of this? I perhaps need to ruminate further.  I don't know if we'd be able to have a single place where we can do all of this, but it seems like this is a fairly non-trivial task.  Aaron and I will discuss it further.

> What you're describing above are completely custom target-specific rules, and in some cases they're novel rules that the ABIs don't normally use for other types.  The standard i386 CCs basically never pass arguments by reference unless they have a non-trivially-copyable C++ type — the only exception is over-aligned types on Windows.  Unless you don't really mean they're passed by pointer; if you're looking at IR, it can be misleading because of `byval`.

For the windows types, they are currently passed directly as iN*.  The only platform that did the 'byval' was the linux 64 bit ABI. This is perhaps novel, but it IS what we do currently for '_ExtInt'.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108643/new/

https://reviews.llvm.org/D108643