[libc-commits] [libc] [llvm] [libc] Add fixed point support to printf (PR #82707)
Michael Jones via libc-commits
libc-commits at lists.llvm.org
Tue Feb 27 10:31:02 PST 2024
https://github.com/michaelrj-google updated https://github.com/llvm/llvm-project/pull/82707
>From d7c1991423daf6def3bfac2c97412aeb802e2c64 Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Thu, 22 Feb 2024 15:38:59 -0800
Subject: [PATCH 1/6] [libc] Add fixed point support to printf
This patch adds the r, R, k, and K conversion specifiers to printf, with
accompanying tests. They are guarded behind the
LIBC_COPT_PRINTF_DISABLE_FIXED_POINT flag as well as automatic fixed
point support detection.
---
libc/docs/dev/printf_behavior.rst | 7 +
libc/src/stdio/printf_core/CMakeLists.txt | 1 +
libc/src/stdio/printf_core/converter.cpp | 8 +
libc/src/stdio/printf_core/converter_atlas.h | 5 +
libc/src/stdio/printf_core/converter_utils.h | 3 +
libc/src/stdio/printf_core/core_structs.h | 23 +-
libc/src/stdio/printf_core/fixed_converter.h | 386 ++++++++++++++++++
.../stdio/printf_core/float_dec_converter.h | 3 -
libc/src/stdio/printf_core/parser.h | 65 +++
libc/src/stdio/printf_core/printf_config.h | 7 +
libc/test/src/stdio/sprintf_test.cpp | 160 ++++++++
.../llvm-project-overlay/libc/BUILD.bazel | 16 +-
12 files changed, 670 insertions(+), 14 deletions(-)
create mode 100644 libc/src/stdio/printf_core/fixed_converter.h
diff --git a/libc/docs/dev/printf_behavior.rst b/libc/docs/dev/printf_behavior.rst
index bc60aa43ee2b6b..7786b27a9fdcd5 100644
--- a/libc/docs/dev/printf_behavior.rst
+++ b/libc/docs/dev/printf_behavior.rst
@@ -62,6 +62,13 @@ When set, this flag disables support for floating point numbers and all their
conversions (%a, %f, %e, %g); any floating point number conversion will be
treated as invalid. This reduces code size.
+LIBC_COPT_PRINTF_DISABLE_FIXED_POINT
+------------------------------------
+When set, this flag disables support for fixed point numbers and all their
+conversions (%r, %k); any fixed point number conversion will be treated as
+invalid. This reduces code size. This has no effect if the current compiler does
+not support fixed point numbers.
+
LIBC_COPT_PRINTF_NO_NULLPTR_CHECKS
----------------------------------
When set, this flag disables the nullptr checks in %n and %s.
diff --git a/libc/src/stdio/printf_core/CMakeLists.txt b/libc/src/stdio/printf_core/CMakeLists.txt
index 8da274395526df..cd75060db37579 100644
--- a/libc/src/stdio/printf_core/CMakeLists.txt
+++ b/libc/src/stdio/printf_core/CMakeLists.txt
@@ -76,6 +76,7 @@ add_object_library(
float_inf_nan_converter.h
float_hex_converter.h
float_dec_converter.h
+ fixed_converter.h #TODO: Check if this should be disabled when fixed unavail
DEPENDS
.writer
.core_structs
diff --git a/libc/src/stdio/printf_core/converter.cpp b/libc/src/stdio/printf_core/converter.cpp
index 52412aef3c5c15..613d693c3cfcb3 100644
--- a/libc/src/stdio/printf_core/converter.cpp
+++ b/libc/src/stdio/printf_core/converter.cpp
@@ -9,6 +9,7 @@
#include "src/stdio/printf_core/converter.h"
#include "src/stdio/printf_core/core_structs.h"
+#include "src/stdio/printf_core/printf_config.h"
#include "src/stdio/printf_core/writer.h"
// This option allows for replacing all of the conversion functions with custom
@@ -75,6 +76,13 @@ int convert(Writer *writer, const FormatSection &to_conv) {
case 'G':
return convert_float_dec_auto(writer, to_conv);
#endif // LIBC_COPT_PRINTF_DISABLE_FLOAT
+#ifdef LIBC_INTERNAL_PRINTF_HAS_FIXED_POINT
+ case 'r':
+ case 'R':
+ case 'k':
+ case 'K':
+ return convert_fixed(writer, to_conv);
+#endif // LIBC_INTERNAL_PRINTF_HAS_FIXED_POINT
#ifndef LIBC_COPT_PRINTF_DISABLE_WRITE_INT
case 'n':
return convert_write_int(writer, to_conv);
diff --git a/libc/src/stdio/printf_core/converter_atlas.h b/libc/src/stdio/printf_core/converter_atlas.h
index 6471f3f2955b75..2189ed11a551ea 100644
--- a/libc/src/stdio/printf_core/converter_atlas.h
+++ b/libc/src/stdio/printf_core/converter_atlas.h
@@ -31,6 +31,11 @@
#include "src/stdio/printf_core/float_hex_converter.h"
#endif // LIBC_COPT_PRINTF_DISABLE_FLOAT
+#ifdef LIBC_INTERNAL_PRINTF_HAS_FIXED_POINT
+// defines convert_fixed
+#include "src/stdio/printf_core/fixed_converter.h"
+#endif // LIBC_INTERNAL_PRINTF_HAS_FIXED_POINT
+
#ifndef LIBC_COPT_PRINTF_DISABLE_WRITE_INT
#include "src/stdio/printf_core/write_int_converter.h"
#endif // LIBC_COPT_PRINTF_DISABLE_WRITE_INT
diff --git a/libc/src/stdio/printf_core/converter_utils.h b/libc/src/stdio/printf_core/converter_utils.h
index 54f0a870d0ac4a..948fe816e9b76d 100644
--- a/libc/src/stdio/printf_core/converter_utils.h
+++ b/libc/src/stdio/printf_core/converter_utils.h
@@ -51,6 +51,9 @@ LIBC_INLINE uintmax_t apply_length_modifier(uintmax_t num, LengthModifier lm) {
return result; \
}
+// This is used to represent which direction the number should be rounded.
+enum class RoundDirection { Up, Down, Even };
+
} // namespace printf_core
} // namespace LIBC_NAMESPACE
diff --git a/libc/src/stdio/printf_core/core_structs.h b/libc/src/stdio/printf_core/core_structs.h
index 7634d45568ab84..681f85dd5a285d 100644
--- a/libc/src/stdio/printf_core/core_structs.h
+++ b/libc/src/stdio/printf_core/core_structs.h
@@ -10,7 +10,9 @@
#define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_CORE_STRUCTS_H
#include "src/__support/CPP/string_view.h"
+#include "src/__support/CPP/type_traits.h"
#include "src/__support/FPUtil/FPBits.h"
+#include "src/stdio/printf_core/printf_config.h"
#include <inttypes.h>
#include <stddef.h>
@@ -77,7 +79,13 @@ struct FormatSection {
}
};
-enum PrimaryType : uint8_t { Unknown = 0, Float = 1, Pointer = 2, Integer = 3 };
+enum PrimaryType : uint8_t {
+ Unknown = 0,
+ Float = 1,
+ Pointer = 2,
+ Integer = 3,
+ FixedPoint = 4,
+};
// TypeDesc stores the information about a type that is relevant to printf in
// a relatively compact manner.
@@ -95,9 +103,16 @@ template <typename T> LIBC_INLINE constexpr TypeDesc type_desc_from_type() {
} else {
constexpr bool IS_POINTER = cpp::is_pointer_v<T>;
constexpr bool IS_FLOAT = cpp::is_floating_point_v<T>;
- return TypeDesc{sizeof(T), IS_POINTER ? PrimaryType::Pointer
- : IS_FLOAT ? PrimaryType::Float
- : PrimaryType::Integer};
+#ifdef LIBC_INTERNAL_PRINTF_HAS_FIXED_POINT
+ constexpr bool IS_FIXED_POINT = cpp::is_fixed_point_v<T>;
+#else
+ constexpr bool IS_FIXED_POINT = false;
+#endif // LIBC_INTERNAL_PRINTF_HAS_FIXED_POINT
+
+ return TypeDesc{sizeof(T), IS_POINTER ? PrimaryType::Pointer
+ : IS_FLOAT ? PrimaryType::Float
+ : IS_FIXED_POINT ? PrimaryType::FixedPoint
+ : PrimaryType::Integer};
}
}
diff --git a/libc/src/stdio/printf_core/fixed_converter.h b/libc/src/stdio/printf_core/fixed_converter.h
new file mode 100644
index 00000000000000..5d521f2957a560
--- /dev/null
+++ b/libc/src/stdio/printf_core/fixed_converter.h
@@ -0,0 +1,386 @@
+//===-- Fixed Point Converter for printf ------------------------*- 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_STDIO_PRINTF_CORE_FIXED_CONVERTER_H
+#define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_FIXED_CONVERTER_H
+
+#include "include/llvm-libc-macros/stdfix-macros.h"
+#include "src/__support/CPP/string_view.h"
+#include "src/__support/fixed_point/fx_bits.h"
+#include "src/__support/fixed_point/fx_rep.h"
+#include "src/__support/integer_to_string.h"
+#include "src/__support/libc_assert.h"
+#include "src/stdio/printf_core/converter_utils.h"
+#include "src/stdio/printf_core/core_structs.h"
+#include "src/stdio/printf_core/writer.h"
+
+#include <inttypes.h>
+#include <stddef.h>
+
+namespace LIBC_NAMESPACE {
+namespace printf_core {
+
+// This is just for assertions. It will be compiled out for release builds.
+LIBC_INLINE constexpr uint32_t const_ten_exp(uint32_t exponent) {
+ uint32_t result = 1;
+ LIBC_ASSERT(exponent < 11);
+ for (size_t i = 0; i < exponent; ++i)
+ result *= 10;
+
+ return result;
+}
+
+LIBC_INLINE int convert_fixed(Writer *writer, const FormatSection &to_conv) {
+ using SAStorageType = fixed_point::FXRep<short accum>::StorageType;
+ using AStorageType = fixed_point::FXRep<accum>::StorageType;
+ using LAStorageType = fixed_point::FXRep<long accum>::StorageType;
+ using SFStorageType = fixed_point::FXRep<short fract>::StorageType;
+ using FStorageType = fixed_point::FXRep<fract>::StorageType;
+ using LFStorageType = fixed_point::FXRep<long fract>::StorageType;
+
+ // Long accum should be the largest type, so we can store all the smaller
+ // numbers in things sized for it.
+ using LARep = fixed_point::FXRep<unsigned long accum>;
+ using StorageType = LARep::StorageType;
+
+ // All of the letters will be defined relative to variable a, which will be
+ // the appropriate case based on the name of the conversion. This converts any
+ // conversion name into the letter 'a' with the appropriate case.
+ const char a = (to_conv.conv_name & 32) | 'A';
+ FormatFlags flags = to_conv.flags;
+
+ bool is_negative;
+ int exponent;
+ StorageType integral;
+ StorageType fractional;
+
+ // TODO: See about simplifying this mess of a 3D matrix if statement.
+ // r = fract
+ // k = accum
+ // lowercase = signed
+ // uppercase = unsigned
+ // h = short
+ // l = long
+ // any other length modifier has no effect
+ if (to_conv.length_modifier == LengthModifier::h) {
+ // short types
+ if (to_conv.conv_name == 'r') {
+ auto fixed_bits = fixed_point::FXBits<short fract>(
+ static_cast<SFStorageType>(to_conv.conv_val_raw));
+ integral = fixed_bits.get_integral();
+ fractional = fixed_bits.get_fraction();
+ exponent = fixed_bits.get_exponent();
+ is_negative = fixed_bits.get_sign();
+ }
+ if (to_conv.conv_name == 'R') {
+ auto fixed_bits = fixed_point::FXBits<unsigned short fract>(
+ static_cast<SFStorageType>(to_conv.conv_val_raw));
+ integral = fixed_bits.get_integral();
+ fractional = fixed_bits.get_fraction();
+ exponent = fixed_bits.get_exponent();
+ is_negative = fixed_bits.get_sign();
+ }
+ if (to_conv.conv_name == 'k') {
+ auto fixed_bits = fixed_point::FXBits<short accum>(
+ static_cast<SAStorageType>(to_conv.conv_val_raw));
+ integral = fixed_bits.get_integral();
+ fractional = fixed_bits.get_fraction();
+ exponent = fixed_bits.get_exponent();
+ is_negative = fixed_bits.get_sign();
+ }
+ if (to_conv.conv_name == 'K') {
+ auto fixed_bits = fixed_point::FXBits<unsigned short accum>(
+ static_cast<SAStorageType>(to_conv.conv_val_raw));
+ integral = fixed_bits.get_integral();
+ fractional = fixed_bits.get_fraction();
+ exponent = fixed_bits.get_exponent();
+ is_negative = fixed_bits.get_sign();
+ }
+ } else if (to_conv.length_modifier == LengthModifier::l) {
+ // long types
+ if (to_conv.conv_name == 'r') {
+ auto fixed_bits = fixed_point::FXBits<long fract>(
+ static_cast<LFStorageType>(to_conv.conv_val_raw));
+ integral = fixed_bits.get_integral();
+ fractional = fixed_bits.get_fraction();
+ exponent = fixed_bits.get_exponent();
+ is_negative = fixed_bits.get_sign();
+ }
+ if (to_conv.conv_name == 'R') {
+ auto fixed_bits = fixed_point::FXBits<unsigned long fract>(
+ static_cast<LFStorageType>(to_conv.conv_val_raw));
+ integral = fixed_bits.get_integral();
+ fractional = fixed_bits.get_fraction();
+ exponent = fixed_bits.get_exponent();
+ is_negative = fixed_bits.get_sign();
+ }
+ if (to_conv.conv_name == 'k') {
+ auto fixed_bits = fixed_point::FXBits<long accum>(
+ static_cast<LAStorageType>(to_conv.conv_val_raw));
+ integral = fixed_bits.get_integral();
+ fractional = fixed_bits.get_fraction();
+ exponent = fixed_bits.get_exponent();
+ is_negative = fixed_bits.get_sign();
+ }
+ if (to_conv.conv_name == 'K') {
+ auto fixed_bits = fixed_point::FXBits<unsigned long accum>(
+ static_cast<LAStorageType>(to_conv.conv_val_raw));
+ integral = fixed_bits.get_integral();
+ fractional = fixed_bits.get_fraction();
+ exponent = fixed_bits.get_exponent();
+ is_negative = fixed_bits.get_sign();
+ }
+ } else {
+ // unspecified types
+ if (to_conv.conv_name == 'r') {
+ auto fixed_bits = fixed_point::FXBits<fract>(
+ static_cast<FStorageType>(to_conv.conv_val_raw));
+ integral = fixed_bits.get_integral();
+ fractional = fixed_bits.get_fraction();
+ exponent = fixed_bits.get_exponent();
+ is_negative = fixed_bits.get_sign();
+ }
+ if (to_conv.conv_name == 'R') {
+ auto fixed_bits = fixed_point::FXBits<unsigned fract>(
+ static_cast<FStorageType>(to_conv.conv_val_raw));
+ integral = fixed_bits.get_integral();
+ fractional = fixed_bits.get_fraction();
+ exponent = fixed_bits.get_exponent();
+ is_negative = fixed_bits.get_sign();
+ }
+ if (to_conv.conv_name == 'k') {
+ auto fixed_bits = fixed_point::FXBits<accum>(
+ static_cast<AStorageType>(to_conv.conv_val_raw));
+ integral = fixed_bits.get_integral();
+ fractional = fixed_bits.get_fraction();
+ exponent = fixed_bits.get_exponent();
+ is_negative = fixed_bits.get_sign();
+ }
+ if (to_conv.conv_name == 'K') {
+ auto fixed_bits = fixed_point::FXBits<unsigned accum>(
+ static_cast<AStorageType>(to_conv.conv_val_raw));
+ integral = fixed_bits.get_integral();
+ fractional = fixed_bits.get_fraction();
+ exponent = fixed_bits.get_exponent();
+ is_negative = fixed_bits.get_sign();
+ }
+ }
+
+ LIBC_ASSERT(static_cast<size_t>(exponent) <=
+ (sizeof(StorageType) - sizeof(uint32_t)) * CHAR_BIT &&
+ "StorageType must be large enough to hold the fractional "
+ "component multiplied by a 32 bit number.");
+
+ char sign_char = 0;
+
+ // Check if the conv name is uppercase
+ if (a == 'A') {
+ // These flags are only for signed conversions, so this removes them if the
+ // conversion is unsigned.
+ flags = FormatFlags(flags &
+ ~(FormatFlags::FORCE_SIGN | FormatFlags::SPACE_PREFIX));
+ }
+
+ if (is_negative)
+ sign_char = '-';
+ else if ((flags & FormatFlags::FORCE_SIGN) == FormatFlags::FORCE_SIGN)
+ sign_char = '+'; // FORCE_SIGN has precedence over SPACE_PREFIX
+ else if ((flags & FormatFlags::SPACE_PREFIX) == FormatFlags::SPACE_PREFIX)
+ sign_char = ' ';
+
+ // If to_conv doesn't specify a precision, the precision defaults to 6.
+ const size_t precision = to_conv.precision < 0 ? 6 : to_conv.precision;
+ bool has_decimal_point =
+ (precision > 0) || ((flags & FormatFlags::ALTERNATE_FORM) != 0);
+
+ // The number of non-zero digits below the decimal point for a negative power
+ // of 2 in base 10 is equal to the magnitude of the power of 2.
+
+ // A quick proof:
+ // Let p be any positive integer.
+ // Let e = 2^(-p)
+ // Let t be a positive integer such that e * 10^t is an integer.
+ // By definition: The smallest allowed value of t must be equal to the number
+ // of non-zero digits below the decimal point in e.
+ // If we evaluate e * 10^t we get the following:
+ // e * 10^t = 2^(-p) * 10*t = 2^(-p) * 2^t * 5^t = 5^t * 2^(t-p)
+ // For 5^t * 2^(t-p) to be an integer, both exponents must be non-negative,
+ // since 5 and 2 are coprime.
+ // The smallest value of t such that t-p is non-negative is p.
+ // Therefor, the number of non-zero digits below the decimal point for a given
+ // negative power of 2 "p" is equal to the value of p.
+
+ constexpr size_t MAX_FRACTION_DIGITS = LARep::FRACTION_LEN;
+
+ char fraction_digits[MAX_FRACTION_DIGITS];
+
+ size_t valid_fraction_digits = 0;
+
+ // TODO: Factor this part out
+ while (fractional > 0) {
+ uint32_t cur_digits = 0;
+ // 10^9 is used since it's the largest power of 10 that fits in a uint32_t
+ constexpr uint32_t TEN_EXP_NINE = 1000000000;
+ constexpr size_t DIGITS_PER_BLOCK = 9;
+
+ // Multiply by 10^9, then grab the digits above the decimal point, then
+ // clear those digits in fractional.
+ fractional = fractional * TEN_EXP_NINE;
+ cur_digits = static_cast<uint32_t>(fractional >> exponent);
+ fractional = fractional % (StorageType(1) << exponent);
+
+ // we add TEN_EXP_NINE to force leading zeroes to show up, then we skip the
+ // first digit in the loop.
+ const IntegerToString<uint32_t> cur_fractional_digits(cur_digits +
+ TEN_EXP_NINE);
+ for (size_t i = 0;
+ i < DIGITS_PER_BLOCK && valid_fraction_digits < MAX_FRACTION_DIGITS;
+ ++i, ++valid_fraction_digits)
+ fraction_digits[valid_fraction_digits] =
+ cur_fractional_digits.view()[i + 1];
+
+ if (valid_fraction_digits >= MAX_FRACTION_DIGITS) {
+ LIBC_ASSERT(fractional == 0 && "If the fraction digit buffer is full, "
+ "there should be no remaining digits.");
+ /*
+ A visual explanation of what this assert is checking:
+
+ 32 digits (max for 32 bit fract)
+ +------------------------------++--+--- must be zero
+ | || |
+ 123456789012345678901234567890120000
+ | || || || |
+ +-------++-------++-------++-------+
+ 9 digit blocks
+ */
+ LIBC_ASSERT(cur_digits % const_ten_exp(
+ DIGITS_PER_BLOCK -
+ (MAX_FRACTION_DIGITS % DIGITS_PER_BLOCK)) ==
+ 0 &&
+ "Digits after the MAX_FRACTION_DIGITS should all be zero.");
+ valid_fraction_digits = MAX_FRACTION_DIGITS;
+ }
+ }
+
+ if (precision < valid_fraction_digits) {
+ // Handle rounding. Just do round to nearest, tie to even since it's
+ // unspecified.
+ RoundDirection round;
+ char first_digit_after = fraction_digits[precision];
+ if (first_digit_after > '5') {
+ round = RoundDirection::Up;
+ } else if (first_digit_after < '5') {
+ round = RoundDirection::Down;
+ } else {
+ // first_digit_after == '5'
+ // need to check the remaining digits, but default to even.
+ round = RoundDirection::Even;
+ for (size_t cur_digit_index = precision + 1;
+ cur_digit_index + 1 < valid_fraction_digits; ++cur_digit_index) {
+ if (fraction_digits[cur_digit_index] != '0') {
+ round = RoundDirection::Up;
+ break;
+ }
+ }
+ }
+
+ // If we need to actually perform rounding, do so.
+ if (round == RoundDirection::Up || round == RoundDirection::Even) {
+ bool keep_rounding = true;
+ int digit_to_round = static_cast<int>(precision) - 1;
+ for (; digit_to_round >= 0 && keep_rounding; --digit_to_round) {
+ keep_rounding = false;
+ char cur_digit = fraction_digits[digit_to_round];
+ // if the digit should not be rounded up
+ if (round == RoundDirection::Even && ((cur_digit - '0') % 2) == 0) {
+ // break out of the loop
+ break;
+ }
+ fraction_digits[digit_to_round] += 1;
+
+ // if the digit was a 9, instead replace with a 0.
+ if (cur_digit == '9') {
+ fraction_digits[digit_to_round] = '0';
+ keep_rounding = true;
+ }
+ }
+
+ // if every digit below the decimal point was rounded up but we need to
+ // keep rounding
+ if (keep_rounding &&
+ (round == RoundDirection::Up ||
+ (round == RoundDirection::Even && ((integral % 2) == 1)))) {
+ // add one to the integral portion to round it up.
+ ++integral;
+ }
+ }
+
+ valid_fraction_digits = precision;
+ }
+
+ const IntegerToString<StorageType> integral_str(integral);
+
+ // these are signed to prevent underflow due to negative values. The
+ // eventual values will always be non-negative.
+ size_t trailing_zeroes = 0;
+ int padding;
+
+ // If the precision is greater than the actual result, pad with 0s
+ if (precision > valid_fraction_digits)
+ trailing_zeroes = precision - (valid_fraction_digits);
+
+ constexpr cpp::string_view DECIMAL_POINT(".");
+
+ padding = static_cast<int>(to_conv.min_width - (sign_char > 0 ? 1 : 0) -
+ integral_str.size() -
+ static_cast<int>(has_decimal_point) -
+ valid_fraction_digits - trailing_zeroes);
+ if (padding < 0)
+ padding = 0;
+
+ if ((flags & FormatFlags::LEFT_JUSTIFIED) == FormatFlags::LEFT_JUSTIFIED) {
+ // The pattern is (sign), integral, (.), (fraction), (zeroes), (spaces)
+ if (sign_char > 0)
+ RET_IF_RESULT_NEGATIVE(writer->write(sign_char));
+ RET_IF_RESULT_NEGATIVE(writer->write(integral_str.view()));
+ if (has_decimal_point)
+ RET_IF_RESULT_NEGATIVE(writer->write(DECIMAL_POINT));
+ if (valid_fraction_digits > 0)
+ RET_IF_RESULT_NEGATIVE(
+ writer->write({fraction_digits, valid_fraction_digits}));
+ if (trailing_zeroes > 0)
+ RET_IF_RESULT_NEGATIVE(writer->write('0', trailing_zeroes));
+ if (padding > 0)
+ RET_IF_RESULT_NEGATIVE(writer->write(' ', padding));
+ } else {
+ // The pattern is (spaces), (sign), (zeroes), integral, (.), (fraction),
+ // (zeroes)
+ if ((padding > 0) &&
+ ((flags & FormatFlags::LEADING_ZEROES) != FormatFlags::LEADING_ZEROES))
+ RET_IF_RESULT_NEGATIVE(writer->write(' ', padding));
+ if (sign_char > 0)
+ RET_IF_RESULT_NEGATIVE(writer->write(sign_char));
+ if ((padding > 0) &&
+ ((flags & FormatFlags::LEADING_ZEROES) == FormatFlags::LEADING_ZEROES))
+ RET_IF_RESULT_NEGATIVE(writer->write('0', padding));
+ RET_IF_RESULT_NEGATIVE(writer->write(integral_str.view()));
+ if (has_decimal_point)
+ RET_IF_RESULT_NEGATIVE(writer->write(DECIMAL_POINT));
+ if (valid_fraction_digits > 0)
+ RET_IF_RESULT_NEGATIVE(
+ writer->write({fraction_digits, valid_fraction_digits}));
+ if (trailing_zeroes > 0)
+ RET_IF_RESULT_NEGATIVE(writer->write('0', trailing_zeroes));
+ }
+ return WRITE_OK;
+}
+
+} // namespace printf_core
+} // namespace LIBC_NAMESPACE
+
+#endif // LLVM_LIBC_SRC_STDIO_PRINTF_CORE_FIXED_CONVERTER_H
diff --git a/libc/src/stdio/printf_core/float_dec_converter.h b/libc/src/stdio/printf_core/float_dec_converter.h
index b54526d3710860..a6c68329e66023 100644
--- a/libc/src/stdio/printf_core/float_dec_converter.h
+++ b/libc/src/stdio/printf_core/float_dec_converter.h
@@ -45,9 +45,6 @@ constexpr uint32_t MAX_BLOCK = 999999999;
// constexpr uint32_t MAX_BLOCK = 999999999999999999;
constexpr char DECIMAL_POINT = '.';
-// This is used to represent which direction the number should be rounded.
-enum class RoundDirection { Up, Down, Even };
-
LIBC_INLINE RoundDirection get_round_direction(int last_digit, bool truncated,
fputil::Sign sign) {
switch (fputil::quick_get_round()) {
diff --git a/libc/src/stdio/printf_core/parser.h b/libc/src/stdio/printf_core/parser.h
index 1e7d2e58c924a6..0876116a0bac86 100644
--- a/libc/src/stdio/printf_core/parser.h
+++ b/libc/src/stdio/printf_core/parser.h
@@ -9,13 +9,19 @@
#ifndef LLVM_LIBC_SRC_STDIO_PRINTF_CORE_PARSER_H
#define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_PARSER_H
+#include "include/llvm-libc-macros/stdfix-macros.h"
#include "src/__support/CPP/optional.h"
+#include "src/__support/CPP/type_traits.h"
#include "src/__support/str_to_integer.h"
#include "src/stdio/printf_core/core_structs.h"
#include "src/stdio/printf_core/printf_config.h"
#include <stddef.h>
+#ifdef LIBC_INTERNAL_PRINTF_HAS_FIXED_POINT
+#include "src/__support/fixed_point/fx_rep.h"
+#endif // LIBC_INTERNAL_PRINTF_HAS_FIXED_POINT
+
namespace LIBC_NAMESPACE {
namespace printf_core {
@@ -28,6 +34,14 @@ template <> struct int_type_of<double> {
template <> struct int_type_of<long double> {
using type = fputil::FPBits<long double>::StorageType;
};
+
+#ifdef LIBC_INTERNAL_PRINTF_HAS_FIXED_POINT
+template <typename T>
+struct int_type_of<cpp::enable_if<cpp::is_fixed_point_v<T>, T>> {
+ using type = typename fixed_point::FXRep<T>::StorageType;
+};
+#endif // LIBC_INTERNAL_PRINTF_HAS_FIXED_POINT
+
template <typename T> using int_type_of_v = typename int_type_of<T>::type;
#ifndef LIBC_COPT_PRINTF_DISABLE_INDEX_MODE
@@ -206,6 +220,25 @@ template <typename ArgProvider> class Parser {
}
break;
#endif // LIBC_COPT_PRINTF_DISABLE_FLOAT
+#ifdef LIBC_INTERNAL_PRINTF_HAS_FIXED_POINT
+ // Capitalization represents sign, but we only need to get the right
+ // bitwidth here so we ignore that.
+ case ('r'):
+ case ('R'):
+ // all fract sizes we support are less than 32 bits, and currently doing
+ // va_args with fixed point types just doesn't work.
+ // TODO: Move to fixed point types once va_args supports it.
+ WRITE_ARG_VAL_SIMPLEST(section.conv_val_raw, uint32_t, conv_index);
+ break;
+ case ('k'):
+ case ('K'):
+ if (lm == LengthModifier::l) {
+ WRITE_ARG_VAL_SIMPLEST(section.conv_val_raw, uint64_t, conv_index);
+ } else {
+ WRITE_ARG_VAL_SIMPLEST(section.conv_val_raw, uint32_t, conv_index);
+ }
+ break;
+#endif // LIBC_INTERNAL_PRINTF_HAS_FIXED_POINT
#ifndef LIBC_COPT_PRINTF_DISABLE_WRITE_INT
case ('n'):
#endif // LIBC_COPT_PRINTF_DISABLE_WRITE_INT
@@ -399,6 +432,22 @@ template <typename ArgProvider> class Parser {
else if (cur_type_desc == type_desc_from_type<long double>())
args_cur.template next_var<long double>();
#endif // LIBC_COPT_PRINTF_DISABLE_FLOAT
+#ifdef LIBC_INTERNAL_PRINTF_HAS_FIXED_POINT
+ // Floating point numbers may be stored separately from the other
+ // arguments.
+ else if (cur_type_desc == type_desc_from_type<short fract>())
+ args_cur.template next_var<short fract>();
+ else if (cur_type_desc == type_desc_from_type<fract>())
+ args_cur.template next_var<fract>();
+ else if (cur_type_desc == type_desc_from_type<long fract>())
+ args_cur.template next_var<long fract>();
+ else if (cur_type_desc == type_desc_from_type<short accum>())
+ args_cur.template next_var<short accum>();
+ else if (cur_type_desc == type_desc_from_type<accum>())
+ args_cur.template next_var<accum>();
+ else if (cur_type_desc == type_desc_from_type<long accum>())
+ args_cur.template next_var<long accum>();
+#endif // LIBC_INTERNAL_PRINTF_HAS_FIXED_POINT
// pointers may be stored separately from normal values.
else if (cur_type_desc == type_desc_from_type<void *>())
args_cur.template next_var<void *>();
@@ -528,6 +577,22 @@ template <typename ArgProvider> class Parser {
conv_size = type_desc_from_type<long double>();
break;
#endif // LIBC_COPT_PRINTF_DISABLE_FLOAT
+#ifdef LIBC_INTERNAL_PRINTF_HAS_FIXED_POINT
+ // Capitalization represents sign, but we only need to get the right
+ // bitwidth here so we ignore that.
+ case ('r'):
+ case ('R'):
+ conv_size = type_desc_from_type<uint32_t>();
+ break;
+ case ('k'):
+ case ('K'):
+ if (lm == LengthModifier::l) {
+ conv_size = type_desc_from_type<uint64_t>();
+ } else {
+ conv_size = type_desc_from_type<uint32_t>();
+ }
+ break;
+#endif // LIBC_INTERNAL_PRINTF_HAS_FIXED_POINT
#ifndef LIBC_COPT_PRINTF_DISABLE_WRITE_INT
case ('n'):
#endif // LIBC_COPT_PRINTF_DISABLE_WRITE_INT
diff --git a/libc/src/stdio/printf_core/printf_config.h b/libc/src/stdio/printf_core/printf_config.h
index e1d9654f3affe4..8a48abdd170eca 100644
--- a/libc/src/stdio/printf_core/printf_config.h
+++ b/libc/src/stdio/printf_core/printf_config.h
@@ -29,6 +29,13 @@
#define LIBC_COPT_PRINTF_INDEX_ARR_LEN 128
#endif
+// If fixed point is available and the user hasn't explicitly opted out, then
+// enable fixed point.
+#if defined(LIBC_COMPILER_HAS_FIXED_POINT) && \
+ !defined(LIBC_COPT_PRINTF_DISABLE_FIXED_POINT)
+#define LIBC_INTERNAL_PRINTF_HAS_FIXED_POINT
+#endif
+
// TODO(michaelrj): Provide a proper interface for these options.
// LIBC_COPT_FLOAT_TO_STR_USE_MEGA_LONG_DOUBLE_TABLE
// LIBC_COPT_FLOAT_TO_STR_USE_DYADIC_FLOAT
diff --git a/libc/test/src/stdio/sprintf_test.cpp b/libc/test/src/stdio/sprintf_test.cpp
index 186b37e2898af6..50a7d8a195a40c 100644
--- a/libc/test/src/stdio/sprintf_test.cpp
+++ b/libc/test/src/stdio/sprintf_test.cpp
@@ -3201,6 +3201,166 @@ TEST_F(LlvmLibcSPrintfTest, FloatAutoLongDoubleConv) {
#endif // LIBC_COPT_PRINTF_DISABLE_FLOAT
+#if defined(LIBC_COMPILER_HAS_FIXED_POINT) && \
+ !defined(LIBC_COPT_PRINTF_DISABLE_FIXED_POINT)
+TEST_F(LlvmLibcSPrintfTest, FixedConv) {
+
+ // These numeric tests are potentially a little weak, but the fuzz test is
+ // more thorough than my handwritten tests tend to be.
+
+ // TODO: Commit the fuzzer
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%k", 0x0);
+ ASSERT_STREQ_LEN(written, buff, "0.000000");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%k", 0x80000000);
+ ASSERT_STREQ_LEN(written, buff, "-0.000000");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%r", 0xffff);
+ ASSERT_STREQ_LEN(written, buff, "-0.999969");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%R", 0xffff);
+ ASSERT_STREQ_LEN(written, buff, "0.999985");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%k", 0xffffffff);
+ ASSERT_STREQ_LEN(written, buff, "-65535.999969");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%K", 0xffffffff);
+ ASSERT_STREQ_LEN(written, buff, "65535.999985");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%r", 0x7fff);
+ ASSERT_STREQ_LEN(written, buff, "0.999969");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%k", 0x7fffffff);
+ ASSERT_STREQ_LEN(written, buff, "65535.999969");
+
+ // Length Modifier Tests.
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%hk", 0x0);
+ ASSERT_STREQ_LEN(written, buff, "0.000000");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%hk", 0xffff);
+ ASSERT_STREQ_LEN(written, buff, "-255.992188");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%lk", 0x0);
+ ASSERT_STREQ_LEN(written, buff, "0.000000");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%lk", 0xffffffffffffffff);
+ ASSERT_STREQ_LEN(written, buff, "-4294967296.000000");
+
+ // Min Width Tests.
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%10k", 0x0000a000);
+ ASSERT_STREQ_LEN(written, buff, " 1.250000");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%10k", 0x8000a000);
+ ASSERT_STREQ_LEN(written, buff, " -1.250000");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%8k", 0x0000a000);
+ ASSERT_STREQ_LEN(written, buff, "1.250000");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%9k", 0x8000a000);
+ ASSERT_STREQ_LEN(written, buff, "-1.250000");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%4k", 0x0000a000);
+ ASSERT_STREQ_LEN(written, buff, "1.250000");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%4k", 0x8000a000);
+ ASSERT_STREQ_LEN(written, buff, "-1.250000");
+
+ // Precision Tests.
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%.16K", 0xFFFFFFFF);
+ ASSERT_STREQ_LEN(written, buff, "65535.9999847412109375");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%.32lK", 0xFFFFFFFFFFFFFFFF);
+ ASSERT_STREQ_LEN(written, buff,
+ "4294967295.99999999976716935634613037109375");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%.0K", 0xFFFFFFFF);
+ ASSERT_STREQ_LEN(written, buff, "65536");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%.0R", 0xFFFF);
+ ASSERT_STREQ_LEN(written, buff, "1");
+
+ // Flag Tests.
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%+k", 0x0000a000);
+ ASSERT_STREQ_LEN(written, buff, "+1.250000");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%+k", 0x8000a000);
+ ASSERT_STREQ_LEN(written, buff, "-1.250000");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "% k", 0x0000a000);
+ ASSERT_STREQ_LEN(written, buff, " 1.250000");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "% k", 0x8000a000);
+ ASSERT_STREQ_LEN(written, buff, "-1.250000");
+
+ // unsigned variants ignore sign flags.
+ written = LIBC_NAMESPACE::sprintf(buff, "%+K", 0x00014000);
+ ASSERT_STREQ_LEN(written, buff, "1.250000");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "% K", 0x00014000);
+ ASSERT_STREQ_LEN(written, buff, "1.250000");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%-10k", 0x0000c000);
+ ASSERT_STREQ_LEN(written, buff, "1.500000 ");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%#.k", 0x00008000);
+ ASSERT_STREQ_LEN(written, buff, "1.");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%#.0k", 0x0000c000);
+ ASSERT_STREQ_LEN(written, buff, "2.");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%010k", 0x0000c000);
+ ASSERT_STREQ_LEN(written, buff, "001.500000");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%010k", 0x8000c000);
+ ASSERT_STREQ_LEN(written, buff, "-01.500000");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%+- #0k", 0);
+ ASSERT_STREQ_LEN(written, buff, "+0.000000");
+
+ // Combined Tests.
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%10.2k", 0x0004feb8);
+ ASSERT_STREQ_LEN(written, buff, " 9.99");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%5.1k", 0x0004feb8);
+ ASSERT_STREQ_LEN(written, buff, " 10.0");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%-10.2k", 0x0004feb8);
+ ASSERT_STREQ_LEN(written, buff, "9.99 ");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%-5.1k", 0x0004feb8);
+ ASSERT_STREQ_LEN(written, buff, "10.0 ");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%-5.1k", 0x00000001);
+ ASSERT_STREQ_LEN(written, buff, "0.0 ");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%30k", 0x7fffffff);
+ ASSERT_STREQ_LEN(written, buff, " 65535.999969");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%-30k", 0x7fffffff);
+ ASSERT_STREQ_LEN(written, buff, "65535.999969 ");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%20.2lK", 0x3b9ac9ffFD70A3D7);
+ ASSERT_STREQ_LEN(written, buff, " 999999999.99");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%20.1lK", 0x3b9ac9ffFD70A3D7);
+ ASSERT_STREQ_LEN(written, buff, " 1000000000.0");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%12.3R %-12.3k", 0x1999, 0x00800000);
+ ASSERT_STREQ_LEN(written, buff, " 0.100 256.000 ");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%+-#12.3lk % 012.3k",
+ 0x000000001013a92a, 0x02740000);
+ ASSERT_STREQ_LEN(written, buff, "+0.126 0001256.000");
+}
+#endif // defined(LIBC_COMPILER_HAS_FIXED_POINT) &&
+ // !defined(LIBC_COPT_PRINTF_DISABLE_FIXED_POINT)
+
#ifndef LIBC_COPT_PRINTF_DISABLE_WRITE_INT
TEST(LlvmLibcSPrintfTest, WriteIntConv) {
char buff[64];
diff --git a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
index 8d11fb9be188ff..5b1a6ff6e5427b 100644
--- a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
@@ -613,8 +613,8 @@ libc_support_library(
libc_support_library(
name = "__support_fixed_point",
hdrs = [
- "src/__support/fixed_point/fx_rep.h",
"src/__support/fixed_point/fx_bits.h",
+ "src/__support/fixed_point/fx_rep.h",
],
deps = [
":__support_cpp_bit",
@@ -2981,20 +2981,21 @@ libc_function(
################################ stdio targets #################################
libc_support_library(
- name = "printf_core_structs",
- hdrs = ["src/stdio/printf_core/core_structs.h"],
+ name = "printf_config",
+ hdrs = ["src/stdio/printf_core/printf_config.h"],
defines = PRINTF_COPTS,
deps = [
- ":__support_cpp_string_view",
- ":__support_fputil_fp_bits",
],
)
libc_support_library(
- name = "printf_config",
- hdrs = ["src/stdio/printf_core/printf_config.h"],
+ name = "printf_core_structs",
+ hdrs = ["src/stdio/printf_core/core_structs.h"],
defines = PRINTF_COPTS,
deps = [
+ ":__support_cpp_string_view",
+ ":__support_fputil_fp_bits",
+ ":printf_config",
],
)
@@ -3080,6 +3081,7 @@ libc_support_library(
":__support_libc_assert",
":__support_uint",
":__support_uint128",
+ ":printf_config",
":printf_core_structs",
":printf_writer",
],
>From 553c7d9a267465d0eb3bf068454db62e0af01c97 Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Fri, 23 Feb 2024 10:53:06 -0800
Subject: [PATCH 2/6] add fuzzer
---
libc/fuzzing/stdio/CMakeLists.txt | 14 ++
libc/fuzzing/stdio/printf_fixed_conv_fuzz.cpp | 144 ++++++++++++++++++
2 files changed, 158 insertions(+)
create mode 100644 libc/fuzzing/stdio/printf_fixed_conv_fuzz.cpp
diff --git a/libc/fuzzing/stdio/CMakeLists.txt b/libc/fuzzing/stdio/CMakeLists.txt
index 22de67d42747fa..8f89baa7020000 100644
--- a/libc/fuzzing/stdio/CMakeLists.txt
+++ b/libc/fuzzing/stdio/CMakeLists.txt
@@ -15,3 +15,17 @@ add_libc_fuzzer(
libc.src.stdio.snprintf
libc.src.__support.FPUtil.fp_bits
)
+
+if(LIBC_COMPILER_HAS_FIXED_POINT)
+ add_libc_fuzzer(
+ printf_fixed_conv_fuzz
+ NEED_MPFR
+ SRCS
+ printf_fixed_conv_fuzz.cpp
+ DEPENDS
+ libc.src.stdio.snprintf
+ libc.src.__support.fixed_point.fx_bits
+ COMPILE_OPTIONS
+ -ffixed-point # TODO: add -ffixed-point to fuzz tests automatically
+ )
+endif()
diff --git a/libc/fuzzing/stdio/printf_fixed_conv_fuzz.cpp b/libc/fuzzing/stdio/printf_fixed_conv_fuzz.cpp
new file mode 100644
index 00000000000000..1c84fb8399ea31
--- /dev/null
+++ b/libc/fuzzing/stdio/printf_fixed_conv_fuzz.cpp
@@ -0,0 +1,144 @@
+//===-- printf_fixed_conv_fuzz.cpp ----------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+///
+/// Fuzzing test for llvm-libc printf %f/e/g/a implementations.
+///
+//===----------------------------------------------------------------------===//
+#include "src/stdio/snprintf.h"
+
+#include "include/llvm-libc-macros/stdfix-macros.h"
+#include "src/__support/fixed_point/fx_bits.h"
+#include "src/__support/fixed_point/fx_rep.h"
+
+#include <stddef.h>
+#include <stdint.h>
+
+#include "utils/MPFRWrapper/mpfr_inc.h"
+
+constexpr int MAX_SIZE = 10000;
+
+inline bool simple_streq(char *first, char *second, int length) {
+ for (int i = 0; i < length; ++i) {
+ if (first[i] != second[i]) {
+ return false;
+ }
+ }
+ return true;
+}
+
+enum class TestResult {
+ Success,
+ BufferSizeFailed,
+ LengthsDiffer,
+ StringsNotEqual,
+};
+
+template <typename F>
+inline TestResult test_vals(const char *fmt, uint64_t num, int prec,
+ int width) {
+ typename LIBC_NAMESPACE::fixed_point::FXRep<F>::StorageType raw_num = num;
+
+ long double ld_num = 0.0L;
+ long double ld_fract = 0.0L;
+
+ auto raw_num_bits = LIBC_NAMESPACE::fixed_point::FXBits<F>(raw_num);
+
+ // This needs to be a float with enough bits of precision to hold the fixed
+ // point number.
+ static_assert(sizeof(long double) > sizeof(long accum));
+
+ // build a long double that is equivalent to the fixed point number.
+ ld_num = static_cast<long double>(raw_num_bits.get_integral());
+ ld_fract = static_cast<long double>(raw_num_bits.get_fraction()) /
+ static_cast<long double>(1ll << raw_num_bits.get_exponent());
+
+ ld_num = ld_num + ld_fract;
+
+ if (raw_num_bits.get_sign()) {
+ ld_num = -ld_num;
+ }
+
+ // Call snprintf on a nullptr to get the buffer size.
+ int buffer_size = LIBC_NAMESPACE::snprintf(nullptr, 0, fmt, width, prec, num);
+
+ if (buffer_size < 0) {
+ return TestResult::BufferSizeFailed;
+ }
+
+ char *test_buff = new char[buffer_size + 1];
+ char *reference_buff = new char[buffer_size + 1];
+
+ int test_result = 0;
+ int reference_result = 0;
+
+ test_result = LIBC_NAMESPACE::snprintf(test_buff, buffer_size + 1, fmt, width,
+ prec, num);
+
+ // The fixed point format is defined to be %f equivalent.
+ reference_result = mpfr_snprintf(reference_buff, buffer_size + 1, "%*.*Lf",
+ width, prec, ld_num);
+
+ // All of these calls should return that they wrote the same amount.
+ if (test_result != reference_result || test_result != buffer_size) {
+ return TestResult::LengthsDiffer;
+ }
+
+ if (!simple_streq(test_buff, reference_buff, buffer_size)) {
+ return TestResult::StringsNotEqual;
+ }
+
+ delete[] test_buff;
+ delete[] reference_buff;
+ return TestResult::Success;
+}
+
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
+ // const uint8_t raw_data[] = {0x8d,0x43,0x40,0x0,0x0,0x0,};
+ // data = raw_data;
+ // size = sizeof(raw_data);
+ int prec = 0;
+ int width = 0;
+
+ LIBC_NAMESPACE::fixed_point::FXRep<long accum>::StorageType raw_num = 0;
+
+ // Copy as many bytes of data as will fit into num, prec, and with. Any extras
+ // are ignored.
+ for (size_t cur = 0; cur < size; ++cur) {
+ if (cur < sizeof(raw_num)) {
+ raw_num = (raw_num << 8) + data[cur];
+ } else if (cur < sizeof(raw_num) + sizeof(prec)) {
+ prec = (prec << 8) + data[cur];
+ } else if (cur < sizeof(raw_num) + sizeof(prec) + sizeof(width)) {
+ width = (width << 8) + data[cur];
+ }
+ }
+
+ if (width > MAX_SIZE) {
+ width = MAX_SIZE;
+ } else if (width < -MAX_SIZE) {
+ width = -MAX_SIZE;
+ }
+
+ if (prec > MAX_SIZE) {
+ prec = MAX_SIZE;
+ } else if (prec < -MAX_SIZE) {
+ prec = -MAX_SIZE;
+ }
+
+ TestResult result;
+ result = test_vals<long accum>("%*.*lk", raw_num, prec, width);
+ if (result != TestResult::Success) {
+ __builtin_trap();
+ }
+ result = test_vals<unsigned long accum>("%*.*lK", raw_num, prec, width);
+ if (result != TestResult::Success) {
+ __builtin_trap();
+ }
+
+ return 0;
+}
>From 2bd5302372465c28ed99b8813a1767c48cd0a1f3 Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Fri, 23 Feb 2024 11:05:14 -0800
Subject: [PATCH 3/6] add documentation on the fixed point conversions.
---
libc/docs/dev/printf_behavior.rst | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/libc/docs/dev/printf_behavior.rst b/libc/docs/dev/printf_behavior.rst
index 7786b27a9fdcd5..e59cc12497187b 100644
--- a/libc/docs/dev/printf_behavior.rst
+++ b/libc/docs/dev/printf_behavior.rst
@@ -198,3 +198,8 @@ original conversion.
The %p conversion will display a null pointer as if it was the string
"(nullptr)" passed to a "%s" conversion, with all other options remaining the
same as the original conversion.
+
+The %r, %R, %k, and %K fixed point number format specifiers are accepted as
+defined in ISO/IEC TR 18037 (the fixed point number extension). These are
+available when the compiler is detected as having support for fixed point
+numbers and the LIBC_COPT_PRINTF_DISABLE_FIXED_POINT flag is not set.
>From 8d0db6be71fd696fdb69aae6c9e1a3eabe36fd7e Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Mon, 26 Feb 2024 15:28:18 -0800
Subject: [PATCH 4/6] address comments, add fixed point values in comments to
sprintf_test
---
libc/fuzzing/stdio/printf_fixed_conv_fuzz.cpp | 59 ++++------
libc/src/stdio/printf_core/fixed_converter.h | 36 +++---
libc/test/src/stdio/sprintf_test.cpp | 103 ++++++++++--------
3 files changed, 98 insertions(+), 100 deletions(-)
diff --git a/libc/fuzzing/stdio/printf_fixed_conv_fuzz.cpp b/libc/fuzzing/stdio/printf_fixed_conv_fuzz.cpp
index 1c84fb8399ea31..b4a8621891203d 100644
--- a/libc/fuzzing/stdio/printf_fixed_conv_fuzz.cpp
+++ b/libc/fuzzing/stdio/printf_fixed_conv_fuzz.cpp
@@ -23,14 +23,21 @@
constexpr int MAX_SIZE = 10000;
inline bool simple_streq(char *first, char *second, int length) {
- for (int i = 0; i < length; ++i) {
- if (first[i] != second[i]) {
+ for (int i = 0; i < length; ++i)
+ if (first[i] != second[i])
return false;
- }
- }
+
return true;
}
+inline int clamp(int num, int max) {
+ if (num > max)
+ return max;
+ if (num < -max)
+ return -max;
+ return num;
+}
+
enum class TestResult {
Success,
BufferSizeFailed,
@@ -43,9 +50,6 @@ inline TestResult test_vals(const char *fmt, uint64_t num, int prec,
int width) {
typename LIBC_NAMESPACE::fixed_point::FXRep<F>::StorageType raw_num = num;
- long double ld_num = 0.0L;
- long double ld_fract = 0.0L;
-
auto raw_num_bits = LIBC_NAMESPACE::fixed_point::FXBits<F>(raw_num);
// This needs to be a float with enough bits of precision to hold the fixed
@@ -53,22 +57,19 @@ inline TestResult test_vals(const char *fmt, uint64_t num, int prec,
static_assert(sizeof(long double) > sizeof(long accum));
// build a long double that is equivalent to the fixed point number.
- ld_num = static_cast<long double>(raw_num_bits.get_integral());
- ld_fract = static_cast<long double>(raw_num_bits.get_fraction()) /
- static_cast<long double>(1ll << raw_num_bits.get_exponent());
-
- ld_num = ld_num + ld_fract;
+ long double ld_num =
+ static_cast<long double>(raw_num_bits.get_integral()) +
+ (static_cast<long double>(raw_num_bits.get_fraction()) /
+ static_cast<long double>(1ll << raw_num_bits.get_exponent()));
- if (raw_num_bits.get_sign()) {
+ if (raw_num_bits.get_sign())
ld_num = -ld_num;
- }
// Call snprintf on a nullptr to get the buffer size.
int buffer_size = LIBC_NAMESPACE::snprintf(nullptr, 0, fmt, width, prec, num);
- if (buffer_size < 0) {
+ if (buffer_size < 0)
return TestResult::BufferSizeFailed;
- }
char *test_buff = new char[buffer_size + 1];
char *reference_buff = new char[buffer_size + 1];
@@ -84,13 +85,11 @@ inline TestResult test_vals(const char *fmt, uint64_t num, int prec,
width, prec, ld_num);
// All of these calls should return that they wrote the same amount.
- if (test_result != reference_result || test_result != buffer_size) {
+ if (test_result != reference_result || test_result != buffer_size)
return TestResult::LengthsDiffer;
- }
- if (!simple_streq(test_buff, reference_buff, buffer_size)) {
+ if (!simple_streq(test_buff, reference_buff, buffer_size))
return TestResult::StringsNotEqual;
- }
delete[] test_buff;
delete[] reference_buff;
@@ -118,27 +117,17 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
}
}
- if (width > MAX_SIZE) {
- width = MAX_SIZE;
- } else if (width < -MAX_SIZE) {
- width = -MAX_SIZE;
- }
-
- if (prec > MAX_SIZE) {
- prec = MAX_SIZE;
- } else if (prec < -MAX_SIZE) {
- prec = -MAX_SIZE;
- }
+ width = clamp(width, MAX_SIZE);
+ prec = clamp(prec, MAX_SIZE);
TestResult result;
result = test_vals<long accum>("%*.*lk", raw_num, prec, width);
- if (result != TestResult::Success) {
+ if (result != TestResult::Success)
__builtin_trap();
- }
+
result = test_vals<unsigned long accum>("%*.*lK", raw_num, prec, width);
- if (result != TestResult::Success) {
+ if (result != TestResult::Success)
__builtin_trap();
- }
return 0;
}
diff --git a/libc/src/stdio/printf_core/fixed_converter.h b/libc/src/stdio/printf_core/fixed_converter.h
index 5d521f2957a560..525d3d8d100549 100644
--- a/libc/src/stdio/printf_core/fixed_converter.h
+++ b/libc/src/stdio/printf_core/fixed_converter.h
@@ -29,7 +29,7 @@ namespace printf_core {
LIBC_INLINE constexpr uint32_t const_ten_exp(uint32_t exponent) {
uint32_t result = 1;
LIBC_ASSERT(exponent < 11);
- for (size_t i = 0; i < exponent; ++i)
+ for (uint32_t i = 0; i < exponent; ++i)
result *= 10;
return result;
@@ -176,23 +176,6 @@ LIBC_INLINE int convert_fixed(Writer *writer, const FormatSection &to_conv) {
"StorageType must be large enough to hold the fractional "
"component multiplied by a 32 bit number.");
- char sign_char = 0;
-
- // Check if the conv name is uppercase
- if (a == 'A') {
- // These flags are only for signed conversions, so this removes them if the
- // conversion is unsigned.
- flags = FormatFlags(flags &
- ~(FormatFlags::FORCE_SIGN | FormatFlags::SPACE_PREFIX));
- }
-
- if (is_negative)
- sign_char = '-';
- else if ((flags & FormatFlags::FORCE_SIGN) == FormatFlags::FORCE_SIGN)
- sign_char = '+'; // FORCE_SIGN has precedence over SPACE_PREFIX
- else if ((flags & FormatFlags::SPACE_PREFIX) == FormatFlags::SPACE_PREFIX)
- sign_char = ' ';
-
// If to_conv doesn't specify a precision, the precision defaults to 6.
const size_t precision = to_conv.precision < 0 ? 6 : to_conv.precision;
bool has_decimal_point =
@@ -336,6 +319,23 @@ LIBC_INLINE int convert_fixed(Writer *writer, const FormatSection &to_conv) {
constexpr cpp::string_view DECIMAL_POINT(".");
+ char sign_char = 0;
+
+ // Check if the conv name is uppercase
+ if (a == 'A') {
+ // These flags are only for signed conversions, so this removes them if the
+ // conversion is unsigned.
+ flags = FormatFlags(flags &
+ ~(FormatFlags::FORCE_SIGN | FormatFlags::SPACE_PREFIX));
+ }
+
+ if (is_negative)
+ sign_char = '-';
+ else if ((flags & FormatFlags::FORCE_SIGN) == FormatFlags::FORCE_SIGN)
+ sign_char = '+'; // FORCE_SIGN has precedence over SPACE_PREFIX
+ else if ((flags & FormatFlags::SPACE_PREFIX) == FormatFlags::SPACE_PREFIX)
+ sign_char = ' ';
+
padding = static_cast<int>(to_conv.min_width - (sign_char > 0 ? 1 : 0) -
integral_str.size() -
static_cast<int>(has_decimal_point) -
diff --git a/libc/test/src/stdio/sprintf_test.cpp b/libc/test/src/stdio/sprintf_test.cpp
index 50a7d8a195a40c..2209df8db1338b 100644
--- a/libc/test/src/stdio/sprintf_test.cpp
+++ b/libc/test/src/stdio/sprintf_test.cpp
@@ -3208,154 +3208,163 @@ TEST_F(LlvmLibcSPrintfTest, FixedConv) {
// These numeric tests are potentially a little weak, but the fuzz test is
// more thorough than my handwritten tests tend to be.
- // TODO: Commit the fuzzer
+ // TODO: Replace hex literals with their appropriate fixed point literals.
- written = LIBC_NAMESPACE::sprintf(buff, "%k", 0x0);
+ written = LIBC_NAMESPACE::sprintf(buff, "%k", 0x0); // 0.0
ASSERT_STREQ_LEN(written, buff, "0.000000");
- written = LIBC_NAMESPACE::sprintf(buff, "%k", 0x80000000);
+ written = LIBC_NAMESPACE::sprintf(buff, "%k", 0x80000000); // -0.0
ASSERT_STREQ_LEN(written, buff, "-0.000000");
- written = LIBC_NAMESPACE::sprintf(buff, "%r", 0xffff);
+ written = LIBC_NAMESPACE::sprintf(buff, "%r", 0xffff); // -fract max
ASSERT_STREQ_LEN(written, buff, "-0.999969");
- written = LIBC_NAMESPACE::sprintf(buff, "%R", 0xffff);
+ written = LIBC_NAMESPACE::sprintf(buff, "%R", 0xffff); // unsigned fract max
ASSERT_STREQ_LEN(written, buff, "0.999985");
- written = LIBC_NAMESPACE::sprintf(buff, "%k", 0xffffffff);
+ written = LIBC_NAMESPACE::sprintf(buff, "%k", 0xffffffff); // -accum max
ASSERT_STREQ_LEN(written, buff, "-65535.999969");
- written = LIBC_NAMESPACE::sprintf(buff, "%K", 0xffffffff);
+ written =
+ LIBC_NAMESPACE::sprintf(buff, "%K", 0xffffffff); // unsigned accum max
ASSERT_STREQ_LEN(written, buff, "65535.999985");
- written = LIBC_NAMESPACE::sprintf(buff, "%r", 0x7fff);
+ written = LIBC_NAMESPACE::sprintf(buff, "%r", 0x7fff); // fract max
ASSERT_STREQ_LEN(written, buff, "0.999969");
- written = LIBC_NAMESPACE::sprintf(buff, "%k", 0x7fffffff);
+ written = LIBC_NAMESPACE::sprintf(buff, "%k", 0x7fffffff); // accum max
ASSERT_STREQ_LEN(written, buff, "65535.999969");
// Length Modifier Tests.
- written = LIBC_NAMESPACE::sprintf(buff, "%hk", 0x0);
+ written = LIBC_NAMESPACE::sprintf(buff, "%hk", 0x0); // 0.0
ASSERT_STREQ_LEN(written, buff, "0.000000");
- written = LIBC_NAMESPACE::sprintf(buff, "%hk", 0xffff);
+ written = LIBC_NAMESPACE::sprintf(buff, "%hk", 0xffff); // -short accum max
ASSERT_STREQ_LEN(written, buff, "-255.992188");
- written = LIBC_NAMESPACE::sprintf(buff, "%lk", 0x0);
+ written = LIBC_NAMESPACE::sprintf(buff, "%lk", 0x0); // 0.0
ASSERT_STREQ_LEN(written, buff, "0.000000");
- written = LIBC_NAMESPACE::sprintf(buff, "%lk", 0xffffffffffffffff);
+ written = LIBC_NAMESPACE::sprintf(buff, "%lk",
+ 0xffffffffffffffff); //-long accum max
ASSERT_STREQ_LEN(written, buff, "-4294967296.000000");
// Min Width Tests.
- written = LIBC_NAMESPACE::sprintf(buff, "%10k", 0x0000a000);
+ written = LIBC_NAMESPACE::sprintf(buff, "%10k", 0x0000a000); // 1.25
ASSERT_STREQ_LEN(written, buff, " 1.250000");
- written = LIBC_NAMESPACE::sprintf(buff, "%10k", 0x8000a000);
+ written = LIBC_NAMESPACE::sprintf(buff, "%10k", 0x8000a000); //-1.25
ASSERT_STREQ_LEN(written, buff, " -1.250000");
- written = LIBC_NAMESPACE::sprintf(buff, "%8k", 0x0000a000);
+ written = LIBC_NAMESPACE::sprintf(buff, "%8k", 0x0000a000); // 1.25
ASSERT_STREQ_LEN(written, buff, "1.250000");
- written = LIBC_NAMESPACE::sprintf(buff, "%9k", 0x8000a000);
+ written = LIBC_NAMESPACE::sprintf(buff, "%9k", 0x8000a000); //-1.25
ASSERT_STREQ_LEN(written, buff, "-1.250000");
- written = LIBC_NAMESPACE::sprintf(buff, "%4k", 0x0000a000);
+ written = LIBC_NAMESPACE::sprintf(buff, "%4k", 0x0000a000); // 1.25
ASSERT_STREQ_LEN(written, buff, "1.250000");
- written = LIBC_NAMESPACE::sprintf(buff, "%4k", 0x8000a000);
+ written = LIBC_NAMESPACE::sprintf(buff, "%4k", 0x8000a000); //-1.25
ASSERT_STREQ_LEN(written, buff, "-1.250000");
// Precision Tests.
- written = LIBC_NAMESPACE::sprintf(buff, "%.16K", 0xFFFFFFFF);
+ written =
+ LIBC_NAMESPACE::sprintf(buff, "%.16K", 0xFFFFFFFF); // unsigned accum max
ASSERT_STREQ_LEN(written, buff, "65535.9999847412109375");
- written = LIBC_NAMESPACE::sprintf(buff, "%.32lK", 0xFFFFFFFFFFFFFFFF);
+ written = LIBC_NAMESPACE::sprintf(
+ buff, "%.32lK", 0xFFFFFFFFFFFFFFFF); // unsigned long accum max
ASSERT_STREQ_LEN(written, buff,
"4294967295.99999999976716935634613037109375");
- written = LIBC_NAMESPACE::sprintf(buff, "%.0K", 0xFFFFFFFF);
+ written =
+ LIBC_NAMESPACE::sprintf(buff, "%.0K", 0xFFFFFFFF); // unsigned accum max
ASSERT_STREQ_LEN(written, buff, "65536");
- written = LIBC_NAMESPACE::sprintf(buff, "%.0R", 0xFFFF);
+ written = LIBC_NAMESPACE::sprintf(buff, "%.0R", 0xFFFF); // unsigned fract max
ASSERT_STREQ_LEN(written, buff, "1");
// Flag Tests.
- written = LIBC_NAMESPACE::sprintf(buff, "%+k", 0x0000a000);
+ written = LIBC_NAMESPACE::sprintf(buff, "%+k", 0x0000a000); // 1.25
ASSERT_STREQ_LEN(written, buff, "+1.250000");
- written = LIBC_NAMESPACE::sprintf(buff, "%+k", 0x8000a000);
+ written = LIBC_NAMESPACE::sprintf(buff, "%+k", 0x8000a000); //-1.25
ASSERT_STREQ_LEN(written, buff, "-1.250000");
- written = LIBC_NAMESPACE::sprintf(buff, "% k", 0x0000a000);
+ written = LIBC_NAMESPACE::sprintf(buff, "% k", 0x0000a000); // 1.25
ASSERT_STREQ_LEN(written, buff, " 1.250000");
- written = LIBC_NAMESPACE::sprintf(buff, "% k", 0x8000a000);
+ written = LIBC_NAMESPACE::sprintf(buff, "% k", 0x8000a000); //-1.25
ASSERT_STREQ_LEN(written, buff, "-1.250000");
// unsigned variants ignore sign flags.
- written = LIBC_NAMESPACE::sprintf(buff, "%+K", 0x00014000);
+ written = LIBC_NAMESPACE::sprintf(buff, "%+K", 0x00014000); // 1.25
ASSERT_STREQ_LEN(written, buff, "1.250000");
- written = LIBC_NAMESPACE::sprintf(buff, "% K", 0x00014000);
+ written = LIBC_NAMESPACE::sprintf(buff, "% K", 0x00014000); // 1.25
ASSERT_STREQ_LEN(written, buff, "1.250000");
- written = LIBC_NAMESPACE::sprintf(buff, "%-10k", 0x0000c000);
+ written = LIBC_NAMESPACE::sprintf(buff, "%-10k", 0x0000c000); // 1.5
ASSERT_STREQ_LEN(written, buff, "1.500000 ");
- written = LIBC_NAMESPACE::sprintf(buff, "%#.k", 0x00008000);
+ written = LIBC_NAMESPACE::sprintf(buff, "%#.k", 0x00008000); // 1.0
ASSERT_STREQ_LEN(written, buff, "1.");
- written = LIBC_NAMESPACE::sprintf(buff, "%#.0k", 0x0000c000);
+ written = LIBC_NAMESPACE::sprintf(buff, "%#.0k", 0x0000c000); // 1.5
ASSERT_STREQ_LEN(written, buff, "2.");
- written = LIBC_NAMESPACE::sprintf(buff, "%010k", 0x0000c000);
+ written = LIBC_NAMESPACE::sprintf(buff, "%010k", 0x0000c000); // 1.5
ASSERT_STREQ_LEN(written, buff, "001.500000");
- written = LIBC_NAMESPACE::sprintf(buff, "%010k", 0x8000c000);
+ written = LIBC_NAMESPACE::sprintf(buff, "%010k", 0x8000c000); //-1.5
ASSERT_STREQ_LEN(written, buff, "-01.500000");
- written = LIBC_NAMESPACE::sprintf(buff, "%+- #0k", 0);
+ written = LIBC_NAMESPACE::sprintf(buff, "%+- #0k", 0); // 0.0
ASSERT_STREQ_LEN(written, buff, "+0.000000");
// Combined Tests.
- written = LIBC_NAMESPACE::sprintf(buff, "%10.2k", 0x0004feb8);
+ written = LIBC_NAMESPACE::sprintf(buff, "%10.2k", 0x0004feb8); // 9.99
ASSERT_STREQ_LEN(written, buff, " 9.99");
- written = LIBC_NAMESPACE::sprintf(buff, "%5.1k", 0x0004feb8);
+ written = LIBC_NAMESPACE::sprintf(buff, "%5.1k", 0x0004feb8); // 9.99
ASSERT_STREQ_LEN(written, buff, " 10.0");
- written = LIBC_NAMESPACE::sprintf(buff, "%-10.2k", 0x0004feb8);
+ written = LIBC_NAMESPACE::sprintf(buff, "%-10.2k", 0x0004feb8); // 9.99
ASSERT_STREQ_LEN(written, buff, "9.99 ");
- written = LIBC_NAMESPACE::sprintf(buff, "%-5.1k", 0x0004feb8);
+ written = LIBC_NAMESPACE::sprintf(buff, "%-5.1k", 0x0004feb8); // 9.99
ASSERT_STREQ_LEN(written, buff, "10.0 ");
- written = LIBC_NAMESPACE::sprintf(buff, "%-5.1k", 0x00000001);
+ written = LIBC_NAMESPACE::sprintf(buff, "%-5.1k", 0x00000001); // accum min
ASSERT_STREQ_LEN(written, buff, "0.0 ");
- written = LIBC_NAMESPACE::sprintf(buff, "%30k", 0x7fffffff);
+ written = LIBC_NAMESPACE::sprintf(buff, "%30k", 0x7fffffff); // accum max
ASSERT_STREQ_LEN(written, buff, " 65535.999969");
- written = LIBC_NAMESPACE::sprintf(buff, "%-30k", 0x7fffffff);
+ written = LIBC_NAMESPACE::sprintf(buff, "%-30k", 0x7fffffff); // accum max
ASSERT_STREQ_LEN(written, buff, "65535.999969 ");
- written = LIBC_NAMESPACE::sprintf(buff, "%20.2lK", 0x3b9ac9ffFD70A3D7);
+ written = LIBC_NAMESPACE::sprintf(buff, "%20.2lK",
+ 0x3b9ac9ffFD70A3D7); // 999999999.99
ASSERT_STREQ_LEN(written, buff, " 999999999.99");
- written = LIBC_NAMESPACE::sprintf(buff, "%20.1lK", 0x3b9ac9ffFD70A3D7);
+ written = LIBC_NAMESPACE::sprintf(buff, "%20.1lK",
+ 0x3b9ac9ffFD70A3D7); // 999999999.99
ASSERT_STREQ_LEN(written, buff, " 1000000000.0");
- written = LIBC_NAMESPACE::sprintf(buff, "%12.3R %-12.3k", 0x1999, 0x00800000);
+ written = LIBC_NAMESPACE::sprintf(buff, "%12.3R %-12.3k", 0x1999,
+ 0x00800000); // 0.1, 256.0
ASSERT_STREQ_LEN(written, buff, " 0.100 256.000 ");
- written = LIBC_NAMESPACE::sprintf(buff, "%+-#12.3lk % 012.3k",
- 0x000000001013a92a, 0x02740000);
+ written =
+ LIBC_NAMESPACE::sprintf(buff, "%+-#12.3lk % 012.3k", 0x000000001013a92a,
+ 0x02740000); // 0.126, 1256.0
ASSERT_STREQ_LEN(written, buff, "+0.126 0001256.000");
}
#endif // defined(LIBC_COMPILER_HAS_FIXED_POINT) &&
>From 3aacbac3494205e3910d294a1dc8e96c27e3f778 Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Tue, 27 Feb 2024 10:11:53 -0800
Subject: [PATCH 5/6] move type identification to nested macros for simpicity
Also I added more tests and a failure condition.
---
libc/src/stdio/printf_core/core_structs.h | 1 +
libc/src/stdio/printf_core/fixed_converter.h | 137 ++++---------------
libc/test/src/stdio/sprintf_test.cpp | 42 ++++++
3 files changed, 73 insertions(+), 107 deletions(-)
diff --git a/libc/src/stdio/printf_core/core_structs.h b/libc/src/stdio/printf_core/core_structs.h
index 681f85dd5a285d..d3718b49d1b13a 100644
--- a/libc/src/stdio/printf_core/core_structs.h
+++ b/libc/src/stdio/printf_core/core_structs.h
@@ -124,6 +124,7 @@ constexpr int FILE_WRITE_ERROR = -1;
constexpr int FILE_STATUS_ERROR = -2;
constexpr int NULLPTR_WRITE_ERROR = -3;
constexpr int INT_CONVERSION_ERROR = -4;
+constexpr int FIXED_POINT_CONVERSION_ERROR = -5;
} // namespace printf_core
} // namespace LIBC_NAMESPACE
diff --git a/libc/src/stdio/printf_core/fixed_converter.h b/libc/src/stdio/printf_core/fixed_converter.h
index 525d3d8d100549..de69c603be6b63 100644
--- a/libc/src/stdio/printf_core/fixed_converter.h
+++ b/libc/src/stdio/printf_core/fixed_converter.h
@@ -35,14 +35,33 @@ LIBC_INLINE constexpr uint32_t const_ten_exp(uint32_t exponent) {
return result;
}
-LIBC_INLINE int convert_fixed(Writer *writer, const FormatSection &to_conv) {
- using SAStorageType = fixed_point::FXRep<short accum>::StorageType;
- using AStorageType = fixed_point::FXRep<accum>::StorageType;
- using LAStorageType = fixed_point::FXRep<long accum>::StorageType;
- using SFStorageType = fixed_point::FXRep<short fract>::StorageType;
- using FStorageType = fixed_point::FXRep<fract>::StorageType;
- using LFStorageType = fixed_point::FXRep<long fract>::StorageType;
+#define READ_FX_BITS(TYPE) \
+ do { \
+ auto fixed_bits = fixed_point::FXBits<TYPE>( \
+ fixed_point::FXRep<TYPE>::StorageType(to_conv.conv_val_raw)); \
+ integral = fixed_bits.get_integral(); \
+ fractional = fixed_bits.get_fraction(); \
+ exponent = fixed_bits.get_exponent(); \
+ is_negative = fixed_bits.get_sign(); \
+ } while (false)
+
+#define APPLY_FX_LENGTH_MODIFIER(LENGTH_MODIFIER) \
+ do { \
+ if (to_conv.conv_name == 'r') { \
+ READ_FX_BITS(LENGTH_MODIFIER fract); \
+ } else if (to_conv.conv_name == 'R') { \
+ READ_FX_BITS(unsigned LENGTH_MODIFIER fract); \
+ } else if (to_conv.conv_name == 'k') { \
+ READ_FX_BITS(LENGTH_MODIFIER accum); \
+ } else if (to_conv.conv_name == 'K') { \
+ READ_FX_BITS(unsigned LENGTH_MODIFIER accum); \
+ } else { \
+ LIBC_ASSERT(false && "Invalid conversion name passed to convert_fixed"); \
+ return FIXED_POINT_CONVERSION_ERROR; \
+ } \
+ } while (false)
+LIBC_INLINE int convert_fixed(Writer *writer, const FormatSection &to_conv) {
// Long accum should be the largest type, so we can store all the smaller
// numbers in things sized for it.
using LARep = fixed_point::FXRep<unsigned long accum>;
@@ -59,7 +78,6 @@ LIBC_INLINE int convert_fixed(Writer *writer, const FormatSection &to_conv) {
StorageType integral;
StorageType fractional;
- // TODO: See about simplifying this mess of a 3D matrix if statement.
// r = fract
// k = accum
// lowercase = signed
@@ -67,108 +85,13 @@ LIBC_INLINE int convert_fixed(Writer *writer, const FormatSection &to_conv) {
// h = short
// l = long
// any other length modifier has no effect
+
if (to_conv.length_modifier == LengthModifier::h) {
- // short types
- if (to_conv.conv_name == 'r') {
- auto fixed_bits = fixed_point::FXBits<short fract>(
- static_cast<SFStorageType>(to_conv.conv_val_raw));
- integral = fixed_bits.get_integral();
- fractional = fixed_bits.get_fraction();
- exponent = fixed_bits.get_exponent();
- is_negative = fixed_bits.get_sign();
- }
- if (to_conv.conv_name == 'R') {
- auto fixed_bits = fixed_point::FXBits<unsigned short fract>(
- static_cast<SFStorageType>(to_conv.conv_val_raw));
- integral = fixed_bits.get_integral();
- fractional = fixed_bits.get_fraction();
- exponent = fixed_bits.get_exponent();
- is_negative = fixed_bits.get_sign();
- }
- if (to_conv.conv_name == 'k') {
- auto fixed_bits = fixed_point::FXBits<short accum>(
- static_cast<SAStorageType>(to_conv.conv_val_raw));
- integral = fixed_bits.get_integral();
- fractional = fixed_bits.get_fraction();
- exponent = fixed_bits.get_exponent();
- is_negative = fixed_bits.get_sign();
- }
- if (to_conv.conv_name == 'K') {
- auto fixed_bits = fixed_point::FXBits<unsigned short accum>(
- static_cast<SAStorageType>(to_conv.conv_val_raw));
- integral = fixed_bits.get_integral();
- fractional = fixed_bits.get_fraction();
- exponent = fixed_bits.get_exponent();
- is_negative = fixed_bits.get_sign();
- }
+ APPLY_FX_LENGTH_MODIFIER(short);
} else if (to_conv.length_modifier == LengthModifier::l) {
- // long types
- if (to_conv.conv_name == 'r') {
- auto fixed_bits = fixed_point::FXBits<long fract>(
- static_cast<LFStorageType>(to_conv.conv_val_raw));
- integral = fixed_bits.get_integral();
- fractional = fixed_bits.get_fraction();
- exponent = fixed_bits.get_exponent();
- is_negative = fixed_bits.get_sign();
- }
- if (to_conv.conv_name == 'R') {
- auto fixed_bits = fixed_point::FXBits<unsigned long fract>(
- static_cast<LFStorageType>(to_conv.conv_val_raw));
- integral = fixed_bits.get_integral();
- fractional = fixed_bits.get_fraction();
- exponent = fixed_bits.get_exponent();
- is_negative = fixed_bits.get_sign();
- }
- if (to_conv.conv_name == 'k') {
- auto fixed_bits = fixed_point::FXBits<long accum>(
- static_cast<LAStorageType>(to_conv.conv_val_raw));
- integral = fixed_bits.get_integral();
- fractional = fixed_bits.get_fraction();
- exponent = fixed_bits.get_exponent();
- is_negative = fixed_bits.get_sign();
- }
- if (to_conv.conv_name == 'K') {
- auto fixed_bits = fixed_point::FXBits<unsigned long accum>(
- static_cast<LAStorageType>(to_conv.conv_val_raw));
- integral = fixed_bits.get_integral();
- fractional = fixed_bits.get_fraction();
- exponent = fixed_bits.get_exponent();
- is_negative = fixed_bits.get_sign();
- }
+ APPLY_FX_LENGTH_MODIFIER(long);
} else {
- // unspecified types
- if (to_conv.conv_name == 'r') {
- auto fixed_bits = fixed_point::FXBits<fract>(
- static_cast<FStorageType>(to_conv.conv_val_raw));
- integral = fixed_bits.get_integral();
- fractional = fixed_bits.get_fraction();
- exponent = fixed_bits.get_exponent();
- is_negative = fixed_bits.get_sign();
- }
- if (to_conv.conv_name == 'R') {
- auto fixed_bits = fixed_point::FXBits<unsigned fract>(
- static_cast<FStorageType>(to_conv.conv_val_raw));
- integral = fixed_bits.get_integral();
- fractional = fixed_bits.get_fraction();
- exponent = fixed_bits.get_exponent();
- is_negative = fixed_bits.get_sign();
- }
- if (to_conv.conv_name == 'k') {
- auto fixed_bits = fixed_point::FXBits<accum>(
- static_cast<AStorageType>(to_conv.conv_val_raw));
- integral = fixed_bits.get_integral();
- fractional = fixed_bits.get_fraction();
- exponent = fixed_bits.get_exponent();
- is_negative = fixed_bits.get_sign();
- }
- if (to_conv.conv_name == 'K') {
- auto fixed_bits = fixed_point::FXBits<unsigned accum>(
- static_cast<AStorageType>(to_conv.conv_val_raw));
- integral = fixed_bits.get_integral();
- fractional = fixed_bits.get_fraction();
- exponent = fixed_bits.get_exponent();
- is_negative = fixed_bits.get_sign();
- }
+ APPLY_FX_LENGTH_MODIFIER();
}
LIBC_ASSERT(static_cast<size_t>(exponent) <=
diff --git a/libc/test/src/stdio/sprintf_test.cpp b/libc/test/src/stdio/sprintf_test.cpp
index 2209df8db1338b..b9f402027e7fc2 100644
--- a/libc/test/src/stdio/sprintf_test.cpp
+++ b/libc/test/src/stdio/sprintf_test.cpp
@@ -3243,6 +3243,26 @@ TEST_F(LlvmLibcSPrintfTest, FixedConv) {
written = LIBC_NAMESPACE::sprintf(buff, "%hk", 0xffff); // -short accum max
ASSERT_STREQ_LEN(written, buff, "-255.992188");
+ written = LIBC_NAMESPACE::sprintf(buff, "%hr", 0x0); // 0.0
+ ASSERT_STREQ_LEN(written, buff, "0.000000");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%hr", 0xff); // -short fract max
+ ASSERT_STREQ_LEN(written, buff, "-0.992188");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%hK", 0x0); // 0.0
+ ASSERT_STREQ_LEN(written, buff, "0.000000");
+
+ written =
+ LIBC_NAMESPACE::sprintf(buff, "%hK", 0xffff); // unsigned short accum max
+ ASSERT_STREQ_LEN(written, buff, "255.996094");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%hR", 0x0); // 0.0
+ ASSERT_STREQ_LEN(written, buff, "0.000000");
+
+ written =
+ LIBC_NAMESPACE::sprintf(buff, "%hR", 0xff); // unsigned short fract max
+ ASSERT_STREQ_LEN(written, buff, "0.996094");
+
written = LIBC_NAMESPACE::sprintf(buff, "%lk", 0x0); // 0.0
ASSERT_STREQ_LEN(written, buff, "0.000000");
@@ -3250,6 +3270,28 @@ TEST_F(LlvmLibcSPrintfTest, FixedConv) {
0xffffffffffffffff); //-long accum max
ASSERT_STREQ_LEN(written, buff, "-4294967296.000000");
+ written = LIBC_NAMESPACE::sprintf(buff, "%lr", 0x0); // 0.0
+ ASSERT_STREQ_LEN(written, buff, "0.000000");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%lr",
+ 0xffffffff); //-long fract max
+ ASSERT_STREQ_LEN(written, buff, "-1.000000");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%lK", 0x0); // 0.0
+ ASSERT_STREQ_LEN(written, buff, "0.000000");
+
+ written =
+ LIBC_NAMESPACE::sprintf(buff, "%lK",
+ 0xffffffffffffffff); // unsigned long accum max
+ ASSERT_STREQ_LEN(written, buff, "4294967296.000000");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%lR", 0x0); // 0.0
+ ASSERT_STREQ_LEN(written, buff, "0.000000");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%lR",
+ 0xffffffff); // unsigned long fract max
+ ASSERT_STREQ_LEN(written, buff, "1.000000");
+
// Min Width Tests.
written = LIBC_NAMESPACE::sprintf(buff, "%10k", 0x0000a000); // 1.25
>From 150b0bee827fb1016bbca0f210d0d621ca1ed97b Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Tue, 27 Feb 2024 10:30:17 -0800
Subject: [PATCH 6/6] add proper cmake configuration variables
---
libc/config/config.json | 4 ++++
libc/docs/configure.rst | 1 +
libc/src/stdio/printf_core/CMakeLists.txt | 3 +++
libc/test/src/stdio/CMakeLists.txt | 3 +++
4 files changed, 11 insertions(+)
diff --git a/libc/config/config.json b/libc/config/config.json
index 6a208cc5566116..b73c47b1a14bc7 100644
--- a/libc/config/config.json
+++ b/libc/config/config.json
@@ -15,6 +15,10 @@
"LIBC_CONF_PRINTF_FLOAT_TO_STR_USE_MEGA_LONG_DOUBLE_TABLE": {
"value": false,
"doc": "Use large table for better printf long double performance."
+ },
+ "LIBC_CONF_PRINTF_DISABLE_FIXED_POINT": {
+ "value": false,
+ "doc": "Disable printing fixed point values in printf and friends."
}
},
"string": {
diff --git a/libc/docs/configure.rst b/libc/docs/configure.rst
index 9997dde6cf899c..a177550647bd91 100644
--- a/libc/docs/configure.rst
+++ b/libc/docs/configure.rst
@@ -26,6 +26,7 @@ overrides in ``config/<platform>/config.json`` and ``config/<platform>/<arch>/co
to learn about the defaults for your platform and target.
* **"printf" options**
+ - ``LIBC_CONF_PRINTF_DISABLE_FIXED_POINT``: Disable printing fixed point values in printf and friends.
- ``LIBC_CONF_PRINTF_DISABLE_FLOAT``: Disable printing floating point values in printf and friends.
- ``LIBC_CONF_PRINTF_DISABLE_INDEX_MODE``: Disable index mode in the printf format string.
- ``LIBC_CONF_PRINTF_DISABLE_WRITE_INT``: Disable handling of %n in printf format string.
diff --git a/libc/src/stdio/printf_core/CMakeLists.txt b/libc/src/stdio/printf_core/CMakeLists.txt
index cd75060db37579..6d4c5ce02a8d63 100644
--- a/libc/src/stdio/printf_core/CMakeLists.txt
+++ b/libc/src/stdio/printf_core/CMakeLists.txt
@@ -10,6 +10,9 @@ endif()
if(LIBC_CONF_PRINTF_FLOAT_TO_STR_USE_MEGA_LONG_DOUBLE_TABLE)
list(APPEND printf_config_copts "-DLIBC_COPT_FLOAT_TO_STR_USE_MEGA_LONG_DOUBLE_TABLE")
endif()
+if(LIBC_CONF_PRINTF_DISABLE_FIXED_POINT)
+ list(APPEND printf_config_copts "-DLIBC_COPT_PRINTF_DISABLE_FIXED_POINT")
+endif()
if(printf_config_copts)
list(PREPEND printf_config_copts "COMPILE_OPTIONS")
endif()
diff --git a/libc/test/src/stdio/CMakeLists.txt b/libc/test/src/stdio/CMakeLists.txt
index 8db2293ab74a97..7c8981e85f6d75 100644
--- a/libc/test/src/stdio/CMakeLists.txt
+++ b/libc/test/src/stdio/CMakeLists.txt
@@ -121,6 +121,9 @@ endif()
if(LIBC_CONF_PRINTF_DISABLE_WRITE_INT)
list(APPEND sprintf_test_copts "-DLIBC_COPT_PRINTF_DISABLE_WRITE_INT")
endif()
+if(LIBC_CONF_PRINTF_DISABLE_FIXED_POINT)
+ list(APPEND sprintf_test_copts "-DLIBC_COPT_PRINTF_DISABLE_FIXED_POINT")
+endif()
add_fp_unittest(
sprintf_test
More information about the libc-commits
mailing list