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

Richard Smith - zygoloid via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 5 18:30:08 PST 2020


rsmith added a comment.

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.



================
Comment at: clang/lib/AST/ItaniumMangle.cpp:3472
+  BW = T->getNumBits();
+  mangleIntegerLiteral(getASTContext().UnsignedIntTy, BW);
+}
----------------
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.


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

https://reviews.llvm.org/D73967





More information about the cfe-commits mailing list