[PATCH] D81420: Fix size for _ExtInt types with builtins

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

So the idea of the SEMA check is right, but it ultimately isn't correct.  Integers up to 128 can be called, as long as you use compiler-rt (--rtlib=compiler-rt), see https://godbolt.org/z/JDXZG_.

However, 129 bits results in a crash in LLVM: https://godbolt.org/z/q432t_

When you change this, please add tests for 128/127 bit as well.  AND the new error message needs a SEMA test as well.



================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7939
   "to a non-const integer (%0 invalid)">;
+def err_overflow_builtin_extint_size : Error<
+  "_ExtInt argument larger than 64-bits to overflow builtin requires runtime "
----------------
Mentioning target-specific support here seems incorrect. @rjmccall I cannot think of a better wording, can you?


================
Comment at: clang/lib/Sema/SemaChecking.cpp:339
+                : Arg.get()->getType()->getAs<PointerType>()->getPointeeType();
+      if (Ty->isExtIntType() && S.getASTContext().getIntWidth(Ty) > 64) {
+        S.Diag(Arg.get()->getBeginLoc(), diag::err_overflow_builtin_extint_size)
----------------
I'll explain it below, but while I think this is the right IDEA, I think >64 is wrong.  It should be >128.

Second, this requires a SEMA test.


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

https://reviews.llvm.org/D81420