Jump to conversation
Unresolved conversations (3)
@nickdesaulniers nickdesaulniers Nov 30, 2023
I'm observing the following warnings when building `ninja check-libc` with clang: ``` [286/3491] Building CXX object projects/libc/src/math/gene...es/libc.src.math.generic.nexttowardf.dir/nexttowardf.cpp.o In file included from /android0/llvm-project/libc/src/math/generic/nexttowardf.cpp:10: /android0/llvm-project/libc/src/__support/FPUtil/ManipulationFunctions.h:191:12: warning: implicit conversion loses floating-point precision: 'long double' to 'cpp::enable_if_t<cpp::is_floating_point_v<float>, float>' (aka 'float') [-Wimplicit-float-conversion] return to; ~~~~~~ ^~ /android0/llvm-project/libc/src/math/generic/nexttowardf.cpp:16:18: note: in instantiation of function template specialization '__llvm_libc_18_0_0_git::fputil::nexttoward<float>' requested here return fputil::nexttoward(x, y); ^ In file included from /android0/llvm-project/libc/src/math/generic/nexttowardf.cpp:10: /android0/llvm-project/libc/src/__support/FPUtil/ManipulationFunctions.h:194:12: warning: implicit conversion loses floating-point precision: 'long double' to 'cpp::enable_if_t<cpp::is_floating_point_v<float>, float>' (aka 'float') [-Wimplicit-float-conversion] return to; ~~~~~~ ^~ ```
.../__support/FPUtil/ManipulationFunctions.h
nickdesaulniers michaelrj-google
@lntue lntue Nov 19, 2023
For free function, I personally prefer to use SFINAE on the returning type `LIBC_INLINE cpp::enable_if_t<cpp::is_floating_point_v<T>, T> nexttoward(...)`, so that it frees the second template parameter to be used if needed.
Outdated
.../__support/FPUtil/ManipulationFunctions.h
lntue nishantwrp
@nishantwrp nishantwrp Nov 18, 2023
Should probably test the exceptions in the tests for nextafter too. Will do that in a separate pr?
.../__support/FPUtil/ManipulationFunctions.h
lntue
Resolved conversations (3)
@lntue lntue Nov 19, 2023
I think it is ok to change all of these to `-O3`.
Outdated
libc/src/math/generic/CMakeLists.txt
nishantwrp
@lntue lntue Nov 19, 2023
You should include `FEnvImpl.h` instead. It is where `raise_except_if_required` is defined.
Outdated
.../__support/FPUtil/ManipulationFunctions.h
nishantwrp
@nishantwrp nishantwrp Nov 18, 2023
Is this the correct place to reuse the `nextafter` implementation? Or should I create a new function in FpUtils for `nexttoward` and call this function there.
libc/src/math/generic/nexttowardl.cpp
lntue