[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