[libc-commits] [PATCH] D146253: [libc] Enabled more tests for targets other than x86_64

[Tue Ly via Phabricator via libc-commits]

lntue added a comment.

In D146253#4216411 <https://reviews.llvm.org/D146253#4216411>, @mikhail.ramalho wrote:

> In D146253#4201018 <https://reviews.llvm.org/D146253#4201018>, @lntue wrote:
>
>> @mikhail.ramalho : why do you need to force the `FMA_OPT` and `ROUND_OPT` for RISC-V target?  These are to expand to build and test both with and without those instructions automatically.  Do you plan the same thing for RISC-V?  Without this, the current setup should test the trig functions with FMA and rounding instructions by default, and not testing trig functions without those instructions.
>>
>> With this change, on AArch64, AArch32, RISC-V, and GPU, each math function will have 2-4 targets, with and without `FMA_OPT` and  `ROUND_OPT` flags, and since the build system does not do anything, add any special compile options or flags, those targets are all identical.  So I don't think that's your intention?
>>
>> Do you want to test both non-FMA and FMA math implementations on RISC-V64 automatically similar to x86-64 instead?
>
> Yeah, that was my plan: I assumed that by disabling the FMA flag, the tests would automatically pick up the generic fma implementation, but checking the generated asm I saw that's not the case. I'll check how we can fix that, at least for riscv.
>
> Also, I was assuming that the ROUND_OPT flag was related to rounding modes (e.g., adding `-fno-rounding-math`), but it actually checks for SSE 4.2. May I ask what's the purpose of this flag exactly?

By default, it will pick up the fma instruction implementations if available.  This can be seen as follow (I tried this on a VisionFive2 board):

  $ cmake ../llvm -GNinja -DLLVM_ENABLE_PROJECTS=libc -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++
  ...
  $ ninja libc 
  [286/286] Linking CXX static library projects/libc/lib/libllvmlibc.a
  $ objdump -D projects/libc/src/math/generic/CMakeFiles/libc.src.math.generic.logf.dir/logf.cpp.o | grep fma
   1c4:	1230f0c3          	fmadd.d	ft1,ft1,ft3,ft2
   204:	1a207143          	fmadd.d	ft2,ft0,ft2,ft3
   208:	22207143          	fmadd.d	ft2,ft0,ft2,ft4
   20c:	2a207143          	fmadd.d	ft2,ft0,ft2,ft5
   210:	32207143          	fmadd.d	ft2,ft0,ft2,ft6
   214:	0a207043          	fmadd.d	ft0,ft0,ft2,ft1

If you also want to test the generic (non-FMA) version in the same build, similar to what we have for x86-64, then something like https://reviews.llvm.org/D146730 is what you need.
With that patch, I tested on a VisionFive2 board as follow:

  $ cmake ../llvm -GNinja -DLLVM_ENABLE_PROJECTS=libc -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++
  ...
  $ ninja libc 
  [286/286] Linking CXX static library projects/libc/lib/libllvmlibc.a
  $ objdump -D projects/libc/src/math/generic/CMakeFiles/libc.src.math.generic.logf.dir/logf.cpp.o | grep fma
   1c4:	1230f0c3          	fmadd.d	ft1,ft1,ft3,ft2
   204:	1a207143          	fmadd.d	ft2,ft0,ft2,ft3
   208:	22207143          	fmadd.d	ft2,ft0,ft2,ft4
   20c:	2a207143          	fmadd.d	ft2,ft0,ft2,ft5
   210:	32207143          	fmadd.d	ft2,ft0,ft2,ft6
   214:	0a207043          	fmadd.d	ft0,ft0,ft2,ft1
  $ ninja libc.test.src.math.logf_test.__NO_FMA_OPT
  ...
  $ objdump -D projects/libc/src/math/generic/CMakeFiles/libc.src.math.generic.logf.__NO_FMA_OPT.dir/logf.cpp.o | grep fma
  $

Notice that the `.__NO_FMA_TARGET` will also be included when running `ninja check-libc`

And the `ROUND_OPT` is for similar purpose, with using floating point rounding instructions instead of generic quick rounding functions (see `libc/src/__support/FPUtil/nearest_integer.h`).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146253/new/

https://reviews.llvm.org/D146253