mirrors/llvm-project
0
0
Fork 0
mirror of https://github.com/llvm/llvm-project.git synced 2025-04-17 18:16:42 +00:00
Code Issues Projects Releases Wiki Activity

[libc][math] Fix incorrect logic in fputil::generic::add_or_sub (#116129)

Browse Source 
Fixes incorrect logic that went unnoticed until the function was tested
with output and input types that have the same underlying floating-point
format.
This commit is contained in:
OverMighty 2025-03-18 15:04:18 +01:00 committed by GitHub
parent 2fbfbf499e
commit 1bb8b656a9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 285 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
libc/src/__support/FPUtil/generic/CMakeLists.txt
View File

@ -56,7 +56,6 @@ add_header_library(
HDRS
add_sub.h
DEPENDS
libc.hdr.errno_macros
libc.hdr.fenv_macros
libc.src.__support.CPP.algorithm
libc.src.__support.CPP.bit

27
libc/src/__support/FPUtil/generic/add_sub.h
View File

@ -9,7 +9,6 @@
#ifndef LLVM_LIBC_SRC___SUPPORT_FPUTIL_GENERIC_ADD_SUB_H
#define LLVM_LIBC_SRC___SUPPORT_FPUTIL_GENERIC_ADD_SUB_H
#include "hdr/errno_macros.h"
#include "hdr/fenv_macros.h"
#include "src/__support/CPP/algorithm.h"
#include "src/__support/CPP/bit.h"
@ -110,12 +109,8 @@ add_or_sub(InType x, InType y) {
return cast<OutType>(tmp);
}
if (y_bits.is_zero()) {
volatile InType tmp = y;
if constexpr (IsSub)
tmp = -tmp;
return cast<OutType>(tmp);
}
if (y_bits.is_zero())
return cast<OutType>(x);
}
InType x_abs = x_bits.abs().get_val();
@ -160,20 +155,22 @@ add_or_sub(InType x, InType y) {
} else {
InStorageType max_mant = max_bits.get_explicit_mantissa() << GUARD_BITS_LEN;
InStorageType min_mant = min_bits.get_explicit_mantissa() << GUARD_BITS_LEN;
int alignment =
max_bits.get_biased_exponent() - min_bits.get_biased_exponent();
int alignment = (max_bits.get_biased_exponent() - max_bits.is_normal()) -
(min_bits.get_biased_exponent() - min_bits.is_normal());
InStorageType aligned_min_mant =
min_mant >> cpp::min(alignment, RESULT_MANTISSA_LEN);
bool aligned_min_mant_sticky;
if (alignment <= 3)
if (alignment <= GUARD_BITS_LEN)
aligned_min_mant_sticky = false;
else if (alignment <= InFPBits::FRACTION_LEN + 3)
aligned_min_mant_sticky =
(min_mant << (InFPBits::STORAGE_LEN - alignment)) != 0;
else
else if (alignment > InFPBits::FRACTION_LEN + GUARD_BITS_LEN)
aligned_min_mant_sticky = true;
else
aligned_min_mant_sticky =
(static_cast<InStorageType>(
min_mant << (InFPBits::STORAGE_LEN - alignment))) != 0;
InStorageType min_mant_sticky(static_cast<int>(aligned_min_mant_sticky));
@ -183,7 +180,7 @@ add_or_sub(InType x, InType y) {
result_mant = max_mant - (aligned_min_mant | min_mant_sticky);
}
int result_exp = max_bits.get_exponent() - RESULT_FRACTION_LEN;
int result_exp = max_bits.get_explicit_exponent() - RESULT_FRACTION_LEN;
DyadicFloat result(result_sign, result_exp, result_mant);
return result.template as<OutType, /*ShouldSignalExceptions=*/true>();
}

41
libc/test/src/math/AddTest.h
View File

@ -9,6 +9,7 @@
#ifndef LLVM_LIBC_TEST_SRC_MATH_ADDTEST_H
#define LLVM_LIBC_TEST_SRC_MATH_ADDTEST_H
#include "src/__support/CPP/algorithm.h"
#include "test/UnitTest/FEnvSafeTest.h"
#include "test/UnitTest/FPMatcher.h"
#include "test/UnitTest/Test.h"
@ -36,16 +37,19 @@ class AddTest : public LIBC_NAMESPACE::testing::FEnvSafeTest {
InFPBits::min_subnormal().uintval();
public:
typedef OutType (*AddFunc)(InType, InType);
using AddFunc = OutType (*)(InType, InType);
void test_subnormal_range(AddFunc func) {
constexpr InStorageType COUNT = 100'001;
constexpr InStorageType STEP =
(IN_MAX_SUBNORMAL_U - IN_MIN_SUBNORMAL_U) / COUNT;
for (InStorageType i = 0, v = 0, w = IN_MAX_SUBNORMAL_U; i <= COUNT;
++i, v += STEP, w -= STEP) {
InType x = InFPBits(v).get_val();
InType y = InFPBits(w).get_val();
constexpr int COUNT = 100'001;
constexpr InStorageType STEP = LIBC_NAMESPACE::cpp::max(
static_cast<InStorageType>((IN_MAX_SUBNORMAL_U - IN_MIN_SUBNORMAL_U) /
COUNT),
InStorageType(1));
for (InStorageType i = IN_MIN_SUBNORMAL_U; i <= IN_MAX_SUBNORMAL_U;
i += STEP) {
InType x = InFPBits(i).get_val();
InType y = InFPBits(static_cast<InStorageType>(IN_MAX_SUBNORMAL_U - i))
.get_val();
mpfr::BinaryInput<InType> input{x, y};
EXPECT_MPFR_MATCH_ALL_ROUNDING(mpfr::Operation::Add, input, func(x, y),
0.5);
@ -53,12 +57,14 @@ public:
}
void test_normal_range(AddFunc func) {
constexpr InStorageType COUNT = 100'001;
constexpr InStorageType STEP = (IN_MAX_NORMAL_U - IN_MIN_NORMAL_U) / COUNT;
for (InStorageType i = 0, v = 0, w = IN_MAX_NORMAL_U; i <= COUNT;
++i, v += STEP, w -= STEP) {
InType x = InFPBits(v).get_val();
InType y = InFPBits(w).get_val();
constexpr int COUNT = 100'001;
constexpr InStorageType STEP = LIBC_NAMESPACE::cpp::max(
static_cast<InStorageType>((IN_MAX_NORMAL_U - IN_MIN_NORMAL_U) / COUNT),
InStorageType(1));
for (InStorageType i = IN_MIN_NORMAL_U; i <= IN_MAX_NORMAL_U; i += STEP) {
InType x = InFPBits(i).get_val();
InType y =
InFPBits(static_cast<InStorageType>(IN_MAX_NORMAL_U - i)).get_val();
mpfr::BinaryInput<InType> input{x, y};
EXPECT_MPFR_MATCH_ALL_ROUNDING(mpfr::Operation::Add, input, func(x, y),
0.5);
@ -71,4 +77,11 @@ public:
TEST_F(LlvmLibcAddTest, SubnormalRange) { test_subnormal_range(&func); } \
TEST_F(LlvmLibcAddTest, NormalRange) { test_normal_range(&func); }
#define LIST_ADD_SAME_TYPE_TESTS(suffix, OutType, InType, func) \
using LlvmLibcAddTest##suffix = AddTest<OutType, InType>; \
TEST_F(LlvmLibcAddTest##suffix, SubnormalRange) { \
test_subnormal_range(&func); \
} \
TEST_F(LlvmLibcAddTest##suffix, NormalRange) { test_normal_range(&func); }
#endif // LLVM_LIBC_TEST_SRC_MATH_ADDTEST_H

29
libc/test/src/math/CMakeLists.txt
View File

@ -2800,6 +2800,35 @@ add_fp_unittest(
libc.src.stdlib.srand
)
add_fp_unittest(
add_same_type_test
NEED_MPFR
SUITE
libc-math-unittests
SRCS
add_same_type_test.cpp
HDRS
AddTest.h
DEPENDS
libc.src.__support.CPP.algorithm
libc.src.__support.FPUtil.generic.add_sub
libc.src.__support.macros.properties.types
)
add_fp_unittest(
sub_same_type_test
NEED_MPFR
SUITE
libc-math-unittests
SRCS
sub_same_type_test.cpp
HDRS
SubTest.h
DEPENDS
libc.src.__support.CPP.algorithm
libc.src.__support.FPUtil.generic.add_sub
libc.src.__support.macros.properties.types
)
add_subdirectory(generic)
add_subdirectory(smoke)

39
libc/test/src/math/SubTest.h
View File

@ -9,6 +9,7 @@
#ifndef LLVM_LIBC_TEST_SRC_MATH_SUBTEST_H
#define LLVM_LIBC_TEST_SRC_MATH_SUBTEST_H
#include "src/__support/CPP/algorithm.h"
#include "test/UnitTest/FEnvSafeTest.h"
#include "test/UnitTest/FPMatcher.h"
#include "test/UnitTest/Test.h"
@ -39,13 +40,16 @@ public:
using SubFunc = OutType (*)(InType, InType);
void test_subnormal_range(SubFunc func) {
constexpr InStorageType COUNT = 100'001;
constexpr InStorageType STEP =
(IN_MAX_SUBNORMAL_U - IN_MIN_SUBNORMAL_U) / COUNT;
for (InStorageType i = 0, v = 0, w = IN_MAX_SUBNORMAL_U; i <= COUNT;
++i, v += STEP, w -= STEP) {
InType x = InFPBits(v).get_val();
InType y = InFPBits(w).get_val();
constexpr int COUNT = 100'001;
constexpr InStorageType STEP = LIBC_NAMESPACE::cpp::max(
static_cast<InStorageType>((IN_MAX_SUBNORMAL_U - IN_MIN_SUBNORMAL_U) /
COUNT),
InStorageType(1));
for (InStorageType i = IN_MIN_SUBNORMAL_U; i <= IN_MAX_SUBNORMAL_U;
i += STEP) {
InType x = InFPBits(i).get_val();
InType y = InFPBits(static_cast<InStorageType>(IN_MAX_SUBNORMAL_U - i))
.get_val();
mpfr::BinaryInput<InType> input{x, y};
EXPECT_MPFR_MATCH_ALL_ROUNDING(mpfr::Operation::Sub, input, func(x, y),
0.5);
@ -53,12 +57,14 @@ public:
}
void test_normal_range(SubFunc func) {
constexpr InStorageType COUNT = 100'001;
constexpr InStorageType STEP = (IN_MAX_NORMAL_U - IN_MIN_NORMAL_U) / COUNT;
for (InStorageType i = 0, v = 0, w = IN_MAX_NORMAL_U; i <= COUNT;
++i, v += STEP, w -= STEP) {
InType x = InFPBits(v).get_val();
InType y = InFPBits(w).get_val();
constexpr int COUNT = 100'001;
constexpr InStorageType STEP = LIBC_NAMESPACE::cpp::max(
static_cast<InStorageType>((IN_MAX_NORMAL_U - IN_MIN_NORMAL_U) / COUNT),
InStorageType(1));
for (InStorageType i = IN_MIN_NORMAL_U; i <= IN_MAX_NORMAL_U; i += STEP) {
InType x = InFPBits(i).get_val();
InType y =
InFPBits(static_cast<InStorageType>(IN_MAX_NORMAL_U - i)).get_val();
mpfr::BinaryInput<InType> input{x, y};
EXPECT_MPFR_MATCH_ALL_ROUNDING(mpfr::Operation::Sub, input, func(x, y),
0.5);
@ -71,4 +77,11 @@ public:
TEST_F(LlvmLibcSubTest, SubnormalRange) { test_subnormal_range(&func); } \
TEST_F(LlvmLibcSubTest, NormalRange) { test_normal_range(&func); }
#define LIST_SUB_SAME_TYPE_TESTS(suffix, OutType, InType, func) \
using LlvmLibcSubTest##suffix = SubTest<OutType, InType>; \
TEST_F(LlvmLibcSubTest##suffix, SubnormalRange) { \
test_subnormal_range(&func); \
} \
TEST_F(LlvmLibcSubTest##suffix, NormalRange) { test_normal_range(&func); }
#endif // LLVM_LIBC_TEST_SRC_MATH_SUBTEST_H

25
libc/test/src/math/add_same_type_test.cpp Normal file
View File

@ -0,0 +1,25 @@
//===-- Unittests for fputil::generic::add --------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
#include "AddTest.h"
#include "src/__support/FPUtil/generic/add_sub.h"
#include "src/__support/macros/properties/types.h"
#define ADD_FUNC(T) (LIBC_NAMESPACE::fputil::generic::add<T, T>)
LIST_ADD_SAME_TYPE_TESTS(Double, double, double, ADD_FUNC(double))
LIST_ADD_SAME_TYPE_TESTS(Float, float, float, ADD_FUNC(float))
LIST_ADD_SAME_TYPE_TESTS(LongDouble, long double, long double,
ADD_FUNC(long double))
#ifdef LIBC_TYPES_HAS_FLOAT16
LIST_ADD_SAME_TYPE_TESTS(Float16, float16, float16, ADD_FUNC(float16))
#endif
#ifdef LIBC_TYPES_HAS_FLOAT128
LIST_ADD_SAME_TYPE_TESTS(Float128, float128, float128, ADD_FUNC(float128))
#endif

34
libc/test/src/math/smoke/AddTest.h
View File

@ -11,7 +11,6 @@
#include "hdr/errno_macros.h"
#include "hdr/fenv_macros.h"
#include "src/__support/FPUtil/BasicOperations.h"
#include "src/__support/macros/properties/os.h"
#include "test/UnitTest/FEnvSafeTest.h"
#include "test/UnitTest/FPMatcher.h"
@ -59,6 +58,10 @@ public:
#ifndef LIBC_TARGET_OS_IS_WINDOWS
using namespace LIBC_NAMESPACE::fputil::testing;
if (LIBC_NAMESPACE::fputil::get_fp_type<OutType>() ==
LIBC_NAMESPACE::fputil::get_fp_type<InType>())
return;
if (ForceRoundingMode r(RoundingMode::Nearest); r.success) {
EXPECT_FP_EQ_WITH_EXCEPTION(inf, func(in.max_normal, in.max_normal),
FE_OVERFLOW | FE_INEXACT);
@ -136,6 +139,16 @@ public:
func(InType(1.0), in.min_denormal);
EXPECT_FP_EXCEPTION(FE_INEXACT);
}
void test_mixed_normality(AddFunc func) {
if (LIBC_NAMESPACE::fputil::get_fp_type<OutType>() !=
LIBC_NAMESPACE::fputil::get_fp_type<InType>())
return;
EXPECT_FP_EQ(FPBits::create_value(Sign::POS, 2U, 0b1U).get_val(),
func(InFPBits::create_value(Sign::POS, 2U, 0U).get_val(),
InFPBits::create_value(Sign::POS, 0U, 0b10U).get_val()));
}
};
#define LIST_ADD_TESTS(OutType, InType, func) \
@ -145,6 +158,23 @@ public:
test_invalid_operations(&func); \
} \
TEST_F(LlvmLibcAddTest, RangeErrors) { test_range_errors(&func); } \
TEST_F(LlvmLibcAddTest, InexactResults) { test_inexact_results(&func); }
TEST_F(LlvmLibcAddTest, InexactResults) { test_inexact_results(&func); } \
TEST_F(LlvmLibcAddTest, MixedNormality) { test_mixed_normality(&func); }
#define LIST_ADD_SAME_TYPE_TESTS(suffix, OutType, InType, func) \
using LlvmLibcAddTest##suffix = AddTest<OutType, InType>; \
TEST_F(LlvmLibcAddTest##suffix, SpecialNumbers) { \
test_special_numbers(&func); \
} \
TEST_F(LlvmLibcAddTest##suffix, InvalidOperations) { \
test_invalid_operations(&func); \
} \
TEST_F(LlvmLibcAddTest##suffix, RangeErrors) { test_range_errors(&func); } \
TEST_F(LlvmLibcAddTest##suffix, InexactResults) { \
test_inexact_results(&func); \
} \
TEST_F(LlvmLibcAddTest##suffix, MixedNormality) { \
test_mixed_normality(&func); \
}
#endif // LLVM_LIBC_TEST_SRC_MATH_SMOKE_ADDTEST_H

32
libc/test/src/math/smoke/CMakeLists.txt
View File

@ -5160,3 +5160,35 @@ add_fp_unittest(
DEPENDS
libc.src.math.ddivf128
)
add_fp_unittest(
add_same_type_test
SUITE
libc-math-smoke-tests
SRCS
add_same_type_test.cpp
HDRS
AddTest.h
DEPENDS
libc.hdr.errno_macros
libc.hdr.fenv_macros
libc.src.__support.FPUtil.generic.add_sub
libc.src.__support.macros.properties.os
libc.src.__support.macros.properties.types
)
add_fp_unittest(
sub_same_type_test
SUITE
libc-math-smoke-tests
SRCS
sub_same_type_test.cpp
HDRS
SubTest.h
DEPENDS
libc.hdr.errno_macros
libc.hdr.fenv_macros
libc.src.__support.FPUtil.generic.add_sub
libc.src.__support.macros.properties.os
libc.src.__support.macros.properties.types
)

17
libc/test/src/math/smoke/SubTest.h
View File

@ -58,6 +58,10 @@ public:
#ifndef LIBC_TARGET_OS_IS_WINDOWS
using namespace LIBC_NAMESPACE::fputil::testing;
if (LIBC_NAMESPACE::fputil::get_fp_type<OutType>() ==
LIBC_NAMESPACE::fputil::get_fp_type<InType>())
return;
if (ForceRoundingMode r(RoundingMode::Nearest); r.success) {
EXPECT_FP_EQ_WITH_EXCEPTION(inf, func(in.max_normal, in.neg_max_normal),
FE_OVERFLOW | FE_INEXACT);
@ -147,4 +151,17 @@ public:
TEST_F(LlvmLibcSubTest, RangeErrors) { test_range_errors(&func); } \
TEST_F(LlvmLibcSubTest, InexactResults) { test_inexact_results(&func); }
#define LIST_SUB_SAME_TYPE_TESTS(suffix, OutType, InType, func) \
using LlvmLibcSubTest##suffix = SubTest<OutType, InType>; \
TEST_F(LlvmLibcSubTest##suffix, SpecialNumbers) { \
test_special_numbers(&func); \
} \
TEST_F(LlvmLibcSubTest##suffix, InvalidOperations) { \
test_invalid_operations(&func); \
} \
TEST_F(LlvmLibcSubTest##suffix, RangeErrors) { test_range_errors(&func); } \
TEST_F(LlvmLibcSubTest##suffix, InexactResults) { \
test_inexact_results(&func); \
}
#endif // LLVM_LIBC_TEST_SRC_MATH_SMOKE_SUBTEST_H

25
libc/test/src/math/smoke/add_same_type_test.cpp Normal file
View File

@ -0,0 +1,25 @@
//===-- Unittests for fputil::generic::add --------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
#include "AddTest.h"
#include "src/__support/FPUtil/generic/add_sub.h"
#include "src/__support/macros/properties/types.h"
#define ADD_FUNC(T) (LIBC_NAMESPACE::fputil::generic::add<T, T>)
LIST_ADD_SAME_TYPE_TESTS(Double, double, double, ADD_FUNC(double))
LIST_ADD_SAME_TYPE_TESTS(Float, float, float, ADD_FUNC(float))
LIST_ADD_SAME_TYPE_TESTS(LongDouble, long double, long double,
ADD_FUNC(long double))
#ifdef LIBC_TYPES_HAS_FLOAT16
LIST_ADD_SAME_TYPE_TESTS(Float16, float16, float16, ADD_FUNC(float16))
#endif
#ifdef LIBC_TYPES_HAS_FLOAT128
LIST_ADD_SAME_TYPE_TESTS(Float128, float128, float128, ADD_FUNC(float128))
#endif

25
libc/test/src/math/smoke/sub_same_type_test.cpp Normal file
View File

@ -0,0 +1,25 @@
//===-- Unittests for fputil::generic::sub --------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
#include "SubTest.h"
#include "src/__support/FPUtil/generic/add_sub.h"
#include "src/__support/macros/properties/types.h"
#define SUB_FUNC(T) (LIBC_NAMESPACE::fputil::generic::sub<T, T>)
LIST_SUB_SAME_TYPE_TESTS(Double, double, double, SUB_FUNC(double))
LIST_SUB_SAME_TYPE_TESTS(Float, float, float, SUB_FUNC(float))
LIST_SUB_SAME_TYPE_TESTS(LongDouble, long double, long double,
SUB_FUNC(long double))
#ifdef LIBC_TYPES_HAS_FLOAT16
LIST_SUB_SAME_TYPE_TESTS(Float16, float16, float16, SUB_FUNC(float16))
#endif
#ifdef LIBC_TYPES_HAS_FLOAT128
LIST_SUB_SAME_TYPE_TESTS(Float128, float128, float128, SUB_FUNC(float128))
#endif

25
libc/test/src/math/sub_same_type_test.cpp Normal file
View File

@ -0,0 +1,25 @@
//===-- Unittests for fputil::generic::sub --------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
#include "SubTest.h"
#include "src/__support/FPUtil/generic/add_sub.h"
#include "src/__support/macros/properties/types.h"
#define SUB_FUNC(T) (LIBC_NAMESPACE::fputil::generic::sub<T, T>)
LIST_SUB_SAME_TYPE_TESTS(Double, double, double, SUB_FUNC(double))
LIST_SUB_SAME_TYPE_TESTS(Float, float, float, SUB_FUNC(float))
LIST_SUB_SAME_TYPE_TESTS(LongDouble, long double, long double,
SUB_FUNC(long double))
#ifdef LIBC_TYPES_HAS_FLOAT16
LIST_SUB_SAME_TYPE_TESTS(Float16, float16, float16, SUB_FUNC(float16))
#endif
#ifdef LIBC_TYPES_HAS_FLOAT128
LIST_SUB_SAME_TYPE_TESTS(Float128, float128, float128, SUB_FUNC(float128))
#endif

10
libc/utils/MPFRWrapper/MPFRUtils.cpp
View File

@ -401,6 +401,10 @@ template void explain_binary_operation_one_output_error(
template void explain_binary_operation_one_output_error(
Operation, const BinaryInput<long double> &, float16, double, RoundingMode);
#endif
#ifdef LIBC_TYPES_HAS_FLOAT128
template void explain_binary_operation_one_output_error(
Operation, const BinaryInput<float128> &, float128, double, RoundingMode);
#endif
template <typename InputType, typename OutputType>
void explain_ternary_operation_one_output_error(
@ -625,6 +629,12 @@ template bool
compare_binary_operation_one_output(Operation, const BinaryInput<long double> &,
float16, double, RoundingMode);
#endif
#ifdef LIBC_TYPES_HAS_FLOAT128
template bool compare_binary_operation_one_output(Operation,
const BinaryInput<float128> &,
float128, double,
RoundingMode);
#endif
template <typename InputType, typename OutputType>
bool compare_ternary_operation_one_output(Operation op,