[llvm-libc] support for 96b long double (i386)

[nickdesaulniers]

building llvm-libc for i386 fails to build in the printf code; it seems that sizeof(long double) on i386 is 12. example:

In file included from /android0/llvm-project/libc/src/stdio/printf_core/converter.cpp:9:
In file included from /android0/llvm-project/libc/src/stdio/printf_core/converter.h:13:
In file included from /android0/llvm-project/libc/src/stdio/printf_core/core_structs.h:16:
/android0/llvm-project/libc/src/__support/FPUtil/FPBits.h:824:52: error: no matching function for call to 'bit_cast'
  824 |   LIBC_INLINE constexpr T get_val() const { return cpp::bit_cast<T>(UP::bits); }
      |                                                    ^~~~~~~~~~~~~~~~
/android0/llvm-project/libc/src/stdio/printf_core/float_dec_converter.h:502:47: note: in instantiation of member function '__llvm_libc_19_0_0_git::fputil::FPBits<long double>::get_val' requested here
  502 |   FloatToString<T> float_converter(float_bits.get_val());
      |                                               ^
/android0/llvm-project/libc/src/stdio/printf_core/float_dec_converter.h:1116:14: note: in instantiation of function template specialization '__llvm_libc_19_0_0_git::printf_core::convert_float_decimal_typed<long double, 0>' requested here
 1116 |       return convert_float_decimal_typed<long double>(writer, to_conv,
      |              ^
/android0/llvm-project/libc/src/__support/CPP/bit.h:38:1: note: candidate template ignored: substitution failure [with To = long double, From = StorageType]: implicit instantiation of undefined template '__llvm_libc_19_0_0_git::cpp::enable_if<false, long double>'
   38 | bit_cast(const From &from) {
      | ^
/android0/llvm-project/libc/src/__support/big_int.h:1099:1: note: candidate template ignored: substitution failure [with To = long double, From = StorageType]: implicit instantiation of undefined template '__llvm_libc_19_0_0_git::cpp::enable_if<false, long double>'
 1099 | bit_cast(const From &from) {
      | ^
/android0/llvm-project/libc/src/__support/big_int.h:1114:1: note: candidate template ignored: substitution failure [with To = long double, Bits = 128]: implicit instantiation of undefined template '__llvm_libc_19_0_0_git::cpp::enable_if<false, long double>'
 1114 | bit_cast(const UInt<Bits> &from) {
      | ^

perhaps the StorageType for FPLayout<FPType::X86_Binary80> should be a 96b UInt for i386, but that leads to other errors?

Filing a bug to track this, as it's a blocker for i386 support.

cc @michaelrj-google

[llvmbot]

@llvm/issue-subscribers-libc

Author: Nick Desaulniers (nickdesaulniers)

building llvm-libc for i386 fails to build in the printf code; it seems that `sizeof(long double)` on i386 is `12`. example: ``` In file included from /android0/llvm-project/libc/src/stdio/printf_core/converter.cpp:9: In file included from /android0/llvm-project/libc/src/stdio/printf_core/converter.h:13: In file included from /android0/llvm-project/libc/src/stdio/printf_core/core_structs.h:16: /android0/llvm-project/libc/src/__support/FPUtil/FPBits.h:824:52: error: no matching function for call to 'bit_cast' 824 | LIBC_INLINE constexpr T get_val() const { return cpp::bit_cast<T>(UP::bits); } | ^~~~~~~~~~~~~~~~ /android0/llvm-project/libc/src/stdio/printf_core/float_dec_converter.h:502:47: note: in instantiation of member function '__llvm_libc_19_0_0_git::fputil::FPBits<long double>::get_val' requested here 502 | FloatToString<T> float_converter(float_bits.get_val()); | ^ /android0/llvm-project/libc/src/stdio/printf_core/float_dec_converter.h:1116:14: note: in instantiation of function template specialization '__llvm_libc_19_0_0_git::printf_core::convert_float_decimal_typed<long double, 0>' requested here 1116 | return convert_float_decimal_typed<long double>(writer, to_conv, | ^ /android0/llvm-project/libc/src/__support/CPP/bit.h:38:1: note: candidate template ignored: substitution failure [with To = long double, From = StorageType]: implicit instantiation of undefined template '__llvm_libc_19_0_0_git::cpp::enable_if<false, long double>' 38 | bit_cast(const From &from) { | ^ /android0/llvm-project/libc/src/__support/big_int.h:1099:1: note: candidate template ignored: substitution failure [with To = long double, From = StorageType]: implicit instantiation of undefined template '__llvm_libc_19_0_0_git::cpp::enable_if<false, long double>' 1099 | bit_cast(const From &from) { | ^ /android0/llvm-project/libc/src/__support/big_int.h:1114:1: note: candidate template ignored: substitution failure [with To = long double, Bits = 128]: implicit instantiation of undefined template '__llvm_libc_19_0_0_git::cpp::enable_if<false, long double>' 1114 | bit_cast(const UInt<Bits> &from) { | ^ ```

perhaps the StorageType for FPLayout<FPType::X86_Binary80> should be a 96b UInt for i386, but that leads to other errors?

Filing a bug to track this, as it's a blocker for i386 support.

cc @michaelrj-google

[lntue]

We will need to explicitly specialize:

template <> struct WordTypeSelector<96> : cpp::type_identity<uint32_t> {};

https://github.com/llvm/llvm-project/blob/main/libc/src/__support/big_int.h#L983
and i386 should be able to use LIBC_NAMESPACE::UInt<96> as storage type for long double.

[gchatelet]

@nickdesaulniers I'm still blocked on the syscall issues when trying to compile for i386 (i.e., CMAKE_C_FLAGS=-m32 and CMAKE_CXX_FLAGS=-m32). Can you give me a repro for the error above?

[nickdesaulniers]

I've pushed a WIP branch: https://github.com/nickdesaulniers/llvm-project/commits/i386/. The syscall implementation, and most changes aren't yet correct (I need to be able to compile for i386 to verify some of these changes) but eliminate the low hanging fruit blocking compilation.

[nickdesaulniers]

i386 should be able to use LIBC_NAMESPACE::UInt<96> as storage type for long double.

Should that be:

using StorageType = BigInt<__SIZEOF_LONG_DOUBLE__ * CHAR_BIT, false, uint32_t>;`

[lntue]

Once struct WordTypeSelector<96> is overloaded, we can just use

using StorageType = BigInt<__SIZEOF_LONG_DOUBLE__ * CHAR_BIT, false>;`

in general then? Let the default WordTypeSelector choose the underlying type (mainly for 128-bit cases).

[nickdesaulniers]

https://gist.github.com/nickdesaulniers/3c9892af61273db863fc08626ac69c75

(ah looks like jmp_buf needs a definition for i386 as well)

some of the resulting errors from:

• libc/src/__support/FPUtil/FPBits.h
• libc/src/__support/CPP/type_traits/is_unsigned.h
• libc/src/__support/big_int.h
  I don't really understand. We do have the binary operators defined for BigInt.

[lntue]

Once struct WordTypeSelector<96> is overloaded, we can just use

using StorageType = BigInt<__SIZEOF_LONG_DOUBLE__ * CHAR_BIT, false>;`

in general then? Let the default WordTypeSelector choose the underlying type (mainly for 128-bit cases).

Probably the correct one is:

using StorageType = UInt<__SIZEOF_LONG_DOUBLE__ * CHAR_BIT>;

[gchatelet]

I synced to nickdesaulniers@0d46082 and tried to compile atof.

Here is the log (a bit redacted) it's quite different from the errors I could spot in your gist #110894 (comment). I'll work on fixing them and report back.

ninja: Entering directory `/home/gchatelet/build/libc_x86'
[1/1] Building CXX object projects/libc/src/stdlib/CMakeFiles/libc.src.stdlib.atof.dir/atof.cpp.o
FAILED: projects/libc/src/stdlib/CMakeFiles/libc.src.stdlib.atof.dir/atof.cpp.o 
/usr/bin/clang++ -DLIBC_NAMESPACE=__llvm_libc_20_0_0_git -I/home/gchatelet/build/libc_x86/projects/libc/src/stdlib -I/home/gchatelet/git/llvm-project/libc/src/stdlib -I/home/gchatelet/git/llvm-project/libc -isystem /home/gchatelet/build/libc_x86/projects/libc/include -m32 -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG -std=c++17 -DLIBC_QSORT_IMPL=LIBC_QSORT_QUICK_SORT -DLIBC_ADD_NULL_CHECKS -fpie -fno-builtin -fno-exceptions -fno-lax-vector-conversions -fno-unwind-tables -fno-asynchronous-unwind-tables -fno-rtti -ftrivial-auto-var-init=pattern -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -Wall -Wextra -Werror -Wconversion -Wno-sign-conversion -Wimplicit-fallthrough -Wwrite-strings -Wextra-semi -Wnewline-eof -Wnonportable-system-include-path -Wstrict-prototypes -Wthread-safety -Wglobal-constructors -DLIBC_COPT_PUBLIC_PACKAGING -MD -MT projects/libc/src/stdlib/CMakeFiles/libc.src.stdlib.atof.dir/atof.cpp.o -MF projects/libc/src/stdlib/CMakeFiles/libc.src.stdlib.atof.dir/atof.cpp.o.d -o projects/libc/src/stdlib/CMakeFiles/libc.src.stdlib.atof.dir/atof.cpp.o -c /home/gchatelet/git/llvm-project/libc/src/stdlib/atof.cpp
In file included from /home/gchatelet/git/llvm-project/libc/src/stdlib/atof.cpp:12:
/home/gchatelet/git/llvm-project/libc/src/__support/str_to_float.h:223:47: error: no matching function for call to 'high64'
  UInt128 approx_upper = static_cast<UInt128>(high64(mantissa)) *
                                              ^~~~~~
/home/gchatelet/git/llvm-project/libc/src/__support/str_to_float.h:49:22: note: candidate function not viable: no known conversion from 'BigInt<12 * 8 aka 96, [...], uint32_t>' to 'const BigInt<128U aka 128, [...], internal::WordTypeSelectorT<128U>>' for 1st argument
LIBC_INLINE uint64_t high64(const UInt128 &num) {
                     ^
/home/gchatelet/git/llvm-project/libc/src/__support/str_to_float.h:226:50: error: no matching function for call to 'high64'
  UInt128 approx_middle_a = static_cast<UInt128>(high64(mantissa)) *
                                                 ^~~~~~
/home/gchatelet/git/llvm-project/libc/src/__support/str_to_float.h:49:22: note: candidate function not viable: no known conversion from 'BigInt<12 * 8 aka 96, [...], uint32_t>' to 'const BigInt<128U aka 128, [...], internal::WordTypeSelectorT<128U>>' for 1st argument
LIBC_INLINE uint64_t high64(const UInt128 &num) {
                     ^
/home/gchatelet/git/llvm-project/libc/src/__support/str_to_float.h:228:50: error: no matching function for call to 'low64'
  UInt128 approx_middle_b = static_cast<UInt128>(low64(mantissa)) *
                                                 ^~~~~
/home/gchatelet/git/llvm-project/libc/src/__support/str_to_float.h:45:22: note: candidate function not viable: no known conversion from 'BigInt<12 * 8 aka 96, [...], uint32_t>' to 'const BigInt<128U aka 128, [...], internal::WordTypeSelectorT<128U>>' for 1st argument
LIBC_INLINE uint64_t low64(const UInt128 &num) {
                     ^
/home/gchatelet/git/llvm-project/libc/src/__support/str_to_float.h:236:47: error: no matching function for call to 'low64'
  UInt128 approx_lower = static_cast<UInt128>(low64(mantissa)) *
                                              ^~~~~
/home/gchatelet/git/llvm-project/libc/src/__support/str_to_float.h:45:22: note: candidate function not viable: no known conversion from 'BigInt<12 * 8 aka 96, [...], uint32_t>' to 'const BigInt<128U aka 128, [...], internal::WordTypeSelectorT<128U>>' for 1st argument
LIBC_INLINE uint64_t low64(const UInt128 &num) {
                     ^
/home/gchatelet/git/llvm-project/libc/src/__support/str_to_float.h:253:26: error: invalid operands to binary expression ('UInt128' (aka 'BigInt<128U, false, unsigned long long>') and 'StorageType' (aka 'BigInt<12 * 8, false, unsigned int>'))
      final_approx_lower + mantissa < mantissa) {
      ~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~
/home/gchatelet/git/llvm-project/libc/src/__support/big_int.h:470:32: note: candidate function not viable: no known conversion from 'BigInt<12 * 8 aka 96, [...], uint32_t>' to 'const BigInt<128, [...], unsigned long long>' for 1st argument
  LIBC_INLINE constexpr BigInt operator+(const BigInt &other) const {
                               ^
/home/gchatelet/git/llvm-project/libc/src/__support/big_int.h:478:32: note: candidate function not viable: no known conversion from 'BigInt<12 * 8 aka 96, [...], uint32_t>' to 'BigInt<128, [...], unsigned long long>' for 1st argument
  LIBC_INLINE constexpr BigInt operator+(BigInt &&other) const {
                               ^
In file included from /home/gchatelet/git/llvm-project/libc/src/stdlib/atof.cpp:12:
/home/gchatelet/git/llvm-project/libc/src/__support/str_to_float.h:260:15: error: no viable conversion from 'BigInt<128, [...], unsigned long long>' to 'BigInt<12 * 8 aka 96, [...], uint32_t>'
  StorageType final_mantissa =
              ^
/home/gchatelet/git/llvm-project/libc/src/__support/big_int.h:362:25: note: candidate constructor not viable: no known conversion from 'BigInt<128, false>' to 'const BigInt<96, false, unsigned int> &' for 1st argument
  LIBC_INLINE constexpr BigInt(const BigInt &other) = default;
                        ^
/home/gchatelet/git/llvm-project/libc/src/__support/big_int.h:365:25: note: candidate template ignored: could not match 'unsigned int' against 'unsigned long long'
  LIBC_INLINE constexpr BigInt(
                        ^
/home/gchatelet/git/llvm-project/libc/src/__support/big_int.h:379:45: note: candidate template ignored: could not match 'unsigned int[N]' against 'BigInt<128, false>'
  template <size_t N> LIBC_INLINE constexpr BigInt(const WordType (&nums)[N]) {
                                            ^
/home/gchatelet/git/llvm-project/libc/src/__support/big_int.h:393:25: note: candidate template ignored: substitution failure [with T = BigInt<128, false>]: implicit instantiation of undefined template '__llvm_libc_20_0_0_git::cpp::enable_if<false>'
  LIBC_INLINE constexpr BigInt(T v) {
                        ^
/home/gchatelet/git/llvm-project/libc/src/__support/big_int.h:385:34: note: explicit constructor is not a candidate
  LIBC_INLINE constexpr explicit BigInt(
                                 ^
/home/gchatelet/git/llvm-project/libc/src/__support/big_int.h:430:56: note: explicit conversion function is not a candidate
  template <typename T> LIBC_INLINE constexpr explicit operator T() const {

[gchatelet]

So ultimately it has to do with how eisel_lemire is implemented. Currently it assumes that long double StorageType is 64 or 128 bits. @michaelrj-google can you give it a try?

[michaelrj-google]

I'm happy to take a look, what cmake command are you using to build for i386?

[gchatelet]

I've been using the following cmd lines

> git checkout nickdesaulniers/i386
> cmake -DBUILD_SHARED_LIBS=OFF -DCMAKE_BUILD_TYPE="Release" -DCMAKE_CXX_COMPILER="/usr/bin/clang++" -DCMAKE_CXX_FLAGS="-m32" -DCMAKE_C_COMPILER="/usr/bin/clang" -DCMAKE_C_FLAGS="-m32" -DLLVM_ENABLE_PROJECTS="libc" -S/path/to/home/git/llvm-project/llvm -B/path/to/home/build/libc_x86 -GNinja
> ninja -C /path/to/home/build/libc_x86 libc.src.stdlib.atof

[nickdesaulniers]

I'm just setting -DLIBC_TARGET_TRIPLE=i386-linux-gnu

[nickdesaulniers]

Thanks @michaelrj-google for those patches! The next issue I run into is that the StorageType for FPLayout<FPType::X86_Binary80> currently is UInt128, but I suspect it should be a UInt96 (or rather UInt<__SIZEOF_LONG_DOUBLE__* CHAR_BIT>) (for long double on i386). This seems to blow up in eisel_lemire (str_to_float.h) for !defined(LIBC_TYPES_LONG_DOUBLE_IS_FLOAT64). The intermediary calculations are done on UInt128, but the mantissa is the storage type of UInt<96>. That causes comparisons using binary operators to fail. Should we be able to compare via bin ops two different kinds of BigInts, or should the intermediaries values in eisel_lemire be changed?

ninja libc.src.stdio.scanf_core.converter is a good target to build that exposes this.

diff --git a/libc/src/__support/FPUtil/FPBits.h b/libc/src/__support/FPUtil/FPBits.h
index 5d1f633bb56e..b87ef6281051 100644
--- a/libc/src/__support/FPUtil/FPBits.h
+++ b/libc/src/__support/FPUtil/FPBits.h
@@ -121,7 +121,7 @@ template <> struct FPLayout<FPType::IEEE754_Binary128> {
 };
 
 template <> struct FPLayout<FPType::X86_Binary80> {
-  using StorageType = UInt128;
+  using StorageType = UInt<__SIZEOF_LONG_DOUBLE__* CHAR_BIT>;
   LIBC_INLINE_VAR static constexpr int SIGN_LEN = 1;
   LIBC_INLINE_VAR static constexpr int EXP_LEN = 15;
   LIBC_INLINE_VAR static constexpr int SIG_LEN = 64;
diff --git a/libc/src/__support/big_int.h b/libc/src/__support/big_int.h
index 246b89f08f2f..4ec57b8d4f98 100644
--- a/libc/src/__support/big_int.h
+++ b/libc/src/__support/big_int.h
@@ -1055,9 +1055,10 @@ struct WordTypeSelector : cpp::type_identity<
 #endif // LIBC_TYPES_HAS_INT64
                               > {
 };
-// Except if we request 16 or 32 bits explicitly.
+// Except if we request 16, 32, or 96 bits explicitly.
 template <> struct WordTypeSelector<16> : cpp::type_identity<uint16_t> {};
 template <> struct WordTypeSelector<32> : cpp::type_identity<uint32_t> {};
+template <> struct WordTypeSelector<96> : cpp::type_identity<uint32_t> {};
 template <size_t Bits>
 using WordTypeSelectorT = typename WordTypeSelector<Bits>::type;
 } // namespace internal