Bug 462398 - x86 FMA - wrong rounding mode
Summary: x86 FMA - wrong rounding mode
Status: REPORTED
Alias: None
Product: valgrind
Classification: Developer tools
Component: vex (show other bugs)
Version: 3.19.0
Platform: unspecified Linux
: NOR normal
Target Milestone: ---
Assignee: Paul Floyd
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-11-29 15:23 UTC by jl_kdebugs@conductive.de
Modified: 2024-10-15 13:38 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
fmatest.cpp reproducer (1.12 KB, text/plain)
2022-11-29 15:23 UTC, jl_kdebugs@conductive.de
Details

Note You need to log in before you can comment on or make changes to this bug.
Description jl_kdebugs@conductive.de 2022-11-29 15:23:43 UTC
Created attachment 154139 [details]
fmatest.cpp reproducer

SUMMARY
It seems the valgrind VM will always round down for the FMA instructions.
So assuming the program didn't change the rounding mode, valgrind computes the wrong result:
0.0 + (- (0.0 * 1.0)) = -0.0
which should be 0.0 in all cases except _MM_ROUND_DOWN, which is not the default.


STEPS TO REPRODUCE
1.  Compile attached cpp with "g++ -O2 -mfma fmatest.cpp"
2.  Run natively, observe no negative numbers
3.  Run through valgrind with "valgrind --tool=none ./a.out" and observe the wrong results

OBSERVED RESULT
negative zero

EXPECTED RESULT
positive zero
Even if not implementing switching rounding modes, the most common rounding mode should be chosen
Comment 1 jl_kdebugs@conductive.de 2022-11-30 13:25:52 UTC
Action is happening inside vex `dis_FMA` and `h_generic_calc_MAddF32`
Rounding can be quite misleading but from my understanding the issue is that  `dis_FMA` will (in case of vfnmadd used here) negate the result of `h_generic_calc_MAddF32`. But for this special case (zero) this is broken/wrong since it will negate the positive zero from `h_generic_calc_MAddF32`.
The negation of the result should either be "conditional" or the MAddF32 implementation needs to be changed to propagate signs and return -0.0, which will then be changed to the correct value of +0.0.

I'm not familiar with the valgrind codebase so it's hard for me to pinpoint a fix.
Comment 2 Albin Ahlbäck 2024-10-14 04:28:12 UTC
This also came up for multiple developers in FLINT. Please see our issue at https://github.com/flintlib/flint/issues/1366.
Comment 3 Paul Floyd 2024-10-15 13:38:03 UTC
Codegen is

00000000004013e0 <_Z7naccFMAPfff>:
  4013e0:       c4 e2 71 ad 07          vfnmadd213ss (%rdi),%xmm1,%xmm0
  4013e5:       c5 fa 11 07             vmovss %xmm0,(%rdi)
  4013e9:       c3                      ret
  4013ea:       66 0f 1f 44 00 00       nopw   0x0(%rax,%rax,1)

Looking briefly at the code, it could be rounding but it could also be an error in the negation