On Fri, 2 May 2025 17:03:30 GMT, fabioromano1 <d...@openjdk.org> wrote:
>> I don't think there's a quick, _precise_ pre-check that would ensure that >> the loop can just use simple, unchecked `*` multiplications. >> >> Consider `unsignedPowExact(3L, 40)`, which does not overflow, versus >> `unsignedPowExact(3L, 41)`, which does. >> How would you pre-check these two cases using integer arithmetic? >> >> IMO, you still need checked multiplications in the loop. >> >> (Besides, the product in your checks can overflow, so you would have to add >> a guard.) > >> I don't think there's a quick, _precise_ pre-check that would ensure that >> the loop can just use simple, unchecked `*` multiplications. >> >> Consider `unsignedPowExact(3L, 40)`, which does not overflow, versus >> `unsignedPowExact(3L, 41)`, which does. How would you pre-check these two >> cases using integer arithmetic? >> >> IMO, you still need checked multiplications in the loop. >> >> (Besides, the product in your checks can overflow, so you would have to add >> a guard.) > > @rgiulietti > > 1. `BigInteger.bitLengthForLong(x) * n <= Long.SIZE` is a sufficient > condition to ensure no overflow; > > 2. `(BigInteger.bitLengthForLong(x) - 1L) * n + 1L > Long.SIZE` is a > sufficient condition to ensure the overflow. > > > Thus, there remain only the cases when ` Long.SIZE < > BigInteger.bitLengthForLong(x) * n && (BigInteger.bitLengthForLong(x) - 1L) * > n + 1L <= Long.SIZE`, in this cases checked multiplications in the loop are > needed. > > Moreover, `BigInteger.bitLengthForLong(x) - 1L` is a `long`, so the product > does not overflow, so the condition at point 1 never overflows if it is > evaluated only if the condition at point 2 is false. @fabioromano1 Well, there are two checks. In one the product can overflow, you'd need to convert one of the operands to `long`. Anyway, since the pre-checks are not precise, that would lead to an implementation with a loop with checked, and another one with unchecked multiplications. I don't think this buys you anything. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25003#discussion_r2071920476
