[libc][stdfix] Implement fixed point bitsfx functions in llvm libc #128413
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: krishna2803 <[email protected]>
Signed-off-by: krishna2803 <[email protected]>
Signed-off-by: krishna2803 <[email protected]>
Signed-off-by: krishna2803 <[email protected]>
Signed-off-by: krishna2803 <[email protected]>
Signed-off-by: krishna2803 <[email protected]>
Signed-off-by: krishna2803 <[email protected]>
Signed-off-by: krishna2803 <[email protected]>
Signed-off-by: krishna2803 <[email protected]>
|
@llvm/pr-subscribers-libc Author: Krishna Pandey (krishna2803) ChangesFixes #113359 Patch is 48.48 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/128413.diff 47 Files Affected:
diff --git a/libc/config/baremetal/arm/entrypoints.txt b/libc/config/baremetal/arm/entrypoints.txt
index 370b5462fe9e8..97705960e7b54 100644
--- a/libc/config/baremetal/arm/entrypoints.txt
+++ b/libc/config/baremetal/arm/entrypoints.txt
@@ -507,6 +507,18 @@ if(LIBC_COMPILER_HAS_FIXED_POINT)
libc.src.stdfix.ukbits
libc.src.stdfix.lkbits
libc.src.stdfix.ulkbits
+ libc.src.stdfix.bitshr
+ libc.src.stdfix.bitsr
+ libc.src.stdfix.bitslr
+ libc.src.stdfix.bitshk
+ libc.src.stdfix.bitsk
+ libc.src.stdfix.bitslk
+ libc.src.stdfix.bitsuhr
+ libc.src.stdfix.bitsur
+ libc.src.stdfix.bitsulr
+ libc.src.stdfix.bitsuhk
+ libc.src.stdfix.bitsuk
+ libc.src.stdfix.bitsulk
libc.src.stdfix.countlshr
libc.src.stdfix.countlsr
libc.src.stdfix.countlslr
diff --git a/libc/config/baremetal/riscv/entrypoints.txt b/libc/config/baremetal/riscv/entrypoints.txt
index 07311a60a17a2..bebbc94f183cb 100644
--- a/libc/config/baremetal/riscv/entrypoints.txt
+++ b/libc/config/baremetal/riscv/entrypoints.txt
@@ -502,6 +502,18 @@ if(LIBC_COMPILER_HAS_FIXED_POINT)
libc.src.stdfix.ukbits
libc.src.stdfix.lkbits
libc.src.stdfix.ulkbits
+ libc.src.stdfix.bitshr
+ libc.src.stdfix.bitsr
+ libc.src.stdfix.bitslr
+ libc.src.stdfix.bitshk
+ libc.src.stdfix.bitsk
+ libc.src.stdfix.bitslk
+ libc.src.stdfix.bitshr
+ libc.src.stdfix.bitsur
+ libc.src.stdfix.bitsulr
+ libc.src.stdfix.bitsuhk
+ libc.src.stdfix.bitsuk
+ libc.src.stdfix.bitsulk
libc.src.stdfix.countlshr
libc.src.stdfix.countlsr
libc.src.stdfix.countlslr
diff --git a/libc/config/linux/riscv/entrypoints.txt b/libc/config/linux/riscv/entrypoints.txt
index a9ba0c257755b..978bc62635df3 100644
--- a/libc/config/linux/riscv/entrypoints.txt
+++ b/libc/config/linux/riscv/entrypoints.txt
@@ -749,6 +749,18 @@ if(LIBC_COMPILER_HAS_FIXED_POINT)
# TODO: https://github.com/llvm/llvm-project/issues/115778
libc.src.stdfix.lkbits
libc.src.stdfix.ulkbits
+ libc.src.stdfix.bitshr
+ libc.src.stdfix.bitsr
+ libc.src.stdfix.bitslr
+ libc.src.stdfix.bitshk
+ libc.src.stdfix.bitsk
+ libc.src.stdfix.bitslk
+ libc.src.stdfix.bitsuhr
+ libc.src.stdfix.bitsur
+ libc.src.stdfix.bitsulr
+ libc.src.stdfix.bitsuhk
+ libc.src.stdfix.bitsuk
+ libc.src.stdfix.bitsulk
libc.src.stdfix.countlshr
libc.src.stdfix.countlsr
libc.src.stdfix.countlslr
diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index a4f6671a59789..dbe8575405a3b 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -875,6 +875,18 @@ if(LIBC_COMPILER_HAS_FIXED_POINT)
libc.src.stdfix.ukbits
libc.src.stdfix.lkbits
libc.src.stdfix.ulkbits
+ libc.src.stdfix.bitshr
+ libc.src.stdfix.bitsr
+ libc.src.stdfix.bitslr
+ libc.src.stdfix.bitshk
+ libc.src.stdfix.bitsk
+ libc.src.stdfix.bitslk
+ libc.src.stdfix.bitsuhr
+ libc.src.stdfix.bitsur
+ libc.src.stdfix.bitsulr
+ libc.src.stdfix.bitsuhk
+ libc.src.stdfix.bitsuk
+ libc.src.stdfix.bitsulk
libc.src.stdfix.countlshr
libc.src.stdfix.countlsr
libc.src.stdfix.countlslr
diff --git a/libc/docs/headers/math/stdfix.rst b/libc/docs/headers/math/stdfix.rst
index 4507f2b608bf1..e12780a32203c 100644
--- a/libc/docs/headers/math/stdfix.rst
+++ b/libc/docs/headers/math/stdfix.rst
@@ -69,7 +69,7 @@ The following functions are included in the ISO/IEC TR 18037:2008 standard.
+===============+================+=============+===============+============+================+=============+================+=============+===============+============+================+=============+
| abs | | |check| | | |check| | | |check| | | |check| | | |check| | | |check| |
+---------------+----------------+-------------+---------------+------------+----------------+-------------+----------------+-------------+---------------+------------+----------------+-------------+
-| bits\* | | | | | | | | | | | | |
+| bits\* | | |check| | |check| | |check| | |check| | |check| | |check| | |check| | |check| | |check| | | |check| |
+---------------+----------------+-------------+---------------+------------+----------------+-------------+----------------+-------------+---------------+------------+----------------+-------------+
| \*bits | | | | | | | | | | | | |
+---------------+----------------+-------------+---------------+------------+----------------+-------------+----------------+-------------+---------------+------------+----------------+-------------+
diff --git a/libc/include/stdfix.yaml b/libc/include/stdfix.yaml
index 0abf2f3a9b3b6..707e5a2b09819 100644
--- a/libc/include/stdfix.yaml
+++ b/libc/include/stdfix.yaml
@@ -148,6 +148,90 @@ functions:
arguments:
- type: uint_ulk_t
guard: LIBC_COMPILER_HAS_FIXED_POINT
+ - name: bitshr
+ standards:
+ - stdc_ext
+ return_type: int_hr_t
+ arguments:
+ - type: short fract
+ guard: LIBC_COMPILER_HAS_FIXED_POINT
+ - name: bitsuhr
+ standards:
+ - stdc_ext
+ return_type: uint_uhr_t
+ arguments:
+ - type: unsigned short fract
+ guard: LIBC_COMPILER_HAS_FIXED_POINT
+ - name: bitsr
+ standards:
+ - stdc_ext
+ return_type: int_r_t
+ arguments:
+ - type: fract
+ guard: LIBC_COMPILER_HAS_FIXED_POINT
+ - name: bitsur
+ standards:
+ - stdc_ext
+ return_type: uint_ur_t
+ arguments:
+ - type: unsigned fract
+ guard: LIBC_COMPILER_HAS_FIXED_POINT
+ - name: bitslr
+ standards:
+ - stdc_ext
+ return_type: int_lr_t
+ arguments:
+ - type: long fract
+ guard: LIBC_COMPILER_HAS_FIXED_POINT
+ - name: bitsulr
+ standards:
+ - stdc_ext
+ return_type: uint_ulr_t
+ arguments:
+ - type: unsigned long fract
+ guard: LIBC_COMPILER_HAS_FIXED_POINT
+ - name: bitshk
+ standards:
+ - stdc_ext
+ return_type: int_hk_t
+ arguments:
+ - type: short accum
+ guard: LIBC_COMPILER_HAS_FIXED_POINT
+ - name: bitsuhk
+ standards:
+ - stdc_ext
+ return_type: uint_uhk_t
+ arguments:
+ - type: unsigned short accum
+ guard: LIBC_COMPILER_HAS_FIXED_POINT
+ - name: bitsk
+ standards:
+ - stdc_ext
+ return_type: int_k_t
+ arguments:
+ - type: accum
+ guard: LIBC_COMPILER_HAS_FIXED_POINT
+ - name: bitsuk
+ standards:
+ - stdc_ext
+ return_type: uint_uk_t
+ arguments:
+ - type: unsigned accum
+ guard: LIBC_COMPILER_HAS_FIXED_POINT
+ - name: bitslk
+ standards:
+ - stdc_ext
+ return_type: int_lk_t
+ arguments:
+ - type: long accum
+ guard: LIBC_COMPILER_HAS_FIXED_POINT
+ - name: bitsulk
+ standards:
+ - stdc_ext
+ return_type: uint_ulk_t
+ arguments:
+ - type: unsigned long accum
+ guard: LIBC_COMPILER_HAS_FIXED_POINT
- name: roundhk
standards:
- stdc_ext
diff --git a/libc/src/__support/fixed_point/fx_bits.h b/libc/src/__support/fixed_point/fx_bits.h
index 7509419da0c43..a9b7fe248f6f1 100644
--- a/libc/src/__support/fixed_point/fx_bits.h
+++ b/libc/src/__support/fixed_point/fx_bits.h
@@ -121,7 +121,7 @@ bit_and(T x, T y) {
using BitType = typename FXRep<T>::StorageType;
BitType x_bit = cpp::bit_cast<BitType>(x);
BitType y_bit = cpp::bit_cast<BitType>(y);
- // For some reason, bit_cast cannot deduce BitType from the input.
+ // For some reason, bit_cast cannot deduce BitType T the input.
return cpp::bit_cast<T, BitType>(x_bit & y_bit);
}
@@ -131,7 +131,7 @@ bit_or(T x, T y) {
using BitType = typename FXRep<T>::StorageType;
BitType x_bit = cpp::bit_cast<BitType>(x);
BitType y_bit = cpp::bit_cast<BitType>(y);
- // For some reason, bit_cast cannot deduce BitType from the input.
+ // For some reason, bit_cast cannot deduce BitType T the input.
return cpp::bit_cast<T, BitType>(x_bit | y_bit);
}
@@ -140,7 +140,7 @@ LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_fixed_point_v<T>, T>
bit_not(T x) {
using BitType = typename FXRep<T>::StorageType;
BitType x_bit = cpp::bit_cast<BitType>(x);
- // For some reason, bit_cast cannot deduce BitType from the input.
+ // For some reason, bit_cast cannot deduce BitType T the input.
return cpp::bit_cast<T, BitType>(static_cast<BitType>(~x_bit));
}
@@ -194,6 +194,13 @@ countls(T f) {
return cpp::countl_zero(value_bits) - FXRep::SIGN_LEN;
}
+// fixed-point to integer conversion
+template <typename T, typename XType>
+LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_fixed_point_v<T>, XType>
+bitsfx(T f) {
+ return cpp::bit_cast<XType, T>(f);
+}
+
} // namespace fixed_point
} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdfix/CMakeLists.txt b/libc/src/stdfix/CMakeLists.txt
index 6fb06b8d7e9ae..362af0bf0d55c 100644
--- a/libc/src/stdfix/CMakeLists.txt
+++ b/libc/src/stdfix/CMakeLists.txt
@@ -48,6 +48,20 @@ foreach(suffix IN ITEMS hr r lr hk k lk uhr ur ulr uhk uk ulk)
libc.src.__support.fixed_point.fx_bits
)
+ add_entrypoint_object(
+ bits${suffix}
+ HDRS
+ bits${suffix}.h
+ SRCS
+ bits${suffix}.cpp
+ COMPILE_OPTIONS
+ ${libc_opt_high_flag}
+ DEPENDS
+ libc.src.__support.fixed_point.fx_bits
+ libc.include.llvm-libc-types.stdfix-types
+ libc.include.llvm-libc-macros.stdfix_macros
+ )
+
add_entrypoint_object(
countls${suffix}
HDRS
diff --git a/libc/src/stdfix/bitshk.cpp b/libc/src/stdfix/bitshk.cpp
new file mode 100644
index 0000000000000..d0a3e128bdd65
--- /dev/null
+++ b/libc/src/stdfix/bitshk.cpp
@@ -0,0 +1,22 @@
+//===-- Implementation of bitshk function --------------------------------===//
+//
+// 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 "bitshk.h"
+#include "include/llvm-libc-macros/stdfix-macros.h" // short accum
+#include "include/llvm-libc-types/stdfix-types.h" // int_hk_t
+#include "src/__support/common.h" // LLVM_LIBC_FUNCTION
+#include "src/__support/fixed_point/fx_bits.h" // fixed_point
+#include "src/__support/macros/config.h" // LIBC_NAMESPACE_DECL
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(int_hk_t, bitshk, (short accum f)) {
+ return fixed_point::bitsfx<short accum, int_hk_t>(f);
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdfix/bitshk.h b/libc/src/stdfix/bitshk.h
new file mode 100644
index 0000000000000..a1505e2e56d85
--- /dev/null
+++ b/libc/src/stdfix/bitshk.h
@@ -0,0 +1,22 @@
+//===-- Implementation header for bitshk function ---------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_STDFIX_BITSHK_H
+#define LLVM_LIBC_SRC_STDFIX_BITSHK_H
+
+#include "include/llvm-libc-macros/stdfix-macros.h" // short accum
+#include "include/llvm-libc-types/stdfix-types.h" // int_hk_t
+#include "src/__support/macros/config.h" // LIBC_NAMESPACE_DECL
+
+namespace LIBC_NAMESPACE_DECL {
+
+int_hk_t bitshk(short accum f);
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_STDFIX_BITSHK_H
diff --git a/libc/src/stdfix/bitshr.cpp b/libc/src/stdfix/bitshr.cpp
new file mode 100644
index 0000000000000..394d1f08f6ae5
--- /dev/null
+++ b/libc/src/stdfix/bitshr.cpp
@@ -0,0 +1,22 @@
+//===-- Implementation of bitshr function --------------------------------===//
+//
+// 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 "bitshr.h"
+#include "include/llvm-libc-macros/stdfix-macros.h" // short fract
+#include "include/llvm-libc-types/stdfix-types.h" // int_hr_t
+#include "src/__support/common.h" // LLVM_LIBC_FUNCTION
+#include "src/__support/fixed_point/fx_bits.h" // fixed_point
+#include "src/__support/macros/config.h" // LIBC_NAMESPACE_DECL
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(int_hr_t, bitshr, (short fract f)) {
+ return fixed_point::bitsfx<short fract, int_hr_t>(f);
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdfix/bitshr.h b/libc/src/stdfix/bitshr.h
new file mode 100644
index 0000000000000..d5b4b8f56a7e9
--- /dev/null
+++ b/libc/src/stdfix/bitshr.h
@@ -0,0 +1,22 @@
+//===-- Implementation header for bitshr function ---------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_STDFIX_BITSHR_H
+#define LLVM_LIBC_SRC_STDFIX_BITSHR_H
+
+#include "include/llvm-libc-macros/stdfix-macros.h" // short fract
+#include "include/llvm-libc-types/stdfix-types.h" // int_hr_t
+#include "src/__support/macros/config.h" // LIBC_NAMESPACE_DECL
+
+namespace LIBC_NAMESPACE_DECL {
+
+int_hr_t bitshr(short fract f);
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_STDFIX_BITSHR_H
diff --git a/libc/src/stdfix/bitsk.cpp b/libc/src/stdfix/bitsk.cpp
new file mode 100644
index 0000000000000..f8c9d77d56e9c
--- /dev/null
+++ b/libc/src/stdfix/bitsk.cpp
@@ -0,0 +1,22 @@
+//===-- Implementation for bitsk function --------------------------------===//
+//
+// 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 "bitsk.h"
+#include "include/llvm-libc-macros/stdfix-macros.h" // accum
+#include "include/llvm-libc-types/stdfix-types.h" // int_k_t
+#include "src/__support/common.h" // LLVM_LIBC_FUNCTION
+#include "src/__support/fixed_point/fx_bits.h" // fixed_point
+#include "src/__support/macros/config.h" // LIBC_NAMESPACE_DECL
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(int_k_t, bitsk, (accum f)) {
+ return fixed_point::bitsfx<accum, int_k_t>(f);
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdfix/bitsk.h b/libc/src/stdfix/bitsk.h
new file mode 100644
index 0000000000000..32d5a724dfb0b
--- /dev/null
+++ b/libc/src/stdfix/bitsk.h
@@ -0,0 +1,22 @@
+//===-- Implementation header for bitsk function ----------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_STDFIX_BITSK_H
+#define LLVM_LIBC_SRC_STDFIX_BITSK_H
+
+#include "include/llvm-libc-macros/stdfix-macros.h" // accum
+#include "include/llvm-libc-types/stdfix-types.h" // int_k_t
+#include "src/__support/macros/config.h" // LIBC_NAMESPACE_DECL
+
+namespace LIBC_NAMESPACE_DECL {
+
+int_k_t bitsk(accum f);
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_STDFIX_BITSK_H
diff --git a/libc/src/stdfix/bitslk.cpp b/libc/src/stdfix/bitslk.cpp
new file mode 100644
index 0000000000000..f4af2a8cd8b99
--- /dev/null
+++ b/libc/src/stdfix/bitslk.cpp
@@ -0,0 +1,22 @@
+//===-- Implementation for bitslk function -------------------------------===//
+//
+// 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 "bitslk.h"
+#include "include/llvm-libc-macros/stdfix-macros.h" // long accum
+#include "include/llvm-libc-types/stdfix-types.h" // int_lk_t
+#include "src/__support/common.h" // LLVM_LIBC_FUNCTION
+#include "src/__support/fixed_point/fx_bits.h" // fixed_point
+#include "src/__support/macros/config.h" // LIBC_NAMESPACE_DECL
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(int_lk_t, bitslk, (long accum f)) {
+ return fixed_point::bitsfx<long accum, int_lk_t>(f);
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdfix/bitslk.h b/libc/src/stdfix/bitslk.h
new file mode 100644
index 0000000000000..821116b9a7c1b
--- /dev/null
+++ b/libc/src/stdfix/bitslk.h
@@ -0,0 +1,22 @@
+//===-- Implementation header for bitslk function ---------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_STDFIX_BITSLK_H
+#define LLVM_LIBC_SRC_STDFIX_BITSLK_H
+
+#include "include/llvm-libc-macros/stdfix-macros.h" // long accum
+#include "include/llvm-libc-types/stdfix-types.h" // int_lk_t
+#include "src/__support/macros/config.h" // LIBC_NAMESPACE_DECL
+
+namespace LIBC_NAMESPACE_DECL {
+
+int_lk_t bitslk(long accum f);
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_STDFIX_BITSLK_H
diff --git a/libc/src/stdfix/bitslr.cpp b/libc/src/stdfix/bitslr.cpp
new file mode 100644
index 0000000000000..3b38aa21a6338
--- /dev/null
+++ b/libc/src/stdfix/bitslr.cpp
@@ -0,0 +1,22 @@
+//===-- Implementation of bitslr function --------------------------------===//
+//
+// 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 "bitslr.h"
+#include "include/llvm-libc-macros/stdfix-macros.h" // long fract
+#include "include/llvm-libc-types/stdfix-types.h" // int_lr_t
+#include "src/__support/common.h" // LLVM_LIBC_FUNCTION
+#include "src/__support/fixed_point/fx_bits.h" // fixed_point
+#include "src/__support/macros/config.h" // LIBC_NAMESPACE_DECL
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(int_lr_t, bitslr, (long fract f)) {
+ return fixed_point::bitsfx<long fract, int_lr_t>(f);
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdfix/bitslr.h b/libc/src/stdfix/bitslr.h
new file mode 100644
index 0000000000000..0cb597214f550
--- /dev/null
+++ b/libc/src/stdfix/bitslr.h
@@ -0,0 +1,22 @@
+//===-- Implementation header for bitslr function ---------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_STDFIX_BITSLR_H
+#define LLVM_LIBC_SRC_STDFIX_BITSLR_H
+
+#include "include/llvm-libc-macros/stdfix-macros.h" // long fract
+#include "include/llvm-libc-types/stdfix-types.h" // int_lr_t
+#include "src/__support/macros/config.h" // LIBC_NAMESPACE_DECL
+
+namespace LIBC_NAMESPACE_DE...
[truncated]
|
Signed-off-by: krishna2803 <[email protected]>
Signed-off-by: krishna2803 <[email protected]>
Signed-off-by: krishna2803 <[email protected]>
|
@PiJoules @lntue @nickdesaulniers Tests for A side note: I noticed that the documentation for the fxbits functions is not updated in libc/docs/headers/math/stdfix.rst, even though we have an implementation for those. This also means that the "checks" are missing from the llvm website. |
Signed-off-by: krishna2803 <[email protected]>
PiJoules
reviewed
Feb 24, 2025
| @@ -121,7 +121,7 @@ bit_and(T x, T y) { | |||
| using BitType = typename FXRep<T>::StorageType; | |||
| BitType x_bit = cpp::bit_cast<BitType>(x); | |||
| BitType y_bit = cpp::bit_cast<BitType>(y); | |||
| // For some reason, bit_cast cannot deduce BitType from the input. | |||
| // For some reason, bit_cast cannot deduce BitType T the input. | |||
There was a problem hiding this comment.
Is the from -> T here intentional?
There was a problem hiding this comment.
oh sorry, that's an artifact of me doing a find and replace of a silly variable name that i overlooked
There was a problem hiding this comment.
fixed in 6bd83934
libc/test/src/stdfix/BitsFxTest.h
Outdated
|
|
||
| // (0.6875)_10 = (0.1011)_2 | ||
| EXPECT_EQ(static_cast<XType>(11ULL << (FXRep::FRACTION_LEN - 4)), | ||
| func(0.6875)); |
There was a problem hiding this comment.
I might be overthinking this, but using 0.6875 here is a double which gets implicitly converted to some fixed point type. This could be evaluated at compile time, but I don't think the standard guarantees that a compiler always has to do it.
Kind of a nit: I think we can always force the compile-time conversion by instead assigning this to a constexpr variable of the corresponding fixed point type. This just avoids having to emit the instructions for doing the implicit conversion from double to fixed point.
Something like constexpr T special = 0.6875; should do and group it with the other special numbers above.
There was a problem hiding this comment.
fixed here:
llvm-project/libc/test/src/stdfix/BitsFxTest.h
Lines 44 to 46 in 9d0a7d2
| public: | ||
| typedef XType (*BitsFxFunc)(T); | ||
|
|
||
| void testSpecialNumbers(BitsFxFunc func) { |
There was a problem hiding this comment.
Could you also add tests for negative values for each of the special numbers when the fixed point type is signed?
There was a problem hiding this comment.
added here:
llvm-project/libc/test/src/stdfix/BitsFxTest.h
Lines 48 to 50 in 9d0a7d2
and here:
llvm-project/libc/test/src/stdfix/BitsFxTest.h
Lines 69 to 85 in 9d0a7d2
Comment on lines
+18
to
+19
| static constexpr T max = FXRep::MAX(); | ||
| static constexpr T min = FXRep::MIN(); |
There was a problem hiding this comment.
It looks like these two are unused. Could you also add tests that check against these?
libc/test/src/stdfix/BitsFxTest.h
Outdated
| // (0.42)_10 = | ||
| // (0.0110101110000101000111101011100001010001111010111000010100011110)_2 = | ||
| // (0.0x6b851eb851eb851e)_16 | ||
| static constexpr unsigned long long zero_point_forty_two = |
There was a problem hiding this comment.
Could you also add one or two special numbers greater than 1?
There was a problem hiding this comment.
in an attempt to test for 10.72 (just as an example), i tried:
// (10.72)_10 =
// (1010.101110000101000111101011100001010001111010111000010100011110)_2 =
// (a.b851eb851eb851e)_16
static constexpr unsigned long long ten_point_seven_two = 0xab851eb851eb851eULL;
static constexpr T ten_point_seven_two_t = 10.72;
if constexpr (FXRep::INTEGRAL_LEN > 0)
EXPECT_EQ(
static_cast<XType>(ten_point_seven_two >> (64 - (FXRep::VALUE_LEN))),
func(ten_point_seven_two_t)
);where FXRep::VALUE_LEN is just FXRep::INTEGRAL_LEN + FXRep::FRACTION_LEN. When I traced the definition for FXRep::INTEGRAL_LEN, I found them to be different than the spec for example for short accum the spec suggests s4.7 (ref, pg11) while the FXRep::INTEGRAL_LEN is 8 for short fract defined here:
llvm-project/libc/include/llvm-libc-macros/stdfix-macros.h
Lines 164 to 168 in dff2ca4
so the above test fails.
but when i do:
// (10.72)_10 =
// (1010.101110000101000111101011100001010001111010111000010100011110)_2 =
// (a.b851eb851eb851e)_16
static constexpr unsigned long long ten_point_seven_two = 0xab851eb851eb851eULL;
static constexpr T ten_point_seven_two_t = 10.72;
if constexpr (FXRep::INTEGRAL_LEN > 0)
EXPECT_EQ(
// just hardcoded 4 here in place of INTEGRAL_LEN
static_cast<XType>(ten_point_seven_two >> (64 - (FXRep::FRACTION_LEN + 4))),
func(ten_point_seven_two_t)
);the test passes! could you please provide some guidance on how to handle this?
There was a problem hiding this comment.
in an attempt to test for
10.72(just as an example), i tried:// (10.72)_10 = // (1010.101110000101000111101011100001010001111010111000010100011110)_2 = // (a.b851eb851eb851e)_16 static constexpr unsigned long long ten_point_seven_two = 0xab851eb851eb851eULL; static constexpr T ten_point_seven_two_t = 10.72; if constexpr (FXRep::INTEGRAL_LEN > 0) EXPECT_EQ( static_cast<XType>(ten_point_seven_two >> (64 - (FXRep::VALUE_LEN))), func(ten_point_seven_two_t) );where
FXRep::VALUE_LENis justFXRep::INTEGRAL_LEN + FXRep::FRACTION_LEN. When I traced the definition forFXRep::INTEGRAL_LEN, I found them to be different than the spec for example forshort accumthe spec suggestss4.7(ref, pg11) while theFXRep::INTEGRAL_LENis8forshort fractdefined here:llvm-project/libc/include/llvm-libc-macros/stdfix-macros.h
Lines 164 to 168 in dff2ca4
so the above test fails.
That section is for the minimal formats of each type. Any implementation can have different integral and fractional bits as long as they meet those minimum formats. There's other possible formats in A.3 Possible Data Type Implementations. Clang's default formats are the typical desktop processor formats but we shouldn't assume those will always be the formats.
but when i do:
// (10.72)_10 = // (1010.101110000101000111101011100001010001111010111000010100011110)_2 = // (a.b851eb851eb851e)_16 static constexpr unsigned long long ten_point_seven_two = 0xab851eb851eb851eULL; static constexpr T ten_point_seven_two_t = 10.72; if constexpr (FXRep::INTEGRAL_LEN > 0) EXPECT_EQ( // just hardcoded 4 here in place of INTEGRAL_LEN static_cast<XType>(ten_point_seven_two >> (64 - (FXRep::FRACTION_LEN + 4))), func(ten_point_seven_two_t) );the test passes! could you please provide some guidance on how to handle this?
Hmmm so 10.72 can't precisely fit in a fixed point number, so the actual fixed point value will be either rounded up or down to the nearest fixed point value within 1 ULP. This means for something like 10.72 as a short accum with 7 fractional bits, it's actual value will be either 10.71875 or 10.7265625. If we test against T ten_point_seven_two_t = 10.72 then we'd actually be testing against one of these 2 values. I think perhaps it would instead be better to test precise values that fit into the fixed point type rather than guessing the rounding here. That is, for a short accum use either 10.7265625 or 10.71875 rather than 10.72 directly.
Rather than using a hardcoded hex and shifting from a more precise number, I think maybe it would be easier if we tested against known values. That is, for a short accum with a value of 10.71875 and 7 fractional bits, we explicitly expect 1372.
There was a problem hiding this comment.
for checks with the other accum types, would it be okay if i use a switch statement something like:
if constexpr (FXRep::INTEGRAL_LEN > 0) {
switch (FXRep::FRACTIONAL_LEN) {
case 7:
// todo
break;
case 15:
// todo
break;
case 23:
// todo
break;
default:
break;
}
}There was a problem hiding this comment.
I think that should be fine.
libc/test/src/stdfix/BitsFxTest.h
Outdated
Comment on lines
45
to
48
| EXPECT_EQ( | ||
| static_cast<XType>(zero_point_forty_two >> (64 - FXRep::FRACTION_LEN)), | ||
| func(0.42)); | ||
| } |
There was a problem hiding this comment.
Note that if the value cannot precisely fit in the fixed point type, rounding up or down is implementation-defined. I think this check assumes we always round down. Perhaps for numbers that don't precisely fit in a fixed point type, we should check the numbers both above and below the true value.
For example, for 0.42 as a short fract which has 7 fractional bits for this implementation, we check the result of bitshr is 53 or 54.
There was a problem hiding this comment.
Actually disregard this comment, the rounding I mentioned will happen when 0.42 is passed as the argument, so the argument will have one of 2 values but the result of func could then have more than 2 possible return values. So I think rather than using values that don't fit precisely in a fixed point type, we should use precise values that do. So rather than 0.42, maybe use one of the fixed point values that is within 1 ULP of that value for that type. For example, for a short accum with 7 fractional bits, this would instead be either 0.421875 or 0.4140625.
What's the failure you're seeing for |
Signed-off-by: krishna2803 <[email protected]>
this is the exact failure for |
You might need to add |
Signed-off-by: krishna2803 <[email protected]>
Signed-off-by: krishna2803 <[email protected]>
Signed-off-by: krishna2803 <[email protected]>
Signed-off-by: krishna2803 <[email protected]>
PiJoules
reviewed
Feb 26, 2025
libc/test/src/stdfix/BitsFxTest.h
Outdated
Comment on lines
54
to
65
| switch (FXRep::FRACTION_LEN) { | ||
| case 7: | ||
| EXPECT_EQ(static_cast<XType>(1372), func(special_num_t)); | ||
| break; | ||
| case 15: | ||
| EXPECT_EQ(static_cast<XType>(351232), func(special_num_t)); | ||
| break; | ||
| case 23: | ||
| EXPECT_EQ(static_cast<XType>(89915392), func(special_num_t)); | ||
| break; | ||
| default: | ||
| break; |
There was a problem hiding this comment.
Hmm ok rather than having the switch, perhaps we could do something like have a preset number for the lowest value we know and do the shifts. That is, we have something like
constexpr size_t kMinFbits = 7;
if (max >= 11 && FXRep::FRACTION_LEN >= kMinFbits) {
constexpr XType kExpected = 1372; // 10.71875 with a scale of 7 equals this.
EXPECT_EQ(XType << (FXRep::FRACTION_LEN - kMinFbits), func(special_num_t));
}
This way we can always have these tests even if the fractional bits aren't one of 7, 15, or 23.
Sorry I probably should've said this instead of accepting the switch from your comment earlier. I just though of this now.
There was a problem hiding this comment.
updated! here:
llvm-project/libc/test/src/stdfix/BitsFxTest.h
Lines 55 to 61 in 2f9b642
libc/test/src/stdfix/BitsFxTest.h
Outdated
|
|
||
| if constexpr (FXRep::SIGN_LEN > 0 && FXRep::INTEGRAL_LEN > 0) { | ||
| if (static_cast<int>(min) <= -11) | ||
| switch (FXRep::FRACTION_LEN) { |
There was a problem hiding this comment.
Could you also apply the comment I made above to this switch?
There was a problem hiding this comment.
updated here:
llvm-project/libc/test/src/stdfix/BitsFxTest.h
Lines 63 to 70 in 2f9b642
also refactored the tests for signed accum types to share the common if for FXRep::INTEGRAL_LEN > 0
Signed-off-by: krishna2803 <[email protected]>
PiJoules
approved these changes
Feb 26, 2025
lntue
approved these changes
Feb 27, 2025
joaosaffran
pushed a commit
to joaosaffran/llvm-project
that referenced
this pull request
Mar 3, 2025
…lvm#128413) Fixes llvm#113359 --------- Signed-off-by: krishna2803 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #113359