[libc] move to combined integer converter

The functions converting integers into decimal, hexadecimal, and octal,
are all very similar. This patch moves to a combined converter to save
code size.

Reviewed By: sivachandra

Differential Revision: https://reviews.llvm.org/D131302

Author: michaelrj-google

libc/src/stdio/printf_core/CMakeLists.txt

@@ -63,9 +63,7 @@ add_object_library(
     string_converter.h
     char_converter.h
     int_converter.h
-    hex_converter.h
     ptr_converter.h
-    oct_converter.h
     write_int_converter.h
     float_inf_nan_converter.h
     float_hex_converter.h
@@ -74,6 +72,7 @@ add_object_library(
     .core_structs
     libc.src.__support.integer_to_string
     libc.src.__support.CPP.limits
+    libc.src.__support.CPP.string_view
     libc.src.__support.FPUtil.fputil
 )
 

libc/src/stdio/printf_core/converter.cpp

@@ -38,12 +38,10 @@ int convert(Writer *writer, const FormatSection &to_conv) {
   case 'd':
   case 'i':
   case 'u':
-    return convert_int(writer, to_conv);
   case 'o':
-    return convert_oct(writer, to_conv);
   case 'x':
   case 'X':
-    return convert_hex(writer, to_conv);
+    return convert_int(writer, to_conv);
 #ifndef LLVM_LIBC_PRINTF_DISABLE_FLOAT
   // case 'f':
   // case 'F':

libc/src/stdio/printf_core/converter_atlas.h

@@ -22,11 +22,6 @@
 // defines convert_int
 #include "src/stdio/printf_core/int_converter.h"
 
-// defines convert_oct
-#include "src/stdio/printf_core/oct_converter.h"
-// defines convert_hex
-#include "src/stdio/printf_core/hex_converter.h"
-
 #ifndef LLVM_LIBC_PRINTF_DISABLE_FLOAT
 // defines convert_float_decimal
 // defines convert_float_dec_exp

libc/src/stdio/printf_core/core_structs.h

@@ -86,6 +86,7 @@ constexpr int WRITE_OK = 0;
 constexpr int FILE_WRITE_ERROR = -1;
 constexpr int FILE_STATUS_ERROR = -2;
 constexpr int NULLPTR_WRITE_ERROR = -3;
+constexpr int INT_CONVERSION_ERROR = -4;
 
 } // namespace printf_core
 } // namespace __llvm_libc

libc/src/stdio/printf_core/hex_converter.h

@@ -9,6 +9,8 @@
 #ifndef LLVM_LIBC_SRC_STDIO_PRINTF_CORE_INT_CONVERTER_H
 #define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_INT_CONVERTER_H
 
+#include "src/__support/CPP/ArrayRef.h"
+#include "src/__support/CPP/StringView.h"
 #include "src/__support/integer_to_string.h"
 #include "src/stdio/printf_core/converter_utils.h"
 #include "src/stdio/printf_core/core_structs.h"
@@ -20,6 +22,23 @@
 namespace __llvm_libc {
 namespace printf_core {
 
+// These functions only work on characters that are already known to be in the
+// alphabet. Their behavior is undefined otherwise.
+constexpr char inline to_lower(char a) { return a | 32; }
+constexpr bool inline is_lower(char a) { return (a & 32) > 0; }
+
+cpp::optional<cpp::StringView> inline num_to_strview(
+    uintmax_t num, cpp::MutableArrayRef<char> bufref, char conv_name) {
+  if (to_lower(conv_name) == 'x') {
+    return IntegerToString<uintmax_t, 16>::convert(num, bufref,
+                                                   is_lower(conv_name));
+  } else if (conv_name == 'o') {
+    return IntegerToString<uintmax_t, 8>::convert(num, bufref, true);
+  } else {
+    return IntegerToString<uintmax_t, 10>::convert(num, bufref, true);
+  }
+}
+
 int inline convert_int(Writer *writer, const FormatSection &to_conv) {
   static constexpr size_t BITS_IN_BYTE = 8;
   static constexpr size_t BITS_IN_NUM = sizeof(uintmax_t) * BITS_IN_BYTE;
@@ -28,24 +47,33 @@ int inline convert_int(Writer *writer, const FormatSection &to_conv) {
   bool is_negative = false;
   FormatFlags flags = to_conv.flags;
 
-  if (to_conv.conv_name == 'u') {
-    // 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));
-  } else {
+  const char a = is_lower(to_conv.conv_name) ? 'a' : 'A';
+
+  // If the conversion is signed, then handle negative values.
+  if (to_conv.conv_name == 'd' || to_conv.conv_name == 'i') {
     // Check if the number is negative by checking the high bit. This works even
     // for smaller numbers because they're sign extended by default.
     if ((num & (uintmax_t(1) << (BITS_IN_NUM - 1))) > 0) {
       is_negative = true;
       num = -num;
     }
+  } else {
+    // 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));
   }
 
   num = apply_length_modifier(num, to_conv.length_modifier);
 
-  auto const int_to_str = integer_to_string(num);
-  size_t digits_written = int_to_str.str().size();
+  static constexpr size_t BUFSIZE = IntegerToString<uintmax_t, 8>::BUFSIZE;
+  char buff[BUFSIZE];
+  cpp::MutableArrayRef<char> bufref(buff, BUFSIZE);
+  auto str = num_to_strview(num, bufref, to_conv.conv_name);
+  if (!str)
+    return INT_CONVERSION_ERROR;
+
+  size_t digits_written = str->size();
 
   char sign_char = 0;
 
@@ -56,29 +84,39 @@ int inline convert_int(Writer *writer, const FormatSection &to_conv) {
   else if ((flags & FormatFlags::SPACE_PREFIX) == FormatFlags::SPACE_PREFIX)
     sign_char = ' ';
 
-  int sign_char_len = (sign_char == 0 ? 0 : 1);
-
   // These are signed to prevent underflow due to negative values. The eventual
   // values will always be non-negative.
   int zeroes;
   int spaces;
 
+  // prefix is "0x" for hexadecimal, or the sign character for signed
+  // conversions. Since hexadecimal is unsigned these will never conflict.
+  int prefix_len;
+  char prefix[2];
+  if ((to_lower(to_conv.conv_name) == 'x') &&
+      ((flags & FormatFlags::ALTERNATE_FORM) != 0)) {
+    prefix_len = 2;
+    prefix[0] = '0';
+    prefix[1] = a + ('x' - 'a');
+  } else {
+    prefix_len = (sign_char == 0 ? 0 : 1);
+    prefix[0] = sign_char;
+  }
+
   // Negative precision indicates that it was not specified.
   if (to_conv.precision < 0) {
     if ((flags & (FormatFlags::LEADING_ZEROES | FormatFlags::LEFT_JUSTIFIED)) ==
         FormatFlags::LEADING_ZEROES) {
       // If this conv has flag 0 but not - and no specified precision, it's
       // padded with 0's instead of spaces identically to if precision =
       // min_width - (1 if sign_char). For example: ("%+04d", 1) -> "+001"
-      zeroes = to_conv.min_width - digits_written - sign_char_len;
-      if (zeroes < 0)
-        zeroes = 0;
+      zeroes = to_conv.min_width - digits_written - prefix_len;
       spaces = 0;
     } else {
       // If there are enough digits to pass over the precision, just write the
       // number, padded by spaces.
       zeroes = 0;
-      spaces = to_conv.min_width - digits_written - sign_char_len;
+      spaces = to_conv.min_width - digits_written - prefix_len;
     }
   } else {
     // If precision was specified, possibly write zeroes, and possibly write
@@ -94,33 +132,35 @@ int inline convert_int(Writer *writer, const FormatSection &to_conv) {
     zeroes = to_conv.precision - digits_written; // a negative value means 0
     if (zeroes < 0)
       zeroes = 0;
-    spaces = to_conv.min_width - zeroes - digits_written - sign_char_len;
+    spaces = to_conv.min_width - zeroes - digits_written - prefix_len;
+  }
+
+  if ((to_conv.conv_name == 'o') &&
+      ((to_conv.flags & FormatFlags::ALTERNATE_FORM) != 0) && zeroes < 1) {
+    zeroes = 1;
+    --spaces;
   }
-  if (spaces < 0)
-    spaces = 0;
 
   if ((flags & FormatFlags::LEFT_JUSTIFIED) == FormatFlags::LEFT_JUSTIFIED) {
-    // If left justified it goes sign zeroes digits spaces
-    if (sign_char != 0)
-      RET_IF_RESULT_NEGATIVE(writer->write(&sign_char, 1));
+    // If left justified it goes prefix zeroes digits spaces
+    if (prefix_len != 0)
+      RET_IF_RESULT_NEGATIVE(writer->write(prefix, prefix_len));
     if (zeroes > 0)
       RET_IF_RESULT_NEGATIVE(writer->write_chars('0', zeroes));
     if (digits_written > 0)
-      RET_IF_RESULT_NEGATIVE(
-          writer->write(int_to_str.str().data(), digits_written));
+      RET_IF_RESULT_NEGATIVE(writer->write(str->data(), digits_written));
     if (spaces > 0)
       RET_IF_RESULT_NEGATIVE(writer->write_chars(' ', spaces));
   } else {
-    // Else it goes spaces sign zeroes digits
+    // Else it goes spaces prefix zeroes digits
     if (spaces > 0)
       RET_IF_RESULT_NEGATIVE(writer->write_chars(' ', spaces));
-    if (sign_char != 0)
-      RET_IF_RESULT_NEGATIVE(writer->write(&sign_char, 1));
+    if (prefix_len != 0)
+      RET_IF_RESULT_NEGATIVE(writer->write(prefix, prefix_len));
     if (zeroes > 0)
       RET_IF_RESULT_NEGATIVE(writer->write_chars('0', zeroes));
     if (digits_written > 0)
-      RET_IF_RESULT_NEGATIVE(
-          writer->write(int_to_str.str().data(), digits_written));
+      RET_IF_RESULT_NEGATIVE(writer->write(str->data(), digits_written));
   }
   return WRITE_OK;
 }