Details
- Reviewers
sivachandra - Commits
- rG4726bec8f29b: [libc] Add implementation of fmaf.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
libc/src/math/fmaf.cpp | ||
---|---|---|
22 | Nit: Can we use z directly? | |
24 | Can you add comments explaining why this correction is required? | |
libc/utils/FPUtil/FPBits.h | ||
89 | This will not be true for long double on i386. Will replacing it with an inequality be OK? sizeof(XType) <= sizeof(UIntType) | |
libc/utils/MPFRWrapper/MPFRUtils.cpp | ||
39 | Is this used anywhere? | |
42 | Does it make sense to put these values in FPBits? | |
365 | Can you add a comment explaining why we can't use a single large precision for all types? |
libc/src/math/fmaf.cpp | ||
---|---|---|
22 | Change to use bitz directly (which is already in double) to reduce the number of float -> double conversion needed (just in case compiler does not optimize it). | |
libc/utils/FPUtil/FPBits.h | ||
89 | the (in)equality only apply when XType is of Integral type, and we were expecting XType == UIntType even when T is long double. That case was specialized before. When T is long double on i386, we still define FPBits<long double>::UIntType=__ uint128_t and the previously specialized constructor becomes FPBits<long double>(__uint128_t), so I think equality works correctly. Changing it to inequality, we will allow extra constructors such as FPBits<long double>(uint32_t), which will not be what we want. |
libc/src/math/fmaf.cpp | ||
---|---|---|
22 | What I meant was, instead of using the conversion operator, we can do something like: fputil::FPBits<double> bit_sum(static_cast<double>(bit_prod) + static_cast<double>(z)); This avoids calling FPBits conversion operator. Likewise with bit_prod may be? That is, store the product in a double value instead of FPBits object, and use it to calculate the sum. double prod = static_cast<double>(x) * static_cast<double>(y); fputil::FPBits<double> bit_prod(prod); fputil::FPBits<double> bitz(static_cast<double>(z)); fputil::FPBits<double> bit_sum(prod + static_cast<double>(z)); | |
libc/utils/FPUtil/FPBits.h | ||
89 | Ah, sorry I misread! Ignore my original comment. | |
libc/utils/MPFRWrapper/MPFRUtils.cpp | ||
365 | Did you add any comment for this somewhere? |
Nit: Can we use z directly?