[PATCH] D108643: Introduce _BitInt, deprecate _ExtInt

Thu Sep 9 09:10:05 PDT 2021

aaron.ballman added a reviewer: hjl.tools.
aaron.ballman added a subscriber: hjl.tools.
aaron.ballman added a comment.

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

> In D108643#2964740 <https://reviews.llvm.org/D108643#2964740>, @aaron.ballman wrote:
>
>> In D108643#2964190 <https://reviews.llvm.org/D108643#2964190>, @rjmccall wrote:
>>
>>> I agree with James; I know you've reached out to the Itanium ABI group about mangling, but ABI involvement needs to mean also reaching out and getting psABI agreement about representation.  I would suggest proposing a generic ABI, getting consensus on that generic ABI with other implementors, and then running that generic ABI past as many platform ABI groups as you can get in touch with.
>>
>> I've already reached out to the psABI maintainers and have started those discussions before ever considering the mangling questions: https://gitlab.com/x86-psABIs/x86-64-ABI/-/commit/21622785649f2cbacb596d135a417171f52ad539
>
> Okay.  To be clear, that's not "the psABI maintainers", that's specifically the x86_64 psABI group.

Heh, you can see how out of my depths I am on the ABI topic. :-D

> Is your expectation that other psABI groups would more or less copy the wording from the x86_64 psABI, potentially replacing 64 with 32 as appropriate, or would you want to host a generic ABI document somewhere else?  If the latter, I think the Clang repository wouldn't be an unreasonable place to do that; we do something similar with blocks.

I was expecting the ABI groups to determine what's best for their respective ABIs; I imagined the x86-64 ABI was sufficient as a template, but I wasn't certain if that's appropriate. We could host a generic ABI document similar to what we do for blocks if that's what people felt was a good approach.

> I would be happy to put you in touch with some other psABI groups when you're ready to talk to them.

Thanks! I'm kind of hoping to avoid being responsible for driving the ABI process with all the different ABI groups because I have no idea how much effort I would be signing myself up for. TBH, I was hoping a "this is a newly standardized type that needs ABI considerations" note would be sufficient because I don't have an opinion on what's best for any given ABI.

>> Note that there are some changes expected to that wording (the "bit-aligned" stuff should say "byte-aligned" and we're missing information about bit-fields), so if there are additional changes we'd like to see made, now's a good time to make them.
>
> The choice that high bits are unspecified rather than extended is an interesting one.  Can you speak to that?  That's good for +, -, *, &, |, ^, <<, and narrowing conversions, but bad for ==, <, /, >>, and widening conversions.

I've added @hjl.tools to the review for his opinions, as he was the primary driver for the x64 ABI proposal. HJ, can you help me out here?

> One advantage of having a generic ABI is that you can give a rationale in the document, which would be out of scope for a psABI.

Ah, good point.

>>> You can't have bit-fields of `_BitInt` type, can you?  If people want that, or want a truly packed `_BitInt` where `sizeof(_BitInt(56)) == 7`, it's going to add a lot of complexity.
>>
>> You can have bit-fields of _BitInt type (and we even tell you when you do dumb things with it https://godbolt.org/z/sTKKdb1o1), but I'm not seeing where the complexity comes in from that
>
> What is your expectation of how a `_BitInt` bit-field interacts with adjacent bit-fields?

It's implementation-defined because the type isn't `_Bool`, `signed int`, or `unsigned int`, but the current behavior is that it combines at the type boundary rather than packing.

> Does `unsigned a: 1; _BitInt(8) b: 7;` only use 8 bits?

  sizeof(struct S {
      unsigned a: 1;
      _BitInt(8) b: 7;
    });

is 4 bytes (on x64).

> Does `unsigned a: 1; _BitInt(95) b: 95;` only use 96?

  sizeof(struct S {
      unsigned a: 1;
      _BitInt(95) b: 95;
    });

is 16 bytes (on x64).

> Does the latter change based on whether this is a 32-bit or 64-bit target?

It does; then it's 12 bytes.

> Does it change if the type is `_BitInt(128)` instead?

128-bit is: 24 bytes (x64) and 20 bytes (x86)
127-bit is: 16 bytes on both x64 and x86

> What is the "storage unit" of a `_BitInt` for the purposes of ABIs that are sensitive to such things?

My understanding is that it's the `_BitInt` itself.

> Also, C requires the width of a bit-field to be no larger than the width of the underlying type, but C++ allows "wide" bit-fields, and psABIs are inconsistent about giving rules for their layout.  Does this match that where supported, or is a different set of rules?

It should be the same set of rules. At least: https://godbolt.org/z/1n9qn38b7

(I should note, I've been describing the behavior Clang currently has using `_ExtInt` and testing on godbolt. If we want different answers than what I've given, additional work is required. @erichkeane may have more information on why the current behavior is the way it is.)

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

https://reviews.llvm.org/D108643