FreddyYe marked an inline comment as done.
FreddyYe added a comment.
Addressed. THX review!
================
Comment at: clang/test/Sema/builtins-overflow.c:45
+ unsigned _ExtInt(128) result;
+ _Bool status = __builtin_mul_overflow(x, y, &result); // expected-error
{{__builtin_mul_overflow does not support special combination operands (signed,
signed, unsigned*) of more than 64 bits}}
+ }
----------------
aaron.ballman wrote:
> Yeah, this diagnostic really doesn't tell me what's going wrong with the code
> or how to fix it. Do we basically want to prevent using larger-than-64-bit
> argument types with mixed signs? Or are there other problematic circumstances?
Yes, let me try to refine. I can explain more what happened to such input
combination.
According to gcc's the definition on this builtin:
https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html
'These built-in functions promote the first two operands into infinite
precision signed type and perform multiply on those promoted operands. The
result is then cast to the type the third pointer argument points to and stored
there.'
Since signing integer has a smaller range than unsigned integer. And now the
API in compiler-rt (`__muloti4`) to checking 128 integer's multiplying is
implemented in signed version. So the overflow max absolute value it can check
is 2^127. When the result input is larger equal than 128 bits, `__muloti4` has
no usage. We should prevent this situation for now. Or the backend will crush
as the example shows.
I found the input operand doesn't need both of them larger than 64 bits, but
just the sum of their larger 128. I'll refine in my patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107420/new/
https://reviews.llvm.org/D107420
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
