From 90e89339a9bf04fb304129a01ca50f25fdde441d Mon Sep 17 00:00:00 2001 From: Thibaut Goetghebuer-Planchon <thibaut.goetghebuer-planchon@arm.com> Date: Fri, 7 Aug 2020 16:56:07 +0100 Subject: [PATCH 1/7] Fix potential overflow in 64-bit MultiplyByQuantizedMultiplier function --- tensorflow/lite/kernels/internal/common.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tensorflow/lite/kernels/internal/common.h b/tensorflow/lite/kernels/internal/common.h index 66a2d977f39..c4a620336d3 100644 --- a/tensorflow/lite/kernels/internal/common.h +++ b/tensorflow/lite/kernels/internal/common.h @@ -179,7 +179,9 @@ inline int32_t MultiplyByQuantizedMultiplier(int64_t x, assert(quantized_multiplier >= 0); assert(shift >= -31 && shift < 8); - int32_t reduced_multiplier = (quantized_multiplier + (1 << 15)) >> 16; + int32_t reduced_multiplier = (quantized_multiplier < 0x7FFF0000) + ? ((quantized_multiplier + (1 << 15)) >> 16) + : 0x7FFF; int total_shift = 15 - shift; x = (x * (int64_t)reduced_multiplier) + ((int64_t)1 << (total_shift - 1)); int32_t result = x >> total_shift; From 36e7533d3c67dcfd7b23a6a19251676871a37bd7 Mon Sep 17 00:00:00 2001 From: Thibaut Goetghebuer-Planchon <thibaut.goetghebuer-planchon@arm.com> Date: Wed, 4 Nov 2020 11:05:19 +0000 Subject: [PATCH 2/7] Add tests for MultiplyByQuantizedMultiplier --- tensorflow/lite/kernels/internal/BUILD | 4 ++ tensorflow/lite/kernels/internal/common.h | 2 + .../internal/quantization_util_test.cc | 66 +++++++++++++++++++ 3 files changed, 72 insertions(+) diff --git a/tensorflow/lite/kernels/internal/BUILD b/tensorflow/lite/kernels/internal/BUILD index ad11c06eb37..d1cddfa59ff 100644 --- a/tensorflow/lite/kernels/internal/BUILD +++ b/tensorflow/lite/kernels/internal/BUILD @@ -380,6 +380,10 @@ cc_library( srcs = ["quantization_util.cc"], hdrs = ["quantization_util.h"], copts = tflite_copts() + micro_copts(), + linkopts = select({ + "//tensorflow:windows": [], + "//conditions:default": ["-lm"], + }), deps = [ ":compatibility", ":cppmath", diff --git a/tensorflow/lite/kernels/internal/common.h b/tensorflow/lite/kernels/internal/common.h index c4a620336d3..5c5d675928b 100644 --- a/tensorflow/lite/kernels/internal/common.h +++ b/tensorflow/lite/kernels/internal/common.h @@ -178,6 +178,8 @@ inline int32_t MultiplyByQuantizedMultiplier(int64_t x, // - input x is in the range -(1<<47) <= x < (1<<47) assert(quantized_multiplier >= 0); assert(shift >= -31 && shift < 8); + assert(x >= -(static_cast<int64_t>(1) << 47) && + x < (static_cast<int64_t>(1) << 47) - 1); int32_t reduced_multiplier = (quantized_multiplier < 0x7FFF0000) ? ((quantized_multiplier + (1 << 15)) >> 16) diff --git a/tensorflow/lite/kernels/internal/quantization_util_test.cc b/tensorflow/lite/kernels/internal/quantization_util_test.cc index 053b3116a15..f8de1c345d0 100644 --- a/tensorflow/lite/kernels/internal/quantization_util_test.cc +++ b/tensorflow/lite/kernels/internal/quantization_util_test.cc @@ -422,6 +422,72 @@ TEST(QuantizationUtilTest, GetInvSqrtQuantizedMultiplierExp) { EXPECT_THAT(inv_sqrt(kInt32Max), Pair(189812531, 12)); } +TEST(QuantizationUtilTest, MultiplyByQuantizedMultiplierInt32) { + auto quant_and_multiply = [](int32 x, double multiplier) { + int32_t quantized_multiplier; + int shift; + QuantizeMultiplier(multiplier, &quantized_multiplier, &shift); + return MultiplyByQuantizedMultiplier(x, quantized_multiplier, shift); + }; + + EXPECT_EQ(quant_and_multiply(0, 0.1), 0); + EXPECT_EQ(quant_and_multiply(0.1, 0), 0); + EXPECT_EQ(quant_and_multiply(10000, 0.00097656), 10); + EXPECT_EQ(quant_and_multiply(10000, -0.00097656), -10); + EXPECT_EQ(quant_and_multiply(-10000, 0.00097656), -10); + EXPECT_EQ(quant_and_multiply(-10000, -0.00097656), 10); + EXPECT_EQ(quant_and_multiply(std::numeric_limits<int32_t>::min(), 0.00001), + -21475); + EXPECT_EQ(quant_and_multiply(std::numeric_limits<int32_t>::min(), -0.00001), + 21475); + EXPECT_EQ(quant_and_multiply(std::numeric_limits<int32_t>::max(), 0.00001), + 21475); + EXPECT_EQ(quant_and_multiply(std::numeric_limits<int32_t>::max(), -0.00001), + -21475); + + // Test with maximum possible x and quantized_multiplier + const int32_t x = std::numeric_limits<int32_t>::max(); + const int32_t quantized_multiplier = std::numeric_limits<int32_t>::max(); + const int32_t expected = + TfLiteRound(static_cast<int64_t>(x) * quantized_multiplier / + static_cast<double>(1ll << 31 - (-3))); + EXPECT_EQ(MultiplyByQuantizedMultiplier(x, quantized_multiplier, -3), + expected); + EXPECT_EQ(MultiplyByQuantizedMultiplier(-x, quantized_multiplier, -3), + -expected); +} + +TEST(QuantizationUtilTest, MultiplyByQuantizedMultiplierInt64) { + auto quant_and_multiply = [](int64 x, double multiplier) { + int32_t quantized_multiplier; + int shift; + QuantizeMultiplier(multiplier, &quantized_multiplier, &shift); + return MultiplyByQuantizedMultiplier(x, quantized_multiplier, shift); + }; + + EXPECT_EQ(quant_and_multiply(0, 0.1), 0); + EXPECT_EQ(quant_and_multiply(0.1, 0), 0); + EXPECT_EQ(quant_and_multiply(10000, 0.00097656), 10); + EXPECT_EQ(quant_and_multiply(10000, -0.00097656), -10); + EXPECT_EQ(quant_and_multiply(-10000, 0.00097656), -10); + EXPECT_EQ(quant_and_multiply(-10000, -0.00097656), 10); + EXPECT_EQ(quant_and_multiply(-(1ll << 47), 0.00001), -1407385600); + EXPECT_EQ(quant_and_multiply(-(1ll << 47), -0.00001), 1407385600); + EXPECT_EQ(quant_and_multiply((1ll << 47) - 1, 0.00001), 1407385600); + EXPECT_EQ(quant_and_multiply((1ll << 47) - 1, -0.00001), -1407385600); + + // Test with maximum possible x and quantized_multiplier + const int64_t x = (1ll << 47) - 1; + const int32_t quantized_multiplier = std::numeric_limits<int32_t>::max(); + // Expected is around 'x * quantized_multiplier / 2**(31 - (-31))' ~= 65536 + // As there is some rounding error, expected is a bit smaller. + const int32_t expected = 65534; + EXPECT_EQ(MultiplyByQuantizedMultiplier(x, quantized_multiplier, -31), + expected); + EXPECT_EQ(MultiplyByQuantizedMultiplier(-x, quantized_multiplier, -31), + -expected); +} + TEST(QuantizationUtilTest, PreprocessSoftmaxScaling) { auto quantize = [](double beta, double scale, int integer_bits) { int32_t q; From 291d17a7f5474a743ef2e5b12167f85e12f58220 Mon Sep 17 00:00:00 2001 From: Thibaut Goetghebuer-Planchon <thibaut.goetghebuer-planchon@arm.com> Date: Tue, 17 Nov 2020 21:39:32 +0000 Subject: [PATCH 3/7] Use intXX_t instead of intxx --- tensorflow/lite/kernels/internal/quantization_util_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tensorflow/lite/kernels/internal/quantization_util_test.cc b/tensorflow/lite/kernels/internal/quantization_util_test.cc index f8de1c345d0..65a65fb8ada 100644 --- a/tensorflow/lite/kernels/internal/quantization_util_test.cc +++ b/tensorflow/lite/kernels/internal/quantization_util_test.cc @@ -423,7 +423,7 @@ TEST(QuantizationUtilTest, GetInvSqrtQuantizedMultiplierExp) { } TEST(QuantizationUtilTest, MultiplyByQuantizedMultiplierInt32) { - auto quant_and_multiply = [](int32 x, double multiplier) { + auto quant_and_multiply = [](int32_t x, double multiplier) { int32_t quantized_multiplier; int shift; QuantizeMultiplier(multiplier, &quantized_multiplier, &shift); @@ -458,7 +458,7 @@ TEST(QuantizationUtilTest, MultiplyByQuantizedMultiplierInt32) { } TEST(QuantizationUtilTest, MultiplyByQuantizedMultiplierInt64) { - auto quant_and_multiply = [](int64 x, double multiplier) { + auto quant_and_multiply = [](int64_t x, double multiplier) { int32_t quantized_multiplier; int shift; QuantizeMultiplier(multiplier, &quantized_multiplier, &shift); From 0cead482e9973828effe32206a9a749c226e039c Mon Sep 17 00:00:00 2001 From: Thibaut Goetghebuer-Planchon <thibaut.goetghebuer-planchon@arm.com> Date: Thu, 19 Nov 2020 19:40:03 +0000 Subject: [PATCH 4/7] Fix compilation error in MultiplyByQuantizedMultiplier test --- tensorflow/lite/kernels/internal/quantization_util_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tensorflow/lite/kernels/internal/quantization_util_test.cc b/tensorflow/lite/kernels/internal/quantization_util_test.cc index 65a65fb8ada..8988056ae04 100644 --- a/tensorflow/lite/kernels/internal/quantization_util_test.cc +++ b/tensorflow/lite/kernels/internal/quantization_util_test.cc @@ -431,7 +431,7 @@ TEST(QuantizationUtilTest, MultiplyByQuantizedMultiplierInt32) { }; EXPECT_EQ(quant_and_multiply(0, 0.1), 0); - EXPECT_EQ(quant_and_multiply(0.1, 0), 0); + EXPECT_EQ(quant_and_multiply(1, 0), 0); EXPECT_EQ(quant_and_multiply(10000, 0.00097656), 10); EXPECT_EQ(quant_and_multiply(10000, -0.00097656), -10); EXPECT_EQ(quant_and_multiply(-10000, 0.00097656), -10); @@ -466,7 +466,7 @@ TEST(QuantizationUtilTest, MultiplyByQuantizedMultiplierInt64) { }; EXPECT_EQ(quant_and_multiply(0, 0.1), 0); - EXPECT_EQ(quant_and_multiply(0.1, 0), 0); + EXPECT_EQ(quant_and_multiply(1, 0), 0); EXPECT_EQ(quant_and_multiply(10000, 0.00097656), 10); EXPECT_EQ(quant_and_multiply(10000, -0.00097656), -10); EXPECT_EQ(quant_and_multiply(-10000, 0.00097656), -10); From 100c797f8125c92ef19f7ec65ebc2e9c60c2e616 Mon Sep 17 00:00:00 2001 From: Thibaut Goetghebuer-Planchon <thibaut.goetghebuer-planchon@arm.com> Date: Wed, 9 Dec 2020 15:15:02 +0000 Subject: [PATCH 5/7] Fix operator precedence warning in MultiplyByQuantizedMultiplier tests --- .../kernels/internal/quantization_util_test.cc | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/tensorflow/lite/kernels/internal/quantization_util_test.cc b/tensorflow/lite/kernels/internal/quantization_util_test.cc index 8988056ae04..c00593de754 100644 --- a/tensorflow/lite/kernels/internal/quantization_util_test.cc +++ b/tensorflow/lite/kernels/internal/quantization_util_test.cc @@ -448,12 +448,13 @@ TEST(QuantizationUtilTest, MultiplyByQuantizedMultiplierInt32) { // Test with maximum possible x and quantized_multiplier const int32_t x = std::numeric_limits<int32_t>::max(); const int32_t quantized_multiplier = std::numeric_limits<int32_t>::max(); - const int32_t expected = + const int shift = -3; + const int32_t expected = static_cast<int32_t>( TfLiteRound(static_cast<int64_t>(x) * quantized_multiplier / - static_cast<double>(1ll << 31 - (-3))); - EXPECT_EQ(MultiplyByQuantizedMultiplier(x, quantized_multiplier, -3), + static_cast<double>(1ll << (31 - shift)))); + EXPECT_EQ(MultiplyByQuantizedMultiplier(x, quantized_multiplier, shift), expected); - EXPECT_EQ(MultiplyByQuantizedMultiplier(-x, quantized_multiplier, -3), + EXPECT_EQ(MultiplyByQuantizedMultiplier(-x, quantized_multiplier, shift), -expected); } @@ -479,12 +480,13 @@ TEST(QuantizationUtilTest, MultiplyByQuantizedMultiplierInt64) { // Test with maximum possible x and quantized_multiplier const int64_t x = (1ll << 47) - 1; const int32_t quantized_multiplier = std::numeric_limits<int32_t>::max(); - // Expected is around 'x * quantized_multiplier / 2**(31 - (-31))' ~= 65536 + const int shift = -31; + // Expected is around 'x * quantized_multiplier / 2**(31 - shift)' ~= 65536 // As there is some rounding error, expected is a bit smaller. const int32_t expected = 65534; - EXPECT_EQ(MultiplyByQuantizedMultiplier(x, quantized_multiplier, -31), + EXPECT_EQ(MultiplyByQuantizedMultiplier(x, quantized_multiplier, shift), expected); - EXPECT_EQ(MultiplyByQuantizedMultiplier(-x, quantized_multiplier, -31), + EXPECT_EQ(MultiplyByQuantizedMultiplier(-x, quantized_multiplier, shift), -expected); } From a66026f713be5f0b737bd6766fe1db6b7ad58156 Mon Sep 17 00:00:00 2001 From: Thibaut Goetghebuer-Planchon <thibaut.goetghebuer-planchon@arm.com> Date: Thu, 10 Dec 2020 11:02:18 +0000 Subject: [PATCH 6/7] Fix assert in 64-bit MultiplyByQuantizedMultiplier --- tensorflow/lite/kernels/internal/common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tensorflow/lite/kernels/internal/common.h b/tensorflow/lite/kernels/internal/common.h index 9b010603e58..db9cb0ce078 100644 --- a/tensorflow/lite/kernels/internal/common.h +++ b/tensorflow/lite/kernels/internal/common.h @@ -179,7 +179,7 @@ inline int32_t MultiplyByQuantizedMultiplier(int64_t x, assert(quantized_multiplier >= 0); assert(shift >= -31 && shift < 8); assert(x >= -(static_cast<int64_t>(1) << 47) && - x < (static_cast<int64_t>(1) << 47) - 1); + x < (static_cast<int64_t>(1) << 47)); int32_t reduced_multiplier = (quantized_multiplier < 0x7FFF0000) ? ((quantized_multiplier + (1 << 15)) >> 16) From a8b15d38e47396418c4e15fd4630a68a9762072b Mon Sep 17 00:00:00 2001 From: Thibaut Goetghebuer-Planchon <thibaut.goetghebuer-planchon@arm.com> Date: Thu, 10 Dec 2020 11:02:39 +0000 Subject: [PATCH 7/7] Remove 64-bit MultiplyByQuantizedMultiplier tests using a negative multiplier as they are not supported --- tensorflow/lite/kernels/internal/quantization_util_test.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tensorflow/lite/kernels/internal/quantization_util_test.cc b/tensorflow/lite/kernels/internal/quantization_util_test.cc index c00593de754..b14b7e51007 100644 --- a/tensorflow/lite/kernels/internal/quantization_util_test.cc +++ b/tensorflow/lite/kernels/internal/quantization_util_test.cc @@ -466,16 +466,14 @@ TEST(QuantizationUtilTest, MultiplyByQuantizedMultiplierInt64) { return MultiplyByQuantizedMultiplier(x, quantized_multiplier, shift); }; + // Negative multipliers are not supported by the 64-bit + // MultiplyByQuantizedMultiplier, only use >= 0 multipliers. EXPECT_EQ(quant_and_multiply(0, 0.1), 0); EXPECT_EQ(quant_and_multiply(1, 0), 0); EXPECT_EQ(quant_and_multiply(10000, 0.00097656), 10); - EXPECT_EQ(quant_and_multiply(10000, -0.00097656), -10); EXPECT_EQ(quant_and_multiply(-10000, 0.00097656), -10); - EXPECT_EQ(quant_and_multiply(-10000, -0.00097656), 10); EXPECT_EQ(quant_and_multiply(-(1ll << 47), 0.00001), -1407385600); - EXPECT_EQ(quant_and_multiply(-(1ll << 47), -0.00001), 1407385600); EXPECT_EQ(quant_and_multiply((1ll << 47) - 1, 0.00001), 1407385600); - EXPECT_EQ(quant_and_multiply((1ll << 47) - 1, -0.00001), -1407385600); // Test with maximum possible x and quantized_multiplier const int64_t x = (1ll << 47) - 1;