[libc][stdfix] Implement fixed point countlsfx functions in llvm-libc
#125356
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
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-libc Author: Krishna Pandey (krishna2803) Changesfixes #113357 Patch is 47.47 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/125356.diff 49 Files Affected:
diff --git a/libc/cmake/modules/CheckCompilerFeatures.cmake b/libc/cmake/modules/CheckCompilerFeatures.cmake
index a5ea66a5935b7b..ccb0e4bfc50976 100644
--- a/libc/cmake/modules/CheckCompilerFeatures.cmake
+++ b/libc/cmake/modules/CheckCompilerFeatures.cmake
@@ -15,6 +15,7 @@ set(
"fixed_point"
"cfloat16"
"cfloat128"
+ "padding_on_unsigned_fixed_point"
)
# Making sure ALL_COMPILER_FEATURES is sorted.
@@ -64,6 +65,8 @@ foreach(feature IN LISTS ALL_COMPILER_FEATURES)
set(link_options "")
if(${feature} STREQUAL "fixed_point")
list(APPEND compile_options "-ffixed-point")
+ elseif(${feature} STREQUAL "padding_on_unsigned_fixed_point")
+ list(APPEND compile_options "-ffixed-point -Xclang=-fpadding-on-unsigned-fixed-point")
elseif(${feature} MATCHES "^builtin_" OR
${feature} STREQUAL "float16_conversion")
set(compile_options ${LIBC_COMPILE_OPTIONS_DEFAULT})
@@ -112,6 +115,8 @@ foreach(feature IN LISTS ALL_COMPILER_FEATURES)
set(LIBC_TYPES_HAS_FLOAT128 TRUE)
elseif(${feature} STREQUAL "fixed_point")
set(LIBC_COMPILER_HAS_FIXED_POINT TRUE)
+ elseif(${feature} STREQUAL "padding_on_unsigned_fixed_point")
+ set(LIBC_COMPILER_HAS_PADDING_ON_UNSIGNED_FIXED_POINT TRUE)
elseif(${feature} STREQUAL "cfloat16")
set(LIBC_TYPES_HAS_CFLOAT16 TRUE)
elseif(${feature} STREQUAL "cfloat128")
diff --git a/libc/cmake/modules/compiler_features/check_padding_on_unsigned_fixed_point.cpp b/libc/cmake/modules/compiler_features/check_padding_on_unsigned_fixed_point.cpp
new file mode 100644
index 00000000000000..40b4da8c79621d
--- /dev/null
+++ b/libc/cmake/modules/compiler_features/check_padding_on_unsigned_fixed_point.cpp
@@ -0,0 +1,5 @@
+#include "include/llvm-libc-macros/stdfix-macros.h"
+
+#ifndef LIBC_COMPILER_HAS_PADDING_ON_UNSIGNED_FIXED_POINT
+#error unsupported
+#endif
diff --git a/libc/config/baremetal/arm/entrypoints.txt b/libc/config/baremetal/arm/entrypoints.txt
index 694cd7b1993ca2..351f727389e3ab 100644
--- a/libc/config/baremetal/arm/entrypoints.txt
+++ b/libc/config/baremetal/arm/entrypoints.txt
@@ -469,6 +469,18 @@ if(LIBC_COMPILER_HAS_FIXED_POINT)
libc.src.stdfix.ukbits
libc.src.stdfix.lkbits
libc.src.stdfix.ulkbits
+ libc.src.stdfix.countlshr
+ libc.src.stdfix.countlsr
+ libc.src.stdfix.countlslr
+ libc.src.stdfix.countlshk
+ libc.src.stdfix.countlsk
+ libc.src.stdfix.countlslk
+ libc.src.stdfix.countlsuhr
+ libc.src.stdfix.countlsur
+ libc.src.stdfix.countlsulr
+ libc.src.stdfix.countlsuhk
+ libc.src.stdfix.countlsuk
+ libc.src.stdfix.countlsulk
)
endif()
diff --git a/libc/config/baremetal/riscv/entrypoints.txt b/libc/config/baremetal/riscv/entrypoints.txt
index 667ab40dca9998..39c70a22a21e0e 100644
--- a/libc/config/baremetal/riscv/entrypoints.txt
+++ b/libc/config/baremetal/riscv/entrypoints.txt
@@ -464,6 +464,18 @@ if(LIBC_COMPILER_HAS_FIXED_POINT)
libc.src.stdfix.ukbits
libc.src.stdfix.lkbits
libc.src.stdfix.ulkbits
+ libc.src.stdfix.countlshr
+ libc.src.stdfix.countlsr
+ libc.src.stdfix.countlslr
+ libc.src.stdfix.countlshk
+ libc.src.stdfix.countlsk
+ libc.src.stdfix.countlslk
+ libc.src.stdfix.countlsuhr
+ libc.src.stdfix.countlsur
+ libc.src.stdfix.countlsulr
+ libc.src.stdfix.countlsuhk
+ libc.src.stdfix.countlsuk
+ libc.src.stdfix.countlsulk
)
endif()
diff --git a/libc/config/linux/riscv/entrypoints.txt b/libc/config/linux/riscv/entrypoints.txt
index 14a05a2f3fbf2a..cb68fb077a869c 100644
--- a/libc/config/linux/riscv/entrypoints.txt
+++ b/libc/config/linux/riscv/entrypoints.txt
@@ -744,6 +744,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.countlshr
+ libc.src.stdfix.countlsr
+ libc.src.stdfix.countlslr
+ libc.src.stdfix.countlshk
+ libc.src.stdfix.countlsk
+ libc.src.stdfix.countlslk
+ libc.src.stdfix.countlsuhr
+ libc.src.stdfix.countlsur
+ libc.src.stdfix.countlsulr
+ libc.src.stdfix.countlsuhk
+ libc.src.stdfix.countlsuk
+ libc.src.stdfix.countlsulk
)
endif()
diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index 366e4d34294d15..ea276f3c963867 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -869,6 +869,18 @@ if(LIBC_COMPILER_HAS_FIXED_POINT)
libc.src.stdfix.ukbits
libc.src.stdfix.lkbits
libc.src.stdfix.ulkbits
+ libc.src.stdfix.countlshr
+ libc.src.stdfix.countlsr
+ libc.src.stdfix.countlslr
+ libc.src.stdfix.countlshk
+ libc.src.stdfix.countlsk
+ libc.src.stdfix.countlslk
+ libc.src.stdfix.countlsuhr
+ libc.src.stdfix.countlsur
+ libc.src.stdfix.countlsulr
+ libc.src.stdfix.countlsuhk
+ libc.src.stdfix.countlsuk
+ libc.src.stdfix.countlsulk
)
endif()
diff --git a/libc/docs/headers/math/stdfix.rst b/libc/docs/headers/math/stdfix.rst
index 58052f000995cd..4507f2b608bf1f 100644
--- a/libc/docs/headers/math/stdfix.rst
+++ b/libc/docs/headers/math/stdfix.rst
@@ -73,7 +73,7 @@ The following functions are included in the ISO/IEC TR 18037:2008 standard.
+---------------+----------------+-------------+---------------+------------+----------------+-------------+----------------+-------------+---------------+------------+----------------+-------------+
| \*bits | | | | | | | | | | | | |
+---------------+----------------+-------------+---------------+------------+----------------+-------------+----------------+-------------+---------------+------------+----------------+-------------+
-| countls | | | | | | | | | | | | |
+| countls | |check| | |check| | |check| | |check| | |check| | |check| | |check| | |check| | |check| | |check| | |check| | |check| |
+---------------+----------------+-------------+---------------+------------+----------------+-------------+----------------+-------------+---------------+------------+----------------+-------------+
| divi | | | | | | | | | | | | |
+---------------+----------------+-------------+---------------+------------+----------------+-------------+----------------+-------------+---------------+------------+----------------+-------------+
diff --git a/libc/include/stdfix.yaml b/libc/include/stdfix.yaml
index 9663ac0c7df4dc..0abf2f3a9b3b6d 100644
--- a/libc/include/stdfix.yaml
+++ b/libc/include/stdfix.yaml
@@ -306,3 +306,87 @@ functions:
arguments:
- type: unsigned int
guard: LIBC_COMPILER_HAS_FIXED_POINT
+ - name: countlshr
+ standards:
+ - stdc_ext
+ return_type: int
+ arguments:
+ - type: short fract
+ guard: LIBC_COMPILER_HAS_FIXED_POINT
+ - name: countlsr
+ standards:
+ - stdc_ext
+ return_type: int
+ arguments:
+ - type: fract
+ guard: LIBC_COMPILER_HAS_FIXED_POINT
+ - name: countlslr
+ standards:
+ - stdc_ext
+ return_type: int
+ arguments:
+ - type: long fract
+ guard: LIBC_COMPILER_HAS_FIXED_POINT
+ - name: countlshk
+ standards:
+ - stdc_ext
+ return_type: int
+ arguments:
+ - type: short accum
+ guard: LIBC_COMPILER_HAS_FIXED_POINT
+ - name: countlsk
+ standards:
+ - stdc_ext
+ return_type: int
+ arguments:
+ - type: accum
+ guard: LIBC_COMPILER_HAS_FIXED_POINT
+ - name: countlslk
+ standards:
+ - stdc_ext
+ return_type: int
+ arguments:
+ - type: long accum
+ guard: LIBC_COMPILER_HAS_FIXED_POINT
+ - name: countlsuhr
+ standards:
+ - stdc_ext
+ return_type: int
+ arguments:
+ - type: unsigned short fract
+ guard: LIBC_COMPILER_HAS_FIXED_POINT
+ - name: countlsur
+ standards:
+ - stdc_ext
+ return_type: int
+ arguments:
+ - type: unsigned fract
+ guard: LIBC_COMPILER_HAS_FIXED_POINT
+ - name: countlsulr
+ standards:
+ - stdc_ext
+ return_type: int
+ arguments:
+ - type: unsigned long fract
+ guard: LIBC_COMPILER_HAS_FIXED_POINT
+ - name: countlsuhk
+ standards:
+ - stdc_ext
+ return_type: int
+ arguments:
+ - type: unsigned short accum
+ guard: LIBC_COMPILER_HAS_FIXED_POINT
+ - name: countlsuk
+ standards:
+ - stdc_ext
+ return_type: int
+ arguments:
+ - type: unsigned accum
+ guard: LIBC_COMPILER_HAS_FIXED_POINT
+ - name: countlsulk
+ standards:
+ - stdc_ext
+ return_type: int
+ arguments:
+ - type: unsigned long accum
+ guard: LIBC_COMPILER_HAS_FIXED_POINT
diff --git a/libc/src/__support/fixed_point/CMakeLists.txt b/libc/src/__support/fixed_point/CMakeLists.txt
index 3b744081765e4f..b415e2c00c488c 100644
--- a/libc/src/__support/fixed_point/CMakeLists.txt
+++ b/libc/src/__support/fixed_point/CMakeLists.txt
@@ -19,6 +19,7 @@ add_header_library(
libc.src.__support.macros.optimization
libc.src.__support.CPP.type_traits
libc.src.__support.CPP.bit
+ libc.src.__support.CPP.limits
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 225ea417760a03..a955ee67745c25 100644
--- a/libc/src/__support/fixed_point/fx_bits.h
+++ b/libc/src/__support/fixed_point/fx_bits.h
@@ -12,8 +12,9 @@
#include "include/llvm-libc-macros/stdfix-macros.h"
#include "src/__support/CPP/bit.h"
#include "src/__support/CPP/type_traits.h"
+#include "src/__support/CPP/limits.h" // numeric_limits
#include "src/__support/macros/attributes.h" // LIBC_INLINE
-#include "src/__support/macros/config.h"
+#include "src/__support/macros/config.h" // LIBC_NAMESPACE_DECL
#include "src/__support/macros/optimization.h" // LIBC_UNLIKELY
#include "src/__support/math_extras.h"
@@ -50,6 +51,12 @@ template <typename T> struct FXBits {
static constexpr StorageType SIGN_MASK =
(fx_rep::SIGN_LEN == 0 ? 0 : StorageType(1) << SIGN_OFFSET);
+ // mask for <integral | fraction>
+ static constexpr StorageType VALUE_MASK = INTEGRAL_MASK | FRACTION_MASK;
+
+ // mask for <sign | integral | fraction>
+ static constexpr StorageType TOTAL_MASK = SIGN_MASK | VALUE_MASK;
+
public:
LIBC_INLINE constexpr FXBits() = default;
@@ -74,6 +81,12 @@ template <typename T> struct FXBits {
return (value & INTEGRAL_MASK) >> INTEGRAL_OFFSET;
}
+ // returns complete bitstring representation the fixed point number
+ // the bitstring is of the form: padding | sign | integral | fraction
+ LIBC_INLINE constexpr StorageType get_bits() {
+ return (value & TOTAL_MASK) >> FRACTION_OFFSET;
+ }
+
// TODO: replace bool with Sign
LIBC_INLINE constexpr bool get_sign() {
return static_cast<bool>((value & SIGN_MASK) >> SIGN_OFFSET);
@@ -163,6 +176,26 @@ template <typename T> LIBC_INLINE constexpr T round(T x, int n) {
return bit_and((x + round_bit), rounding_mask);
}
+// count leading zeros
+template <typename T>
+LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_fixed_point_v<T>, int>
+countls(T f) {
+ using FXRep = FXRep<T>;
+ using BitType = typename FXRep::StorageType;
+ using FXBits = FXBits<T>;
+
+ constexpr int CONTAIN_LEN = cpp::numeric_limits<BitType>::digits;
+ constexpr int PADDING_LEN = CONTAIN_LEN - (FXRep::INTEGRAL_LEN + FXRep::FRACTION_LEN);
+
+ if constexpr (FXRep::SIGN_LEN != 0) {
+ if (x < 0)
+ x = bit_not(x);
+ }
+
+ BitType value_bits = FXBits(x)::get_bits();
+ return cpp::countl_zero(value_bits) - PADDING_LEN;
+}
+
} // namespace fixed_point
} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdfix/CMakeLists.txt b/libc/src/stdfix/CMakeLists.txt
index 815f739d23efa4..ac5b31320a1050 100644
--- a/libc/src/stdfix/CMakeLists.txt
+++ b/libc/src/stdfix/CMakeLists.txt
@@ -104,3 +104,33 @@ add_entrypoint_object(
libc.src.__support.fixed_point.fx_rep
libc.src.__support.CPP.bit
)
+
+
+foreach(suffix IN ITEMS hr r lr hk k lk uhr ur ulr uhk uk ulk)
+ add_entrypoint_object(
+ countls${suffix}
+ HDRS
+ countls${suffix}.h
+ SRCS
+ countls${suffix}.cpp
+ COMPILE_OPTIONS
+ ${libc_opt_high_flag}
+ DEPENDS
+ libc.src.__support.fixed_point.fx_bits
+ )
+ if(LIBC_COMPILER_HAS_PADDING_ON_UNSIGNED_FIXED_POINT)
+ add_entrypoint_object(
+ countls${suffix}_padding_on_unsigned_fixed_point
+ HDRS
+ countls${suffix}.h
+ SRCS
+ countls${suffix}.cpp
+ COMPILE_OPTIONS
+ ${libc_opt_high_flag}
+ -Xclang=-fpadding-on-unsigned-fixed-point
+ DEPENDS
+ libc.src.__support.fixed_point.fx_bits
+ )
+ endif()
+
+endforeach()
diff --git a/libc/src/stdfix/countlshk.cpp b/libc/src/stdfix/countlshk.cpp
new file mode 100644
index 00000000000000..65230dd688a7ca
--- /dev/null
+++ b/libc/src/stdfix/countlshk.cpp
@@ -0,0 +1,20 @@
+//===-- Implementation for countlshk 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 "countlshk.h"
+#include "src/__support/common.h"
+#include "src/__support/fixed_point/fx_bits.h"
+#include "src/__support/macros/config.h" // LIBC_NAMESPACE_DECL
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(int, countlshk, (short accum f)) {
+ return fixed_point::countls(f);
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdfix/countlshk.h b/libc/src/stdfix/countlshk.h
new file mode 100644
index 00000000000000..ecbc9075dcedf5
--- /dev/null
+++ b/libc/src/stdfix/countlshk.h
@@ -0,0 +1,21 @@
+//===-- Implementation header for countlshk 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_COUNTLSHK_H
+#define LLVM_LIBC_SRC_STDFIX_COUNTLSHK_H
+
+#include "include/llvm-libc-macros/stdfix-macros.h"
+#include "src/__support/macros/config.h" // LIBC_NAMESPACE_DECL
+
+namespace LIBC_NAMESPACE_DECL {
+
+int countlshk(short accum f);
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_STDFIX_COUNTLSHK_H
diff --git a/libc/src/stdfix/countlshr.cpp b/libc/src/stdfix/countlshr.cpp
new file mode 100644
index 00000000000000..d07bab8c4a1cd5
--- /dev/null
+++ b/libc/src/stdfix/countlshr.cpp
@@ -0,0 +1,20 @@
+//===-- Implementation for countlshr 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 "countlshr.h"
+#include "src/__support/common.h"
+#include "src/__support/fixed_point/fx_bits.h"
+#include "src/__support/macros/config.h" // LIBC_NAMESPACE_DECL
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(int, countlshr, (short fract f)) {
+ return fixed_point::countls(f);
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdfix/countlshr.h b/libc/src/stdfix/countlshr.h
new file mode 100644
index 00000000000000..728b6873159184
--- /dev/null
+++ b/libc/src/stdfix/countlshr.h
@@ -0,0 +1,21 @@
+//===-- Implementation header for countlshr 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_COUNTLSHR_H
+#define LLVM_LIBC_SRC_STDFIX_COUNTLSHR_H
+
+#include "include/llvm-libc-macros/stdfix-macros.h"
+#include "src/__support/macros/config.h" // LIBC_NAMESPACE_DECL
+
+namespace LIBC_NAMESPACE_DECL {
+
+int countlshr(short fract f);
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_STDFIX_COUNTLSHR_H
diff --git a/libc/src/stdfix/countlsk.cpp b/libc/src/stdfix/countlsk.cpp
new file mode 100644
index 00000000000000..87c009aba0cb5b
--- /dev/null
+++ b/libc/src/stdfix/countlsk.cpp
@@ -0,0 +1,20 @@
+//===-- Implementation for countlsk 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 "countlsk.h"
+#include "src/__support/common.h"
+#include "src/__support/fixed_point/fx_bits.h"
+#include "src/__support/macros/config.h" // LIBC_NAMESPACE_DECL
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(int, countlsk, (accum f)) {
+ return fixed_point::countls(f);
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdfix/countlsk.h b/libc/src/stdfix/countlsk.h
new file mode 100644
index 00000000000000..b7012da95a9c92
--- /dev/null
+++ b/libc/src/stdfix/countlsk.h
@@ -0,0 +1,21 @@
+//===-- Implementation header for countlsk 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_COUNTLSK_H
+#define LLVM_LIBC_SRC_STDFIX_COUNTLSK_H
+
+#include "include/llvm-libc-macros/stdfix-macros.h"
+#include "src/__support/macros/config.h" // LIBC_NAMESPACE_DECL
+
+namespace LIBC_NAMESPACE_DECL {
+
+int countlsk(accum f);
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_STDFIX_COUNTLSK_H
diff --git a/libc/src/stdfix/countlslk.cpp b/libc/src/stdfix/countlslk.cpp
new file mode 100644
index 00000000000000..16120b2f55a11d
--- /dev/null
+++ b/libc/src/stdfix/countlslk.cpp
@@ -0,0 +1,20 @@
+//===-- Implementation for countlslk 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 "countlslk.h"
+#include "src/__support/common.h"
+#include "src/__support/fixed_point/fx_bits.h"
+#include "src/__support/macros/config.h" // LIBC_NAMESPACE_DECL
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(int, countlslk, (long accum f)) {
+ return fixed_point::countls(f);
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdfix/countlslk.h b/libc/src/stdfix/countlslk.h
new file mode 100644
index 00000000000000..90a2ccc33ce641
--- /dev/null
+++ b/libc/src/stdfix/countlslk.h
@@ -0,0 +1,21 @@
+//===-- Implementation header for countlslk 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_COUNTLSLK_H
+#define LLVM_LIBC_SRC_STDFIX_COUNTLSLK_H
+
+#include "include/llvm-libc-macros/stdfix-macros.h"
+#include "src/__support/macros/config.h" // LIBC_NAMESPACE_DECL
+
+namespace LIBC_NAMESPACE_DECL {
+
+int countlslk(long accum f);
+
+} // namespace...
[truncated]
|
jhuber6
reviewed
Feb 1, 2025
| @@ -64,6 +65,8 @@ foreach(feature IN LISTS ALL_COMPILER_FEATURES) | |||
| set(link_options "") | |||
| if(${feature} STREQUAL "fixed_point") | |||
| list(APPEND compile_options "-ffixed-point") | |||
| elseif(${feature} STREQUAL "padding_on_unsigned_fixed_point") | |||
| list(APPEND compile_options "-ffixed-point -Xclang=-fpadding-on-unsigned-fixed-point") | |||
There was a problem hiding this comment.
We need to be able to support GCC right? And in general, using -Xclang options is a little dubious.
There was a problem hiding this comment.
hi! i'm having a tough time finding an alternative that would support GCC too, can you please provide some guidance?
There was a problem hiding this comment.
This option confuses me, it's a language option but it's not enabled by anything but this -cc1 option. Forcing the user to go through -Xclang for anything but hacking on the compiler internals is generally bad. My assumption is that this flag should be promoted to a marshalling driver flag. I don't know what the equivalent in GCC would be, nor why this flag is necessary.
There was a problem hiding this comment.
Currently fixed point is only supported in clang.
There was a problem hiding this comment.
I would also highly recommend AVOIDING dependencies on clang or llvm internal flags.
There was a problem hiding this comment.
This part is to test whether compiler accepting these flags, and both are recently added to clang. @PiJoules might know more about them.
My only concern is that maybe we should only test -Xclang=-fpadding-on-unsigned-fixed-point flag if -ffixed-point is supported.
There was a problem hiding this comment.
Yeah the original intent I had was having a parallel set of tests with this -Xclang flag to ensure more correctness. -Xclang=-fpadding-on-unsigned-fixed-point changes the layout for the unsigned fixed point types but the function should be able to work correctly independent of ABI changes. It's hidden since users generally shouldn't really care about the layout. GCC AFAICT doesn't have an equivalent flag.
If it's recommended that internal llvm flags shouldn't be used, then I'm fine with not having tests with this flag. Otherwise, it I think should also be gated on clang being the compiler, -ffixed-point being suppported, and this should only be limited to tests.
There was a problem hiding this comment.
i've removed both the usage of the -Xclang=-fpadding-on-unsigned-fixed-point flag and its associated tests, while retaining support for the underlying functionality for the same in libc/src/__support/fixed_point/fx_bits.h in the fixed_point::countls function. Perhaps we can implement that feature (and tests) in another dedicated PR?
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
lntue
reviewed
Feb 4, 2025
libc/test/src/stdfix/CountlsTest.h
Outdated
| #define LIST_COUNTLS_TESTS(T, func) \ | ||
| using LlvmLibcCountlsTest = CountlsTest<T>; \ | ||
| TEST_F(LlvmLibcCountlsTest, SpecialNumbers) { testSpecialNumbers(&func); } \ | ||
| static_assert(true, "Require semicolon.") |
There was a problem hiding this comment.
Add an extra line at the end of this and several other files.
Also fix the clang-format.
There was a problem hiding this comment.
Resolved in b146648a
PiJoules
approved these changes
Feb 4, 2025
| using FXBits = FXBits<T>; | ||
|
|
||
| constexpr int CONTAIN_LEN = cpp::numeric_limits<BitType>::digits; | ||
| constexpr int PADDING_LEN = CONTAIN_LEN - (FXRep::INTEGRAL_LEN + FXRep::FRACTION_LEN); |
There was a problem hiding this comment.
Note this should also account for the SIGN_LEN. Right now, this would treat the sign bit as a padding bit, which just so happens to be subtracted from the final calculation so the result of countls is still correct.
I think you could instead use FXRep::TOTAL_LEN here which should account for all the value bits.
There was a problem hiding this comment.
fixed in 9b499bcb
PiJoules
reviewed
Feb 5, 2025
| @@ -163,6 +176,26 @@ template <typename T> LIBC_INLINE constexpr T round(T x, int n) { | |||
| return bit_and((x + round_bit), rounding_mask); | |||
| } | |||
|
|
|||
| // count leading zeros | |||
There was a problem hiding this comment.
It's probably more appropriate to say count leading sign bits to account for negative values
There was a problem hiding this comment.
Resolved in 475945d9
lntue
reviewed
Feb 5, 2025
| @@ -15,6 +15,7 @@ set( | |||
| "fixed_point" | |||
| "cfloat16" | |||
| "cfloat128" | |||
| "padding_on_unsigned_fixed_point" | |||
There was a problem hiding this comment.
Can we remove changes on this file and check_padding_on_unsigned_fixed_point.cpp and leave them for other PR? They won't work as intended without changes in include/llvm-libc-macros/stdfix-macros.h.
There was a problem hiding this comment.
Resolved in 632a6d8b
lntue
requested changes
Feb 5, 2025
krishna2803
force-pushed
the
implement-countlsfx
branch
from
d087766 to
55e30af
Compare
krishna2803
force-pushed
the
implement-countlsfx
branch
from
3046da2 to
659170d
Compare
lntue
approved these changes
Feb 6, 2025
|
@krishna2803 Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/200/builds/3279 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/196/builds/4704 Here is the relevant piece of the build log for the reference |
| constexpr int PADDING_LEN = CONTAIN_LEN - FXRep::TOTAL_LEN; | ||
|
|
||
| if constexpr (FXRep::SIGN_LEN != 0) { | ||
| if (x < 0) |
There was a problem hiding this comment.
From the buildbot failures:
libc/src/__support/fixed_point/fx_bits.h:191:9: error: use of undeclared identifier 'x'
191 | if (x < 0)
| ^
| // count leading sign bits | ||
| template <typename T> | ||
| LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_fixed_point_v<T>, int> | ||
| countls(T f) { |
There was a problem hiding this comment.
libc/src/__support/fixed_point/fx_bits.h:182:11: error: unused parameter 'f' [-Werror,-Wunused-parameter]
182 | countls(T f) {
| ^
guessing x below was supposed to be f?
jhuber6
pushed a commit
that referenced
this pull request
Feb 7, 2025
Fix build-bot failure caused by #125356
jhuber6
added a commit
that referenced
this pull request
Feb 7, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/22295 Here is the relevant piece of the build log for the reference |
Icohedron
pushed a commit
to Icohedron/llvm-project
that referenced
this pull request
Feb 11, 2025
Icohedron
pushed a commit
to Icohedron/llvm-project
that referenced
this pull request
Feb 11, 2025
Fix build-bot failure caused by llvm#125356
Icohedron
pushed a commit
to Icohedron/llvm-project
that referenced
this pull request
Feb 11, 2025
…126291)" This reverts commit bada922. Revert "[libc][stdfix] Implement fixed point `countlsfx` functions in llvm-libc (llvm#125356)" This reverts commit f2a1103.
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 #113357