From 7ee63b47f1f97e7bbe2ed2e29fd6423f5355c2a9 Mon Sep 17 00:00:00 2001 From: pete Date: Thu, 16 Sep 2021 23:14:52 +0100 Subject: [PATCH] Avoid left shift of negative value in soft-fp16 --- Source/UnitTest/cmake_core.cmake | 13 +++++- Source/UnitTest/test_softfloat.cpp | 68 ++++++++++++++++++++++++++++ Source/astcenc_mathlib_softfloat.cpp | 63 ++++++++++++++------------ 3 files changed, 113 insertions(+), 31 deletions(-) create mode 100644 Source/UnitTest/test_softfloat.cpp diff --git a/Source/UnitTest/cmake_core.cmake b/Source/UnitTest/cmake_core.cmake index 37607e8..656f352 100644 --- a/Source/UnitTest/cmake_core.cmake +++ b/Source/UnitTest/cmake_core.cmake @@ -16,9 +16,9 @@ # ---------------------------------------------------------------------------- if(${UNIVERSAL_BUILD}) - set(ASTC_TEST test-simd) + set(ASTC_TEST test-unit) else() - set(ASTC_TEST test-simd-${ISA_SIMD}) + set(ASTC_TEST test-unit-${ISA_SIMD}) endif() add_executable(${ASTC_TEST}) @@ -26,6 +26,7 @@ add_executable(${ASTC_TEST}) target_sources(${ASTC_TEST} PRIVATE test_simd.cpp + test_softfloat.cpp ../astcenc_mathlib_softfloat.cpp) target_include_directories(${ASTC_TEST} @@ -119,6 +120,14 @@ elseif(${ISA_SIMD} MATCHES "avx2") endif() +target_compile_options(${ASTC_TEST} + PRIVATE + $<$:-fsanitize=undefined>) + +target_link_options(${ASTC_TEST} + PRIVATE + $<$:-fsanitize=undefined>) + target_link_libraries(${ASTC_TEST} PRIVATE gtest_main) diff --git a/Source/UnitTest/test_softfloat.cpp b/Source/UnitTest/test_softfloat.cpp new file mode 100644 index 0000000..d38913f --- /dev/null +++ b/Source/UnitTest/test_softfloat.cpp @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: Apache-2.0 +// ---------------------------------------------------------------------------- +// Copyright 2020-2021 Arm Limited +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not +// use this file except in compliance with the License. You may obtain a copy +// of the License at: +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +// License for the specific language governing permissions and limitations +// under the License. +// ---------------------------------------------------------------------------- + +/** + * @brief Unit tests for the vectorized SIMD functionality. + * + * This test suite is a partial implementation, focussing on 4-wide vectors. + * We're adding things as we touch related parts of the code, but there is some + * technical debt to catch up on to get full coverage. + */ + +#include + +#include "gtest/gtest.h" + +#include "../astcenc_internal.h" +#include "../astcenc_vecmathlib.h" + +namespace astcenc +{ + +#if (ASTCENC_F16C == 0) && (ASTCENC_NEON == 0) + +/** @brief Test normal numbers. */ +TEST(softfloat, FP16NormalNumbers) +{ + float result = sf16_to_float((15 << 10) + 1); + EXPECT_NEAR(result, 1.00098f, 0.00005f); +} + +/** @brief Test VLA loop limit round down. */ +TEST(softfloat, FP16DenormalNumbers) +{ + float result = sf16_to_float((0 << 10) + 1); + EXPECT_NEAR(result, 5.96046e-08f, 0.00005f); +} + +/** @brief Test zero. */ +TEST(softfloat, FP16Zero) +{ + float result = sf16_to_float(0x0000); + EXPECT_EQ(result, 0.0f); +} + +/** @brief Test NaN. */ +TEST(softfloat, FP16NaN) +{ + float result = sf16_to_float(0xFFFF); + EXPECT_FALSE(result == result); +} + +#endif + +} diff --git a/Source/astcenc_mathlib_softfloat.cpp b/Source/astcenc_mathlib_softfloat.cpp index a17305d..2665c0d 100644 --- a/Source/astcenc_mathlib_softfloat.cpp +++ b/Source/astcenc_mathlib_softfloat.cpp @@ -147,42 +147,47 @@ static sf32 sf16_to_sf32(sf16 inp) with just 1 table lookup, 2 shifts and 1 add. */ - #define WITH_MB(a) INT32_C((a) | (1 << 31)) - static const int32_t tbl[64] = + #define WITH_MSB(a) (UINT32_C(a) | (1u << 31)) + static const uint32_t tbl[64] = { - WITH_MB(0x00000), INT32_C(0x1C000), INT32_C(0x1C000), INT32_C(0x1C000), INT32_C(0x1C000), INT32_C(0x1C000), INT32_C(0x1C000), INT32_C(0x1C000), - INT32_C(0x1C000), INT32_C(0x1C000), INT32_C(0x1C000), INT32_C(0x1C000), INT32_C(0x1C000), INT32_C(0x1C000), INT32_C(0x1C000), INT32_C(0x1C000), - INT32_C(0x1C000), INT32_C(0x1C000), INT32_C(0x1C000), INT32_C(0x1C000), INT32_C(0x1C000), INT32_C(0x1C000), INT32_C(0x1C000), INT32_C(0x1C000), - INT32_C(0x1C000), INT32_C(0x1C000), INT32_C(0x1C000), INT32_C(0x1C000), INT32_C(0x1C000), INT32_C(0x1C000), INT32_C(0x1C000), WITH_MB(0x38000), - WITH_MB(0x38000), INT32_C(0x54000), INT32_C(0x54000), INT32_C(0x54000), INT32_C(0x54000), INT32_C(0x54000), INT32_C(0x54000), INT32_C(0x54000), - INT32_C(0x54000), INT32_C(0x54000), INT32_C(0x54000), INT32_C(0x54000), INT32_C(0x54000), INT32_C(0x54000), INT32_C(0x54000), INT32_C(0x54000), - INT32_C(0x54000), INT32_C(0x54000), INT32_C(0x54000), INT32_C(0x54000), INT32_C(0x54000), INT32_C(0x54000), INT32_C(0x54000), INT32_C(0x54000), - INT32_C(0x54000), INT32_C(0x54000), INT32_C(0x54000), INT32_C(0x54000), INT32_C(0x54000), INT32_C(0x54000), INT32_C(0x54000), WITH_MB(0x70000) + WITH_MSB(0x00000), 0x1C000, 0x1C000, 0x1C000, 0x1C000, 0x1C000, 0x1C000, 0x1C000, + 0x1C000, 0x1C000, 0x1C000, 0x1C000, 0x1C000, 0x1C000, 0x1C000, 0x1C000, + 0x1C000, 0x1C000, 0x1C000, 0x1C000, 0x1C000, 0x1C000, 0x1C000, 0x1C000, + 0x1C000, 0x1C000, 0x1C000, 0x1C000, 0x1C000, 0x1C000, 0x1C000, WITH_MSB(0x38000), + WITH_MSB(0x38000), 0x54000, 0x54000, 0x54000, 0x54000, 0x54000, 0x54000, 0x54000, + 0x54000, 0x54000, 0x54000, 0x54000, 0x54000, 0x54000, 0x54000, 0x54000, + 0x54000, 0x54000, 0x54000, 0x54000, 0x54000, 0x54000, 0x54000, 0x54000, + 0x54000, 0x54000, 0x54000, 0x54000, 0x54000, 0x54000, 0x54000, WITH_MSB(0x70000) }; - int32_t res = tbl[inpx >> 10]; + uint32_t res = tbl[inpx >> 10]; res += inpx; - /* the normal cases: the MSB of 'res' is not set. */ - if (res >= 0) /* signed compare */ - return res << 13; - - /* Infinity and Zero: the bottom 10 bits of 'res' are clear. */ - if ((res & UINT32_C(0x3FF)) == 0) - return res << 13; - - /* NaN: the exponent field of 'inp' is not zero; NaNs must be quietened. */ - if ((inpx & 0x7C00) != 0) - return (res << 13) | UINT32_C(0x400000); - - /* the remaining cases are Denormals. */ + /* Normal cases: MSB of 'res' not set. */ + if ((res & WITH_MSB(0)) == 0) { - uint32_t sign = (inpx & UINT32_C(0x8000)) << 16; - uint32_t mskval = inpx & UINT32_C(0x7FFF); - uint32_t leadingzeroes = clz32(mskval); - mskval <<= leadingzeroes; - return (mskval >> 8) + ((0x85 - leadingzeroes) << 23) + sign; + return res << 13; } + + /* Infinity and Zero: 10 LSB of 'res' not set. */ + if ((res & 0x3FF) == 0) + { + return res << 13; + } + + /* NaN: the exponent field of 'inp' is non-zero. */ + if ((inpx & 0x7C00) != 0) + { + /* All NaNs are quietened. */ + return (res << 13) | 0x400000; + } + + /* Denormal cases */ + uint32_t sign = (inpx & 0x8000) << 16; + uint32_t mskval = inpx & 0x7FFF; + uint32_t leadingzeroes = clz32(mskval); + mskval <<= leadingzeroes; + return (mskval >> 8) + ((0x85 - leadingzeroes) << 23) + sign; } /* Conversion routine that converts from FP32 to FP16. It supports denormals and all rounding modes. If a NaN is given as input, it is quietened. */