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: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-11-29 15:23 UTC by jl_kdebugs@conductive.de
Modified: 2022-12-05 10:04 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


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.