Skip to content

[libc] Disabled mpfr tests in full build mode, even if we set the mpfr path #149655

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

[libc] Disabled mpfr tests in full build mode, even if we set the mpfr path #149655

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions libc/cmake/modules/LLVMLibCCheckMPFR.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
set(LLVM_LIBC_MPFR_INSTALL_PATH "" CACHE PATH "Path to where MPFR is installed (e.g. C:/src/install or ~/src/install)")

if(LLVM_LIBC_MPFR_INSTALL_PATH)
set(LIBC_TESTS_CAN_USE_MPFR TRUE)
elseif(LIBC_TARGET_OS_IS_GPU OR LLVM_LIBC_FULL_BUILD)
if(LIBC_TARGET_OS_IS_GPU OR LLVM_LIBC_FULL_BUILD)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will also disable MPFR tests in full build for other architectures?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, If you set -DLLVM_LIBC_FULL_BUILD=ON, it will already disable mpfr tests, unless you explicitly set LLVM_LIBC_MPFR_INSTALL_PATH.

I double-checked all buildbots and the only one that was doing a full build and setting the mpfr path was the rv32 buildbot: https://github.com/llvm/llvm-zorg/blob/6fcd3566a6c9665f70f0bf3f366a148c38787a25/zorg/buildbot/builders/annotated/libc-linux.py#L112

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we update the RV32 bot command instead? It was intentional to so that mpfr tests can be run in full build mode when needed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can, but the way the code is today you won't be able to build in full build mode and set the mpfr path at the same time in any arch, or it will fail with the same error we are seeing on rv32 now.

This is my log from trying to build on x86, with the following cmake command from the llvm libc website:

$ cmake ../runtimes \
   -G Ninja \
   -DCMAKE_C_COMPILER=clang \
   -DCMAKE_CXX_COMPILER=clang++ \
   -DLLVM_ENABLE_RUNTIMES="libc;compiler-rt" \
   -DLLVM_LIBC_FULL_BUILD=ON \
   -DCMAKE_BUILD_TYPE=Debug \
   -DLLVM_LIBC_INCLUDE_SCUDO=OFF \
   -DCOMPILER_RT_BUILD_SCUDO_STANDALONE_WITH_LLVM_LIBC=OFF \
   -DCOMPILER_RT_BUILD_GWP_ASAN=OFF                       \
   -DCOMPILER_RT_SCUDO_STANDALONE_BUILD_SHARED=OFF        \
   -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
   -DLLVM_ENABLE_SPHINX=OFF -DLIBC_INCLUDE_DOCS=OFF \
   -DLIBC_CMAKE_VERBOSE_LOGGING=ON \
   -DLLVM_LIBC_MPFR_INSTALL_PATH=/usr/` # set mpfr path 

$ ninja libc.test.src.__support.FPUtil.bfloat16_test.__unit__
[66/75] Building CXX object libc/test/src/__support/FPUtil/CMakeFiles/libc.test.src.__support.FPUtil.bfloat16_test.__unit__.__build__.dir/bfloat16_test.cpp.o
FAILED: libc/test/src/__support/FPUtil/CMakeFiles/libc.test.src.__support.FPUtil.bfloat16_test.__unit__.__build__.dir/bfloat16_test.cpp.o 
/home/mgadelha/tools/llvm-project/build/bin/clang++ -DLIBC_NAMESPACE=__llvm_libc_22_0_0_git -D_DEBUG -I/home/mgadelha/tools/llvm-project/libc -isystem /home/mgadelha/tools/llvm-project/buildlibc/libc/include -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -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 -g -std=gnu++17 -mavx2 -mfma -DLIBC_QSORT_IMPL=LIBC_QSORT_QUICK_SORT -DLIBC_ADD_NULL_CHECKS -DLIBC_ERRNO_MODE=LIBC_ERRNO_MODE_DEFAULT -fpie -DLIBC_FULL_BUILD -ffreestanding -fno-exceptions -fno-unwind-tables -fno-asynchronous-unwind-tables -fno-rtti -ffixed-point -Wconversion -Wno-sign-conversion -Wimplicit-fallthrough -Wwrite-strings -Wno-c99-extensions -Wno-gnu-imaginary-constant -Wno-pedantic -Wstrict-prototypes -Wextra-semi -Wnewline-eof -Wnonportable-system-include-path -Wthread-safety -MD -MT libc/test/src/__support/FPUtil/CMakeFiles/libc.test.src.__support.FPUtil.bfloat16_test.__unit__.__build__.dir/bfloat16_test.cpp.o -MF libc/test/src/__support/FPUtil/CMakeFiles/libc.test.src.__support.FPUtil.bfloat16_test.__unit__.__build__.dir/bfloat16_test.cpp.o.d -o libc/test/src/__support/FPUtil/CMakeFiles/libc.test.src.__support.FPUtil.bfloat16_test.__unit__.__build__.dir/bfloat16_test.cpp.o -c /home/mgadelha/tools/llvm-project/libc/test/src/__support/FPUtil/bfloat16_test.cpp
In file included from /home/mgadelha/tools/llvm-project/libc/test/src/__support/FPUtil/bfloat16_test.cpp:12:
In file included from /home/mgadelha/tools/llvm-project/libc/utils/MPFRWrapper/MPCommon.h:21:
In file included from /home/mgadelha/tools/llvm-project/libc/utils/MPFRWrapper/mpfr_inc.h:20:
In file included from /usr/include/mpfr.h:53:
In file included from /usr/include/x86_64-linux-gnu/gmp.h:35:
In file included from /usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/iosfwd:38:
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/requires_hosted.h:34:4: error: "This header is not available in freestanding mode."
   34 | #  error "This header is not available in freestanding mode."
      |    ^
In file included from /home/mgadelha/tools/llvm-project/libc/test/src/__support/FPUtil/bfloat16_test.cpp:12:
In file included from /home/mgadelha/tools/llvm-project/libc/utils/MPFRWrapper/MPCommon.h:21:
In file included from /home/mgadelha/tools/llvm-project/libc/utils/MPFRWrapper/mpfr_inc.h:20:
In file included from /usr/include/mpfr.h:53:
In file included from /usr/include/x86_64-linux-gnu/gmp.h:36:
In file included from /usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/cstdio:42:
In file included from /home/mgadelha/tools/llvm-project/buildlibc/libc/include/stdio.h:14:
In file included from /home/mgadelha/tools/llvm-project/buildlibc/libc/include/llvm-libc-macros/stdio-macros.h:12:
/home/mgadelha/tools/llvm-project/buildlibc/libc/include/llvm-libc-macros/../llvm-libc-types/FILE.h:12:16: error: typedef 'FILE' cannot be referenced with the 'struct' specifier
   12 | typedef struct FILE FILE;
      |                ^
/usr/include/x86_64-linux-gnu/bits/types/FILE.h:7:25: note: declared here
    7 | typedef struct _IO_FILE FILE;
      |                         ^
In file included from /home/mgadelha/tools/llvm-project/libc/test/src/__support/FPUtil/bfloat16_test.cpp:12:
In file included from /home/mgadelha/tools/llvm-project/libc/utils/MPFRWrapper/MPCommon.h:21:
In file included from /home/mgadelha/tools/llvm-project/libc/utils/MPFRWrapper/mpfr_inc.h:20:
In file included from /usr/include/mpfr.h:53:
In file included from /usr/include/x86_64-linux-gnu/gmp.h:36:
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/cstdio:99:11: error: no member named 'fpos_t' in the global namespace
   99 |   using ::fpos_t;
      |         ~~^
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/cstdio:107:11: error: no member named 'fgetpos' in the global namespace
  107 |   using ::fgetpos;
      |         ~~^
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/cstdio:114:11: error: no member named 'freopen' in the global namespace
  114 |   using ::freopen;
      |         ~~^
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/cstdio:117:11: error: no member named 'fsetpos' in the global namespace
  117 |   using ::fsetpos;
      |         ~~^
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/cstdio:133:11: error: no member named 'rewind' in the global namespace
  133 |   using ::rewind;
      |         ~~^
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/cstdio:139:11: error: no member named 'tmpfile' in the global namespace
  139 |   using ::tmpfile;
      |         ~~^
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/cstdio:141:11: error: no member named 'tmpnam' in the global namespace
  141 |   using ::tmpnam;
      |         ~~^
In file included from /home/mgadelha/tools/llvm-project/libc/test/src/__support/FPUtil/bfloat16_test.cpp:12:
/home/mgadelha/tools/llvm-project/libc/utils/MPFRWrapper/MPCommon.h:174:5: error: use of undeclared identifier 'mpfr_set_sj'
  174 |     mpfr_set_sj(value, x, mpfr_rounding);
      |     ^~~~~~~~~~~
/home/mgadelha/tools/llvm-project/libc/test/src/__support/FPUtil/bfloat16_test.cpp:65:28: note: in instantiation of function template specialization '__llvm_libc_22_0_0_git::testing::mpfr::MPFRNumber::MPFRNumber<int, 0>' requested here
   65 |     BFloat16 mpfr_bfloat = MPFRNumber(i).as<BFloat16>();
      |                            ^
10 errors generated.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another alternative would be to change bfloat16_test.cpp so it doesn't use mpfr, but I think this should be a bit more complicated.

How do you want me to proceed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you temporarily disable bfloat16_test for full build mode? It is the MPCommon target that needs to be updated so that mpfr_inc.h is not included in the MPCommon.h header.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'll submit a different PR for that!

# In full build mode, the MPFR library should be built using our own facilities,
# which is currently not possible.
set(LIBC_TESTS_CAN_USE_MPFR FALSE)
elseif(LLVM_LIBC_MPFR_INSTALL_PATH)
set(LIBC_TESTS_CAN_USE_MPFR TRUE)
else()
try_compile(
LIBC_TESTS_CAN_USE_MPFR
Expand Down
Loading