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

Tue Jun 9 22:01:45 PDT 2020

rjmccall added inline comments.

================
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 "
----------------
jtmott-intel wrote:
> rjmccall wrote:
> > erichkeane wrote:
> > > Mentioning target-specific support here seems incorrect. @rjmccall I cannot think of a better wording, can you?
> > "overflow builtins do not support _ExtInt operands of more than %0 bits on this target"?  I don't think it's unreasonable to mention the target-specificness of it.   Hard-coding the number 64 in the diagnostic seems excessively targeted, though.
> I discovered there's a good existing message I could use that already takes the bitwidth as a parameter. I decided not to add a new one. Thoughts/preferences? Here's how the message would come out.
> 
>     test.c:5:43: error: signed _ExtInt of bit sizes greater than 128 not supported
>         _Bool status = __builtin_mul_overflow(x, y, &result);
>                                               ^
>     1 error generated.
> 
It'd be much clearer to say something about the fact that it's just this builtin that's not supported.

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

https://reviews.llvm.org/D81420