jsoniter-scala
jsoniter-scala copied to clipboard
INT128 jsoniter_scala_multiply_high() may have problem when one argument is negative.
Recent changes to jsoniter-scala-core/native/src/main/resources/scala-native/multiply_high.c
have advanced the discussion.
I do have a concern about one section of code. Please forgive me if I have missed something, either obvious or subtle.
I have not had time to implement a test program, but it seems to me that the INT128
jsoniter_scala_multiply_high() code may have a problem/defect/bug when either
(but probably not both) arguments are negative.
Later, Later: Removed wrong comments about discarded upper bits, those are preserved. Wake up, Lee.
#elif defined(__SIZEOF_INT128__)
long long jsoniter_scala_multiply_high(long long x, long long y) {
return x * (unsigned __int128) y >> 64;
// Careful with undefined behavior of >>. Probably need it #ifdef compiler being known to use arithmitic shift (GCC or Clang (or Apple?))
}
unsigned long long jsoniter_scala_unsigned_multiply_high(unsigned long long x, unsigned long long y) {
return x * (unsigned __int128) y >> 64;
}
I suspect the code in the #else Hacker's delight branch might have similar issues, but
have not had time to study it or do a executable design study
long long t = x1 * y0 + ((unsigned long long) (x0 * y0) >> 32);
// I think for most contemporary compilers a right shift of an unsigned will fill vacated bits with 0.
// I doubt that is defined behavior.
// I am pretty sure that right shifting of a _signed_ value is Undefined Behavior, even in C 23.
Thank you for your time considering this.