From 3584f845fb4db93578229aa5f6ece2b3287a14f0 Mon Sep 17 00:00:00 2001 From: Mihai Maruseac Date: Fri, 13 Nov 2020 09:38:59 -0800 Subject: [PATCH] Make `MultiplyWithoutOverflow` be a total function. That is, remove the `CHECK`-fails. Now the function will return a negative result if either of the arguments is negative (new) or if the multiplication overflows (old). This doesn't change behavior, as all tests are still passing. The function is now slightly faster. PiperOrigin-RevId: 342279369 Change-Id: Ib6d4cdb038910d8714d09224b2c1bb2b2ab6ee16 --- tensorflow/core/util/overflow.h | 13 +++++++------ tensorflow/core/util/overflow_test.cc | 6 +++--- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/tensorflow/core/util/overflow.h b/tensorflow/core/util/overflow.h index 04be68a111e..4dbb829564e 100644 --- a/tensorflow/core/util/overflow.h +++ b/tensorflow/core/util/overflow.h @@ -23,7 +23,12 @@ limitations under the License. namespace tensorflow { // Multiply two nonnegative int64's, returning negative for overflow +// If any of the arguments is negative, return negative too. inline int64 MultiplyWithoutOverflow(const int64 x, const int64 y) { + if (TF_PREDICT_FALSE(x < 0)) return -1; + if (TF_PREDICT_FALSE(y < 0)) return -1; + if (TF_PREDICT_FALSE(x == 0)) return 0; + // Multiply in uint64 rather than int64 since signed overflow is undefined. // Negative values will wrap around to large unsigned values in the casts // (see section 4.7 [conv.integral] of the C++14 standard). @@ -33,15 +38,11 @@ inline int64 MultiplyWithoutOverflow(const int64 x, const int64 y) { // Check if we overflow uint64, using a cheap check if both inputs are small if (TF_PREDICT_FALSE((ux | uy) >> 32 != 0)) { - // Ensure nonnegativity. Note that negative numbers will appear "large" - // to the unsigned comparisons above. - CHECK(x >= 0 && y >= 0); - // Otherwise, detect overflow using a division - if (ux != 0 && uxy / ux != uy) return -1; + if (uxy / ux != uy) return -1; } - // Cast back to signed. Any negative value will signal an error. + // Cast back to signed. A negative value will signal an error. return static_cast(uxy); } diff --git a/tensorflow/core/util/overflow_test.cc b/tensorflow/core/util/overflow_test.cc index 0f9b3571611..22510166e13 100644 --- a/tensorflow/core/util/overflow_test.cc +++ b/tensorflow/core/util/overflow_test.cc @@ -75,9 +75,9 @@ TEST(OverflowTest, Nonnegative) { TEST(OverflowTest, Negative) { const int64 negatives[] = {-1, std::numeric_limits::min()}; for (const int64 n : negatives) { - EXPECT_DEATH(MultiplyWithoutOverflow(n, 0), "") << n; - EXPECT_DEATH(MultiplyWithoutOverflow(0, n), "") << n; - EXPECT_DEATH(MultiplyWithoutOverflow(n, n), "") << n; + EXPECT_LT(MultiplyWithoutOverflow(n, 0), 0) << n; + EXPECT_LT(MultiplyWithoutOverflow(0, n), 0) << n; + EXPECT_LT(MultiplyWithoutOverflow(n, n), 0) << n; } }