[PATCH] D108643: Introduce _BitInt, deprecate _ExtInt

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

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.



================
Comment at: clang/docs/ReleaseNotes.rst:131
+  ``_BitInt(N)`` is supported as an extension in older C modes and in all C++
+  modes.
+
----------------
Should we document that this is still considered an experimental feature, in the sense of not yet officially having a stable ABI?


================
Comment at: clang/docs/ReleaseNotes.rst:166
+  previously used the ``_ExtInt`` types will now be mangled to instead use the
+  ``_BitInt(N)`` spelling in both the Microsoft and Itanium ABIs.
+
----------------
Probably more accurate to now say something like:

```
The ``_ExtInt(N)`` extension has been standardized in C2X as ``_BitInt(N)``.  The mangling of this type in C++ has accordingly changed: under the Microsoft ABI it is now mangled using the ``_BitInt`` spelling, and under the Itanium ABI it is now mangled using a dedicated production.
```


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

https://reviews.llvm.org/D108643