Move math functions from src/math/generic to src/math folder.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
libc/src/math/sqrt.cpp | ||
---|---|---|
9–24 | @lntue I'm not entirely sure that the redirection to subfolder makes sense. The inlined version would look like this #include "sqrt.h" #include "src/__support/architectures.h" #include "src/__support/FPUtil/Sqrt.h" #include "src/__support/common.h" namespace __llvm_libc { LLVM_LIBC_FUNCTION(double, sqrt, (double x)) { #if defined(LLVM_LIBC_ARCH_AARCH64) double y; __asm__ __volatile__("fsqrt %d0, %d1\n\t" : "=w"(y) : "w"(x)); return y; #elif defined(LLVM_LIBC_ARCH_X86) double result; __asm__ __volatile__("sqrtsd %x1, %x0" : "=x"(result) : "x"(x)); return result; #else return fputil::sqrt(x); #endif } } // namespace __llvm_libc I find it more readable and it's less files overal. Let me know what you think |
libc/src/math/sqrt.cpp | ||
---|---|---|
9–24 | In my opinion, having #ifdef inside the function definition like this will be hard to read for more complicated functions, but for one-line functions like sqrt probably doesn't matter. It is also a bit hard to figure out which one has arch specializations, and which arch is missing, compared to separate files. Another option for separate files that I think work quite well for organizing files and readability is to use separate namespaces for separate arc, something like #if define(LLVM_LIBC_ARCH_AARCH64) LLVM_LIBC_FUNCTION(double, sqrt, (double x)) { return impl::sqrt(x); } One function that I'm optimizing is polyeval, for which by separating arch's, it is easier to manage and copy-paste than #if #else inside the real implementations. But again, I'm fine with this one-line functions such as sqrt. |
libc/src/math/sqrt.cpp | ||
---|---|---|
9–24 | SGTM, I agree that being able to know which arch is implemented just by glancing at the filesystem is valuable, also consistency is important. So I'll stick to the subfolder approach as you both of you suggested. This patch and the following ones will largely impact the build system (CMakeLists.txt and downstream) so to save me some time I will move forward with D114712 first. Once Bazel setup is in place we will be able to restructure the build system more easily (no separate downstream fixes). In the meantime, @michaelrj @lntue @sivachandra please move forward with your various pending patches, I'll rebase (or recreate) mine when I'm ready to move forward. Thank you |
@lntue I'm not entirely sure that the redirection to subfolder makes sense. The inlined version would look like this
I find it more readable and it's less files overal. Let me know what you think