[libc-commits] [libc] [libc] add FXBits class (PR #82065)
Michael Jones via libc-commits
libc-commits at lists.llvm.org
Wed Feb 21 15:25:43 PST 2024
https://github.com/michaelrj-google updated https://github.com/llvm/llvm-project/pull/82065
>From 1fa32df82aa85d35743e2201e5d2c8c5ad6aed1f Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Fri, 16 Feb 2024 15:06:07 -0800
Subject: [PATCH 1/4] [libc] add FXBits class
The FXBits class is what will be used to modify fixed point numbers on a
bit level. This patch adds a basic implementation as well as basic
tests.
---
libc/src/__support/fixed_point/CMakeLists.txt | 1 +
libc/src/__support/fixed_point/fx_bits.h | 68 +++++++++++++++
libc/test/src/__support/CMakeLists.txt | 1 +
.../test/src/__support/FPUtil/fpbits_test.cpp | 2 +-
.../src/__support/fixed_point/CMakeLists.txt | 13 +++
.../__support/fixed_point/fx_bits_test.cpp | 82 +++++++++++++++++++
6 files changed, 166 insertions(+), 1 deletion(-)
create mode 100644 libc/test/src/__support/fixed_point/CMakeLists.txt
create mode 100644 libc/test/src/__support/fixed_point/fx_bits_test.cpp
diff --git a/libc/src/__support/fixed_point/CMakeLists.txt b/libc/src/__support/fixed_point/CMakeLists.txt
index c6bb9e17adfa8e..4203e5914acb60 100644
--- a/libc/src/__support/fixed_point/CMakeLists.txt
+++ b/libc/src/__support/fixed_point/CMakeLists.txt
@@ -17,5 +17,6 @@ add_header_library(
libc.include.llvm-libc-macros.stdfix_macros
libc.src.__support.macros.attributes
libc.src.__support.macros.optimization
+ libc.src.__support.CPP.type_traits
libc.src.__support.CPP.bit
)
diff --git a/libc/src/__support/fixed_point/fx_bits.h b/libc/src/__support/fixed_point/fx_bits.h
index b26be169a593a3..e746010f2841b4 100644
--- a/libc/src/__support/fixed_point/fx_bits.h
+++ b/libc/src/__support/fixed_point/fx_bits.h
@@ -21,6 +21,74 @@
namespace LIBC_NAMESPACE::fixed_point {
+template <typename T> struct FXBits {
+private:
+ using fx_rep = FXRep<T>;
+ using StorageType = typename fx_rep::StorageType;
+
+ StorageType value;
+
+ static_assert(fx_rep::FRACTION_LEN > 0);
+
+ static constexpr size_t FRACTION_OFFSET = 0; // Just for completeness
+ static constexpr size_t INTEGRAL_OFFSET = fx_rep::FRACTION_LEN;
+ static constexpr size_t SIGN_OFFSET =
+ fx_rep::SIGN_LEN == 0
+ ? 0
+ : ((sizeof(StorageType) * CHAR_BIT) - fx_rep::SIGN_LEN);
+
+ static constexpr StorageType FRACTION_MASK =
+ (StorageType(1) << fx_rep::FRACTION_LEN) - 1;
+ static constexpr StorageType INTEGRAL_MASK =
+ ((StorageType(1) << fx_rep::INTEGRAL_LEN) - 1) << INTEGRAL_OFFSET;
+ static constexpr StorageType SIGN_MASK =
+ (fx_rep::SIGN_LEN == 0 ? 0 : StorageType(1) << SIGN_OFFSET);
+
+public:
+ LIBC_INLINE constexpr FXBits() = default;
+
+ template <typename XType> LIBC_INLINE constexpr explicit FXBits(XType x) {
+ using Unqual = typename cpp::remove_cv_t<XType>;
+ if constexpr (cpp::is_same_v<Unqual, T>) {
+ value = cpp::bit_cast<StorageType>(x);
+ } else if constexpr (cpp::is_same_v<Unqual, StorageType>) {
+ value = x;
+ } else {
+ // We don't want accidental type promotions/conversions, so we require
+ // exact type match.
+ static_assert(cpp::always_false<XType>);
+ }
+ }
+
+ LIBC_INLINE constexpr StorageType get_fraction() {
+ return (value & FRACTION_MASK) >> FRACTION_OFFSET;
+ }
+
+ LIBC_INLINE constexpr StorageType get_integral() {
+ return (value & INTEGRAL_MASK) >> INTEGRAL_OFFSET;
+ }
+
+ LIBC_INLINE constexpr StorageType get_sign() {
+ return (value & SIGN_MASK) >> SIGN_OFFSET;
+ }
+
+ LIBC_INLINE constexpr void set_fraction(StorageType fraction) {
+ value = (value & (~FRACTION_MASK)) |
+ ((fraction << FRACTION_OFFSET) & FRACTION_MASK);
+ }
+
+ LIBC_INLINE constexpr void set_integral(StorageType integral) {
+ value = (value & (~INTEGRAL_MASK)) |
+ ((integral << INTEGRAL_OFFSET) & INTEGRAL_MASK);
+ }
+
+ LIBC_INLINE constexpr void set_sign(StorageType sign) {
+ value = (value & (~SIGN_MASK)) | ((sign << SIGN_OFFSET) & SIGN_MASK);
+ }
+
+ LIBC_INLINE constexpr T get_val() const { return cpp::bit_cast<T>(value); }
+};
+
// Bit-wise operations are not available for fixed point types yet.
template <typename T>
LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_fixed_point_v<T>, T>
diff --git a/libc/test/src/__support/CMakeLists.txt b/libc/test/src/__support/CMakeLists.txt
index 493ef9ddabe1e0..9801621e6b3991 100644
--- a/libc/test/src/__support/CMakeLists.txt
+++ b/libc/test/src/__support/CMakeLists.txt
@@ -188,4 +188,5 @@ add_subdirectory(File)
add_subdirectory(RPC)
add_subdirectory(OSUtil)
add_subdirectory(FPUtil)
+add_subdirectory(fixed_point)
add_subdirectory(HashTable)
diff --git a/libc/test/src/__support/FPUtil/fpbits_test.cpp b/libc/test/src/__support/FPUtil/fpbits_test.cpp
index 46f7d250596873..4f9f53afe54785 100644
--- a/libc/test/src/__support/FPUtil/fpbits_test.cpp
+++ b/libc/test/src/__support/FPUtil/fpbits_test.cpp
@@ -1,4 +1,4 @@
-//===-- Unittests for the DyadicFloat class -------------------------------===//
+//===-- Unittests for the FPBits class ------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
diff --git a/libc/test/src/__support/fixed_point/CMakeLists.txt b/libc/test/src/__support/fixed_point/CMakeLists.txt
new file mode 100644
index 00000000000000..a39d4af87527de
--- /dev/null
+++ b/libc/test/src/__support/fixed_point/CMakeLists.txt
@@ -0,0 +1,13 @@
+add_custom_target(libc-fixed-point-tests)
+
+add_libc_test(
+ fx_bits_test
+ SUITE
+ libc-fixed-point-tests
+ SRCS
+ fx_bits_test.cpp
+ DEPENDS
+ libc.src.__support.fixed_point.fx_bits
+ # libc.src.__support.fixed_point.fx_bits_str TODO: make this
+ libc.src.__support.integer_literals
+)
diff --git a/libc/test/src/__support/fixed_point/fx_bits_test.cpp b/libc/test/src/__support/fixed_point/fx_bits_test.cpp
new file mode 100644
index 00000000000000..a168556351c2ce
--- /dev/null
+++ b/libc/test/src/__support/fixed_point/fx_bits_test.cpp
@@ -0,0 +1,82 @@
+//===-- Unittests for the FXBits class ------------------------------------===//
+//
+// 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 "include/llvm-libc-macros/stdfix-macros.h"
+
+#include "src/__support/fixed_point/fx_bits.h"
+// #include "src/__support/FPUtil/fx_bits_str.h"
+#include "src/__support/integer_literals.h"
+#include "test/UnitTest/Test.h"
+
+using LIBC_NAMESPACE::fixed_point::FXBits;
+using LIBC_NAMESPACE::fixed_point::FXRep;
+
+using LIBC_NAMESPACE::operator""_u8;
+using LIBC_NAMESPACE::operator""_u16;
+using LIBC_NAMESPACE::operator""_u32;
+using LIBC_NAMESPACE::operator""_u64;
+using LIBC_NAMESPACE::operator""_u128;
+
+TEST(LlvmLibcFxBitsTest, FXBits_UnsignedShortFract) {
+ auto bits_var = FXBits<unsigned short fract>(0x00_u8);
+
+ EXPECT_EQ(bits_var.get_sign(), 0x00_u8);
+ EXPECT_EQ(bits_var.get_integral(), 0x00_u8);
+ EXPECT_EQ(bits_var.get_fraction(), 0x00_u8);
+
+ // Since an unsigned fract has no sign or integral components, setting either
+ // should have no effect.
+
+ bits_var.set_sign(1);
+
+ EXPECT_EQ(bits_var.get_sign(), 0x00_u8);
+ EXPECT_EQ(bits_var.get_integral(), 0x00_u8);
+ EXPECT_EQ(bits_var.get_fraction(), 0x00_u8);
+
+ bits_var.set_integral(0xab);
+
+ EXPECT_EQ(bits_var.get_sign(), 0x00_u8);
+ EXPECT_EQ(bits_var.get_integral(), 0x00_u8);
+ EXPECT_EQ(bits_var.get_fraction(), 0x00_u8);
+
+ // but setting the fraction should work
+
+ bits_var.set_fraction(0xcd);
+
+ EXPECT_EQ(bits_var.get_sign(), 0x00_u8);
+ EXPECT_EQ(bits_var.get_integral(), 0x00_u8);
+ EXPECT_EQ(bits_var.get_fraction(), 0xcd_u8);
+}
+
+TEST(LlvmLibcFxBitsTest, FXBits_ShortAccum) {
+ auto bits_var = FXBits<short accum>(0b0'00000000'0000000_u16);
+
+ EXPECT_EQ(bits_var.get_sign(), 0x0000_u16);
+ EXPECT_EQ(bits_var.get_integral(), 0x0000_u16);
+ EXPECT_EQ(bits_var.get_fraction(), 0x0000_u16);
+
+ bits_var.set_sign(0xffff); // one sign bit used
+
+ EXPECT_EQ(bits_var.get_sign(), 0x0001_u16);
+ EXPECT_EQ(bits_var.get_integral(), 0x0000_u16);
+ EXPECT_EQ(bits_var.get_fraction(), 0x0000_u16);
+
+ bits_var.set_integral(0xabcd_u16); // 8 integral bits used
+
+ EXPECT_EQ(bits_var.get_sign(), 0x0001_u16);
+ EXPECT_EQ(bits_var.get_integral(), 0x00cd_u16);
+ EXPECT_EQ(bits_var.get_fraction(), 0x0000_u16);
+
+ bits_var.set_fraction(0x21fe_u16); // 7 fract bits used
+
+ EXPECT_EQ(bits_var.get_sign(), 0x0001_u16);
+ EXPECT_EQ(bits_var.get_integral(), 0x00cd_u16);
+ EXPECT_EQ(bits_var.get_fraction(), 0x007e_u16);
+}
+
+// TODO: more types
>From 50bb1abaa335520131649373692b2776325f9f28 Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Tue, 20 Feb 2024 10:43:23 -0800
Subject: [PATCH 2/4] address comments
---
libc/src/__support/fixed_point/CMakeLists.txt | 1 +
libc/src/__support/fixed_point/fx_bits.h | 21 ++++++++++++-------
.../src/__support/fixed_point/CMakeLists.txt | 5 ++++-
.../__support/fixed_point/fx_bits_test.cpp | 21 +++++++++----------
4 files changed, 29 insertions(+), 19 deletions(-)
diff --git a/libc/src/__support/fixed_point/CMakeLists.txt b/libc/src/__support/fixed_point/CMakeLists.txt
index 4203e5914acb60..64f9dacc7ba5f0 100644
--- a/libc/src/__support/fixed_point/CMakeLists.txt
+++ b/libc/src/__support/fixed_point/CMakeLists.txt
@@ -19,4 +19,5 @@ add_header_library(
libc.src.__support.macros.optimization
libc.src.__support.CPP.type_traits
libc.src.__support.CPP.bit
+ libc.src.__support.math_extras
)
diff --git a/libc/src/__support/fixed_point/fx_bits.h b/libc/src/__support/fixed_point/fx_bits.h
index e746010f2841b4..8d04196a612eb2 100644
--- a/libc/src/__support/fixed_point/fx_bits.h
+++ b/libc/src/__support/fixed_point/fx_bits.h
@@ -14,6 +14,7 @@
#include "src/__support/CPP/type_traits.h"
#include "src/__support/macros/attributes.h" // LIBC_INLINE
#include "src/__support/macros/optimization.h" // LIBC_UNLIKELY
+#include "src/__support/math_extras.h"
#include "fx_rep.h"
@@ -31,16 +32,19 @@ template <typename T> struct FXBits {
static_assert(fx_rep::FRACTION_LEN > 0);
static constexpr size_t FRACTION_OFFSET = 0; // Just for completeness
- static constexpr size_t INTEGRAL_OFFSET = fx_rep::FRACTION_LEN;
+ static constexpr size_t INTEGRAL_OFFSET =
+ fx_rep::INTEGRAL_LEN == 0 ? 0 : fx_rep::FRACTION_LEN;
static constexpr size_t SIGN_OFFSET =
fx_rep::SIGN_LEN == 0
? 0
: ((sizeof(StorageType) * CHAR_BIT) - fx_rep::SIGN_LEN);
static constexpr StorageType FRACTION_MASK =
- (StorageType(1) << fx_rep::FRACTION_LEN) - 1;
+ mask_trailing_ones<StorageType, fx_rep::FRACTION_LEN>()
+ << FRACTION_OFFSET;
static constexpr StorageType INTEGRAL_MASK =
- ((StorageType(1) << fx_rep::INTEGRAL_LEN) - 1) << INTEGRAL_OFFSET;
+ mask_trailing_ones<StorageType, fx_rep::INTEGRAL_LEN>()
+ << INTEGRAL_OFFSET;
static constexpr StorageType SIGN_MASK =
(fx_rep::SIGN_LEN == 0 ? 0 : StorageType(1) << SIGN_OFFSET);
@@ -68,8 +72,9 @@ template <typename T> struct FXBits {
return (value & INTEGRAL_MASK) >> INTEGRAL_OFFSET;
}
- LIBC_INLINE constexpr StorageType get_sign() {
- return (value & SIGN_MASK) >> SIGN_OFFSET;
+ // TODO: replace bool with Sign
+ LIBC_INLINE constexpr bool get_sign() {
+ return static_cast<bool>((value & SIGN_MASK) >> SIGN_OFFSET);
}
LIBC_INLINE constexpr void set_fraction(StorageType fraction) {
@@ -82,8 +87,10 @@ template <typename T> struct FXBits {
((integral << INTEGRAL_OFFSET) & INTEGRAL_MASK);
}
- LIBC_INLINE constexpr void set_sign(StorageType sign) {
- value = (value & (~SIGN_MASK)) | ((sign << SIGN_OFFSET) & SIGN_MASK);
+ // TODO: replace bool with Sign
+ LIBC_INLINE constexpr void set_sign(bool sign) {
+ value = (value & (~SIGN_MASK)) |
+ ((static_cast<StorageType>(sign) << SIGN_OFFSET) & SIGN_MASK);
}
LIBC_INLINE constexpr T get_val() const { return cpp::bit_cast<T>(value); }
diff --git a/libc/test/src/__support/fixed_point/CMakeLists.txt b/libc/test/src/__support/fixed_point/CMakeLists.txt
index a39d4af87527de..384cc9394ee79b 100644
--- a/libc/test/src/__support/fixed_point/CMakeLists.txt
+++ b/libc/test/src/__support/fixed_point/CMakeLists.txt
@@ -1,3 +1,7 @@
+if(NOT LIBC_COMPILER_HAS_FIXED_POINT)
+ return()
+endif()
+
add_custom_target(libc-fixed-point-tests)
add_libc_test(
@@ -8,6 +12,5 @@ add_libc_test(
fx_bits_test.cpp
DEPENDS
libc.src.__support.fixed_point.fx_bits
- # libc.src.__support.fixed_point.fx_bits_str TODO: make this
libc.src.__support.integer_literals
)
diff --git a/libc/test/src/__support/fixed_point/fx_bits_test.cpp b/libc/test/src/__support/fixed_point/fx_bits_test.cpp
index a168556351c2ce..24e427fa680268 100644
--- a/libc/test/src/__support/fixed_point/fx_bits_test.cpp
+++ b/libc/test/src/__support/fixed_point/fx_bits_test.cpp
@@ -9,7 +9,6 @@
#include "include/llvm-libc-macros/stdfix-macros.h"
#include "src/__support/fixed_point/fx_bits.h"
-// #include "src/__support/FPUtil/fx_bits_str.h"
#include "src/__support/integer_literals.h"
#include "test/UnitTest/Test.h"
@@ -25,22 +24,22 @@ using LIBC_NAMESPACE::operator""_u128;
TEST(LlvmLibcFxBitsTest, FXBits_UnsignedShortFract) {
auto bits_var = FXBits<unsigned short fract>(0x00_u8);
- EXPECT_EQ(bits_var.get_sign(), 0x00_u8);
+ EXPECT_EQ(bits_var.get_sign(), false);
EXPECT_EQ(bits_var.get_integral(), 0x00_u8);
EXPECT_EQ(bits_var.get_fraction(), 0x00_u8);
// Since an unsigned fract has no sign or integral components, setting either
// should have no effect.
- bits_var.set_sign(1);
+ bits_var.set_sign(true);
- EXPECT_EQ(bits_var.get_sign(), 0x00_u8);
+ EXPECT_EQ(bits_var.get_sign(), false);
EXPECT_EQ(bits_var.get_integral(), 0x00_u8);
EXPECT_EQ(bits_var.get_fraction(), 0x00_u8);
bits_var.set_integral(0xab);
- EXPECT_EQ(bits_var.get_sign(), 0x00_u8);
+ EXPECT_EQ(bits_var.get_sign(), false);
EXPECT_EQ(bits_var.get_integral(), 0x00_u8);
EXPECT_EQ(bits_var.get_fraction(), 0x00_u8);
@@ -48,7 +47,7 @@ TEST(LlvmLibcFxBitsTest, FXBits_UnsignedShortFract) {
bits_var.set_fraction(0xcd);
- EXPECT_EQ(bits_var.get_sign(), 0x00_u8);
+ EXPECT_EQ(bits_var.get_sign(), false);
EXPECT_EQ(bits_var.get_integral(), 0x00_u8);
EXPECT_EQ(bits_var.get_fraction(), 0xcd_u8);
}
@@ -56,25 +55,25 @@ TEST(LlvmLibcFxBitsTest, FXBits_UnsignedShortFract) {
TEST(LlvmLibcFxBitsTest, FXBits_ShortAccum) {
auto bits_var = FXBits<short accum>(0b0'00000000'0000000_u16);
- EXPECT_EQ(bits_var.get_sign(), 0x0000_u16);
+ EXPECT_EQ(bits_var.get_sign(), false);
EXPECT_EQ(bits_var.get_integral(), 0x0000_u16);
EXPECT_EQ(bits_var.get_fraction(), 0x0000_u16);
- bits_var.set_sign(0xffff); // one sign bit used
+ bits_var.set_sign(true); // one sign bit used
- EXPECT_EQ(bits_var.get_sign(), 0x0001_u16);
+ EXPECT_EQ(bits_var.get_sign(), true);
EXPECT_EQ(bits_var.get_integral(), 0x0000_u16);
EXPECT_EQ(bits_var.get_fraction(), 0x0000_u16);
bits_var.set_integral(0xabcd_u16); // 8 integral bits used
- EXPECT_EQ(bits_var.get_sign(), 0x0001_u16);
+ EXPECT_EQ(bits_var.get_sign(), true);
EXPECT_EQ(bits_var.get_integral(), 0x00cd_u16);
EXPECT_EQ(bits_var.get_fraction(), 0x0000_u16);
bits_var.set_fraction(0x21fe_u16); // 7 fract bits used
- EXPECT_EQ(bits_var.get_sign(), 0x0001_u16);
+ EXPECT_EQ(bits_var.get_sign(), true);
EXPECT_EQ(bits_var.get_integral(), 0x00cd_u16);
EXPECT_EQ(bits_var.get_fraction(), 0x007e_u16);
}
>From d0eaa1a7e44a146561f84e0d3dcdcd778066fb44 Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Wed, 21 Feb 2024 15:08:45 -0800
Subject: [PATCH 3/4] add get_exponent which printf will need
---
libc/src/__support/fixed_point/fx_bits.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/libc/src/__support/fixed_point/fx_bits.h b/libc/src/__support/fixed_point/fx_bits.h
index 8d04196a612eb2..fcd47cd72cbb31 100644
--- a/libc/src/__support/fixed_point/fx_bits.h
+++ b/libc/src/__support/fixed_point/fx_bits.h
@@ -77,6 +77,9 @@ template <typename T> struct FXBits {
return static_cast<bool>((value & SIGN_MASK) >> SIGN_OFFSET);
}
+ // This represents the effective negative exponent applied to this number
+ LIBC_INLINE constexpr int get_exponent() { return fx_rep::FRACTION_LEN; }
+
LIBC_INLINE constexpr void set_fraction(StorageType fraction) {
value = (value & (~FRACTION_MASK)) |
((fraction << FRACTION_OFFSET) & FRACTION_MASK);
>From 8f042977fb852892c658cdf12faca90fbdbaeebe Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Wed, 21 Feb 2024 15:24:58 -0800
Subject: [PATCH 4/4] remove unused "using" statements
---
libc/test/src/__support/fixed_point/fx_bits_test.cpp | 3 ---
1 file changed, 3 deletions(-)
diff --git a/libc/test/src/__support/fixed_point/fx_bits_test.cpp b/libc/test/src/__support/fixed_point/fx_bits_test.cpp
index 24e427fa680268..3a0f7b720d079b 100644
--- a/libc/test/src/__support/fixed_point/fx_bits_test.cpp
+++ b/libc/test/src/__support/fixed_point/fx_bits_test.cpp
@@ -17,9 +17,6 @@ using LIBC_NAMESPACE::fixed_point::FXRep;
using LIBC_NAMESPACE::operator""_u8;
using LIBC_NAMESPACE::operator""_u16;
-using LIBC_NAMESPACE::operator""_u32;
-using LIBC_NAMESPACE::operator""_u64;
-using LIBC_NAMESPACE::operator""_u128;
TEST(LlvmLibcFxBitsTest, FXBits_UnsignedShortFract) {
auto bits_var = FXBits<unsigned short fract>(0x00_u8);
More information about the libc-commits
mailing list