[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