[PATCH] D73967: Implement _ExtInt as an extended int type specifier.

[Erich Keane via Phabricator via cfe-commits]

erichkeane marked an inline comment as done.
erichkeane added a comment.

In D73967#1861073 <https://reviews.llvm.org/D73967#1861073>, @rsmith wrote:

> Have you considered how this would interact with our other language extensions? Can we form a vector of `_ExtInt(N)`? A `_Complex _ExtInt(N)`? I expect for some of that to just fall out of the implementation, but we should have documentation and test coverage either way.
>
> I don't see any test coverage for `_Atomic(_ExtInt(N))`, which may not fall out from the other work because (if memory serves) atomic lowering doesn't go through the normal `ConvertTypeForMem` path and instead deals with padding itself.
>
> I would like to see your documentation cover the calling convention used when passing/returning these types by value.


We can do a std::vector or any of the standard containers with no problems.

Extended-vector types don't really make sense for non-powers-of-two (plus have some odd gotchas when it comes to vectors of i1 for example), so I've added a test that shows that this is rejected.

_Complex _ExtInt(N) is rejected when parsing _Complex.  It doesn't seem to make sense to me to support them, so I've added a test that shows it is invalid.  Do you disagree?

_Atomic seems to be broken (atomic memory access size mus be byte-sized), but I'll continue to work on it to update this patch further.

I've also asked a coworker to better describe the calling convention so that we can add that text to the language-extensions.

Thanks!



================
Comment at: clang/lib/AST/ItaniumMangle.cpp:3472
+  BW = T->getNumBits();
+  mangleIntegerLiteral(getASTContext().UnsignedIntTy, BW);
+}
----------------
rsmith wrote:
> This is not a valid vendor-extension mangling. There are two choices here, per the current scheme:
> 
> 1) mangle as a type, using (lowercase) `u` followed by a source-name, such as `u9_ExtInt17` / `u10_UExtInt17`
> 2) mangle as a type qualifier, using (capital) `U` followed by a source-name and optional template-args, such as `U7_ExtIntILi17EEi` / `U7_ExtIntILi17EEj`
> 
> Neither of these gives a particularly nice demangling. If WG14 seems likely to accept the proposal, you should register a proper mangling as part of the Itanium ABI.
Well shucks, I was so close :)  Looks like I'm just missing the last E/i from the second one.  

The second seems consistently changable with the one below (for a dependent version), so I think I'm stuck with that.  It DOES show up a little weird unfortunately, but at least now it demangles.

I'll ask about an official mangling mechanism for ItaniumABI.


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

https://reviews.llvm.org/D73967