Skip to content
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.

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

APFloat's fusedMultiplyAdd gives an incorrectly rounded result for IEEEfloat #104984

Open
sizn-sc opened this issue Aug 20, 2024 · 2 comments · May be fixed by #98721
Open

APFloat's fusedMultiplyAdd gives an incorrectly rounded result for IEEEfloat #104984

sizn-sc opened this issue Aug 20, 2024 · 2 comments · May be fixed by #98721

Comments

@sizn-sc
Copy link

sizn-sc commented Aug 20, 2024

While comparing APFloat against berkeley-softfloat-3e I found a discrepancy in fusedMultiplyAdd in a particular corner-case:

#include <cmath>
#include <iostream>
#include <bit>
#include <cstdint>

int main()
{
    static_assert(sizeof(float) == 4);
    auto a = 0.24999998f;
    auto b = 2.3509885e-38f;
    auto c = -1e-45f;
    auto d = std::fmaf(a, b, c);
    // Clang with optimizations folds d to 3ffffe, without optimizations 3fffff. 
    std::cout << std::hex << std::bit_cast<uint32_t>(d) << "\n";
}

Reproduction available at compiler explorer. This occurs for NearestTiesToEven and NearestTiesToAway rounding modes. Originally this issue was discovered by linking against LLVMSupport and using APFloat directly, but it also affects constant folding with default rounding mode.

GCC and native x86 FPU seem to agree with clang without optimizations.
This is likely a case of incorrect rounding and is unrelated to #63895.

(cc @eddyb @beetrees )

@beetrees
Copy link
Contributor

I've checked and this will be fixed by #98721.

@sizn-sc
Copy link
Author

sizn-sc commented Aug 21, 2024

I've also found input data for half and double precision FMA, which seemingly trigger the same bug (off by 1 ulp error):

  • Double precision - link
  • Half precision - link. Underlying bits of the folded value are 0x808d, but the correct value is 0x808e.

Since this problem is fixed by your PR, then maybe these cases can be added to regression as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants