Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc] i386 support #93709

Open
nickdesaulniers opened this issue May 29, 2024 · 7 comments
Open

[libc] i386 support #93709

nickdesaulniers opened this issue May 29, 2024 · 7 comments
Assignees

Comments

@nickdesaulniers
Copy link
Member

In order to incorporate llvm-libc into Android, we MUST to support 32b x86.

Currently, the most immediate build failure with -m32 is:

In file included from external/llvm-libc/test/UnitTest/LibcTest.cpp:9:
In file included from external/llvm-libc/test/UnitTest/LibcTest.h:28:
In file included from external/llvm-libc/src/__support/CPP/string.h:14:
In file included from external/llvm-libc/src/string/memory_utils/inline_memcpy.h:22:
external/llvm-libc/src/string/memory_utils/x86_64/inline_memcpy.h:195:22: error: no member named 'K_AVX' in namespace '
llvmlibc::x86'
  195 |   if constexpr (x86::K_AVX) {
      |                 ~~~~~^
external/llvm-libc/src/string/memory_utils/x86_64/inline_memcpy.h:214:17: error: no member named 'Memcpy' in namespace 
'llvmlibc::x86'
  214 |     return x86::Memcpy::repmovsb(dst, src, count);
      |            ~~~~~^
external/llvm-libc/src/string/memory_utils/x86_64/inline_memcpy.h:219:19: error: no member named 'Memcpy' in namespace 
'llvmlibc::x86'
  219 |       return x86::Memcpy::repmovsb(dst, src, count);
      |              ~~~~~^

This is because x86::K_AVX is only defined when LIBC_TARGET_ARCH_IS_X86_64 is, which is 64b only. We probably need more preprocessor guards using LIBC_TARGET_ARCH_IS_X86_64 (64b vs 32b).

(There may be more build or runtime failures)

cc @gchatelet @enh-google

It will be less work for us to support 32b x86 then for Android to drop 32b x86 support.

@llvmbot
Copy link
Member

llvmbot commented May 29, 2024

@llvm/issue-subscribers-libc

Author: Nick Desaulniers (paternity leave) (nickdesaulniers)

In order to incorporate llvm-libc into Android, we MUST to support 32b x86.

Currently, the most immediate build failure with -m32 is:

In file included from external/llvm-libc/test/UnitTest/LibcTest.cpp:9:
In file included from external/llvm-libc/test/UnitTest/LibcTest.h:28:
In file included from external/llvm-libc/src/__support/CPP/string.h:14:
In file included from external/llvm-libc/src/string/memory_utils/inline_memcpy.h:22:
external/llvm-libc/src/string/memory_utils/x86_64/inline_memcpy.h:195:22: error: no member named 'K_AVX' in namespace '
llvmlibc::x86'
  195 |   if constexpr (x86::K_AVX) {
      |                 ~~~~~^
external/llvm-libc/src/string/memory_utils/x86_64/inline_memcpy.h:214:17: error: no member named 'Memcpy' in namespace 
'llvmlibc::x86'
  214 |     return x86::Memcpy::repmovsb(dst, src, count);
      |            ~~~~~^
external/llvm-libc/src/string/memory_utils/x86_64/inline_memcpy.h:219:19: error: no member named 'Memcpy' in namespace 
'llvmlibc::x86'
  219 |       return x86::Memcpy::repmovsb(dst, src, count);
      |              ~~~~~^

This is because x86::K_AVX is only defined when LIBC_TARGET_ARCH_IS_X86_64 is, which is 64b only. We probably need more preprocessor guards using LIBC_TARGET_ARCH_IS_X86_64 (64b vs 32b).

(There may be more build or runtime failures)

cc @gchatelet @enh-google

It will be less work for us to support 32b x86 then for Android to drop 32b x86 support.

@enh-google
Copy link
Contributor

In order to incorporate llvm-libc into Android, we MUST to support 32b x86.

is that true? the example you show is a test suite dependency, not an actual libc one...

@gchatelet
Copy link
Contributor

In order to incorporate llvm-libc into Android, we MUST to support 32b x86.

is that true? the example you show is a test suite dependency, not an actual libc one...

The example above is LLVM libc failing to build with -m32. It may seem that the error is coming from LibcTest (which is our equivalent of Google Test) but the failure comes from inline_memcpy.h which is used to implement memcpy. So, yes, LLVM libc is currently not building with -m32. As @nickdesaulniers said though, it's just a macro definition away from working (as far as this error is concerned). I'll prepare a fix.

@gchatelet
Copy link
Contributor

Looking at the remaining places where we have reference to LIBC_TARGET_ARCH_IS_X86_64 instead of LIBC_TARGET_ARCH_IS_X86 (not counting legitimate uses, #93791 and #93790)

% git grep -n LIBC_TARGET_ARCH_IS_X86_64
config/linux/app.h:38:#if defined(LIBC_TARGET_ARCH_IS_X86_64) ||                                     \
src/__support/OSUtil/linux/syscall.h:16:#ifdef LIBC_TARGET_ARCH_IS_X86_64
src/__support/threads/linux/thread.cpp:61:#elif defined(LIBC_TARGET_ARCH_IS_X86_64)
src/__support/threads/linux/thread.cpp:159:#ifdef LIBC_TARGET_ARCH_IS_X86_64
src/__support/threads/linux/thread.cpp:298:#if defined(LIBC_TARGET_ARCH_IS_X86_64)
src/__support/threads/thread.h:43:     defined(LIBC_TARGET_ARCH_IS_X86_64) ||                                    \
src/setjmp/x86_64/longjmp.cpp:12:#if !defined(LIBC_TARGET_ARCH_IS_X86_64)
src/setjmp/x86_64/setjmp.cpp:12:#if !defined(LIBC_TARGET_ARCH_IS_X86_64)
src/threads/linux/Futex.h:17:     defined(LIBC_TARGET_ARCH_IS_X86_64))
test/src/sys/utsname/uname_test.cpp:21:#ifdef LIBC_TARGET_ARCH_IS_X86_64

It seems we have additional work to do around syscall, setjmp, threading, TLSImage (config/linux/app.h).

gchatelet added a commit that referenced this issue May 30, 2024
@nickdesaulniers
Copy link
Member Author

It seems we have additional work to do around syscall,

Indeed, with #93790 applied, the next issue that I encounter is:

In file included from external/llvm-libc/test/UnitTest/TestLogger.cpp:4:
In file included from external/llvm-libc/src/__support/OSUtil/io.h:19:
In file included from external/llvm-libc/src/__support/OSUtil/linux/io.h:13:
external/llvm-libc/src/__support/OSUtil/linux/syscall.h:31:37: error: no matching function for call to 'syscall_impl'
   31 |   return cpp::bit_or_static_cast<R>(syscall_impl(__number, (long)ts...));
      |                                     ^~~~~~~~~~~~
external/llvm-libc/src/__support/OSUtil/linux/io.h:20:19: note: in instantiation of function template specialization 'llvmlibc::syscall_impl<long, int, const char *, unsigned int>' requested here                                       
   20 |   LIBC_NAMESPACE::syscall_impl<long>(SYS_write, 2 /* stderr */, msg.data(),
      |                   ^
external/llvm-libc/src/__support/OSUtil/linux/syscall.h:29:15: note: candidate template ignored: couldn't infer template argument 'R'                                                                                                     
   29 | LIBC_INLINE R syscall_impl(long __number, Ts... ts) {
      |               ^

gchatelet added a commit that referenced this issue May 31, 2024
@nickdesaulniers
Copy link
Member Author

blocker: #110894

@nickdesaulniers
Copy link
Member Author

nickdesaulniers commented Oct 2, 2024

Going through some of the issues now:

@nickdesaulniers nickdesaulniers self-assigned this Oct 2, 2024
nickdesaulniers added a commit to nickdesaulniers/llvm-project that referenced this issue Oct 15, 2024
nickdesaulniers added a commit to nickdesaulniers/llvm-project that referenced this issue Oct 30, 2024
nickdesaulniers added a commit to nickdesaulniers/llvm-project that referenced this issue Oct 30, 2024
nickdesaulniers added a commit to nickdesaulniers/llvm-project that referenced this issue Oct 30, 2024
nickdesaulniers added a commit that referenced this issue Oct 30, 2024
nickdesaulniers added a commit that referenced this issue Oct 31, 2024
nickdesaulniers added a commit to nickdesaulniers/llvm-project that referenced this issue Oct 31, 2024
Configured via:

    $ cmake ../runtimes -G Ninja -DLLVM_ENABLE_LLD=ON \
      -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_RUNTIMES="libc" \
      -DLIBC_TARGET_TRIPLE=i386-linux-gnu -DCMAKE_C_COMPILER=clang \
      -DCMAKE_CXX_COMPILER=clang++

Link: llvm#93709
nickdesaulniers added a commit to nickdesaulniers/llvm-project that referenced this issue Nov 1, 2024
Configured via:

    $ cmake ../runtimes -G Ninja -DLLVM_ENABLE_LLD=ON \
      -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_RUNTIMES="libc" \
      -DLIBC_TARGET_TRIPLE=i386-linux-gnu -DCMAKE_C_COMPILER=clang \
      -DCMAKE_CXX_COMPILER=clang++

Link: llvm#93709
nickdesaulniers added a commit that referenced this issue Nov 1, 2024
Configured via:

    $ cmake ../runtimes -G Ninja -DLLVM_ENABLE_LLD=ON \
      -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_RUNTIMES="libc" \
      -DLIBC_TARGET_TRIPLE=i386-linux-gnu -DCMAKE_C_COMPILER=clang \
      -DCMAKE_CXX_COMPILER=clang++

Link: #93709
smallp-o-p pushed a commit to smallp-o-p/llvm-project that referenced this issue Nov 3, 2024
smallp-o-p pushed a commit to smallp-o-p/llvm-project that referenced this issue Nov 3, 2024
smallp-o-p pushed a commit to smallp-o-p/llvm-project that referenced this issue Nov 3, 2024
smallp-o-p pushed a commit to smallp-o-p/llvm-project that referenced this issue Nov 3, 2024
Configured via:

    $ cmake ../runtimes -G Ninja -DLLVM_ENABLE_LLD=ON \
      -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_RUNTIMES="libc" \
      -DLIBC_TARGET_TRIPLE=i386-linux-gnu -DCMAKE_C_COMPILER=clang \
      -DCMAKE_CXX_COMPILER=clang++

Link: llvm#93709
smallp-o-p pushed a commit to smallp-o-p/llvm-project that referenced this issue Nov 3, 2024
smallp-o-p pushed a commit to smallp-o-p/llvm-project that referenced this issue Nov 3, 2024
Configured via:

    $ cmake ../runtimes -G Ninja -DLLVM_ENABLE_LLD=ON \
      -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_RUNTIMES="libc" \
      -DLIBC_TARGET_TRIPLE=i386-linux-gnu -DCMAKE_C_COMPILER=clang \
      -DCMAKE_CXX_COMPILER=clang++

Link: llvm#93709
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this issue Nov 4, 2024
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this issue Nov 4, 2024
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this issue Nov 4, 2024
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this issue Nov 4, 2024
Configured via:

    $ cmake ../runtimes -G Ninja -DLLVM_ENABLE_LLD=ON \
      -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_RUNTIMES="libc" \
      -DLIBC_TARGET_TRIPLE=i386-linux-gnu -DCMAKE_C_COMPILER=clang \
      -DCMAKE_CXX_COMPILER=clang++

Link: llvm#93709
nickdesaulniers added a commit to nickdesaulniers/llvm-project that referenced this issue Nov 5, 2024
`long double` is haunted on most architectures, but it is especially so on
i386-linux-gnu. While have 80b of significant data, on i386-linux-gnu this type
has 96b of storage.

Fixes for supporting printf family of conversions for `long double` on
i386-linux-gnu. This allows the libc-stdlib-tests and libc_stdio_unittests
ninja target tests to pass on i386-linux-gnu.

Fixes: llvm#110894
Link: llvm#93709
Link: https://developer.android.com/ndk/guides/abis
Co-authored-by: Michael Jones <michaelrj@google.com>
nickdesaulniers added a commit to nickdesaulniers/llvm-project that referenced this issue Nov 7, 2024
`long double` is haunted on most architectures, but it is especially so on
i386-linux-gnu. While have 80b of significant data, on i386-linux-gnu this type
has 96b of storage.

Fixes for supporting printf family of conversions for `long double` on
i386-linux-gnu. This allows the libc-stdlib-tests and libc_stdio_unittests
ninja target tests to pass on i386-linux-gnu.

Fixes: llvm#110894
Link: llvm#93709
Link: https://developer.android.com/ndk/guides/abis
Co-authored-by: Michael Jones <michaelrj@google.com>
nickdesaulniers added a commit that referenced this issue Nov 12, 2024
`long double` is haunted on most architectures, but it is especially so on
i386-linux-gnu. While have 80b of significant data, on i386-linux-gnu this type
has 96b of storage.

Fixes for supporting printf family of conversions for `long double` on
i386-linux-gnu. This allows the libc-stdlib-tests and libc_stdio_unittests
ninja target tests to pass on i386-linux-gnu.

Fixes: #110894
Link: #93709
Co-authored-by: Michael Jones <michaelrj@google.com>
nickdesaulniers added a commit to nickdesaulniers/llvm-project that referenced this issue Jan 28, 2025
`long double` is haunted on most architectures, but it is especially so on
i386-linux-gnu. While have 80b of significant data, on i386-linux-gnu this type
has 96b of storage.

Fixes for supporting printf family of conversions for `long double` on
i386-linux-gnu. This allows the libc-stdlib-tests and libc_stdio_unittests
ninja target tests to pass on i386-linux-gnu.

Fixes: llvm#110894
Link: llvm#93709
Link: https://developer.android.com/ndk/guides/abis
Co-authored-by: Michael Jones <michaelrj@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants