Bug 491667 - std::fmin(val, NaN) should returns NaN on arm.
Summary: std::fmin(val, NaN) should returns NaN on arm.
Status: CONFIRMED
Alias: None
Product: valgrind
Classification: Developer tools
Component: memcheck (other bugs)
Version First Reported In: unspecified
Platform: Other Other
: NOR normal
Target Milestone: ---
Assignee: Paul Floyd
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-08-13 13:15 UTC by meirav.grimberg
Modified: 2024-08-15 05:37 UTC (History)
2 users (show)

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description meirav.grimberg 2024-08-13 13:15:11 UTC
SUMMARY

fmin(NaN,-1) returns NaN on linux arm.
According to fmin documentation:
https://en.cppreference.com/w/cpp/numeric/math/fmin
Error handling
This function is not subject to any of the error conditions specified in math_errhandling.

If the implementation supports IEEE floating-point arithmetic (IEC 60559),

* If one of the two arguments is NaN, the value of the other argument is returned.
* Only if both arguments are NaN, NaN is returned.

STEPS TO REPRODUCE
program test.cpp

``
#include <cmath>
#include <iostream>

int main()
{
    std::cout << "fmin(NaN,-1) = " << std::fmin(NAN, -1) << '\n';
}

``
 g++ test.cpp


OBSERVED RESULT

valgrind ./a.out 
==9822== Memcheck, a memory error detector
==9822== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==9822== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==9822== Command: ./a.out
==9822== 
fmin(NaN,-1) = nan

EXPECTED RESULT

./a.out 
fmin(NaN,-1) = -1

SOFTWARE/OS VERSIONS
Was tested on dockers.
arch: armV8
1. linux jammy. valgrind version 18.1
2. linux apline. valgrind version 23

ADDITIONAL INFORMATION
Comment 1 Mark Wielaard 2024-08-14 11:27:16 UTC
Confirmed on fedora 40 aarch64 with valgrind 3.23.0 and current git trunk VALGRIND_3_23_0-94-g033baf8f1

Dump of assembler code for function __fmin:
=> 0x0000fffff7f42880 <+0>:	paciasp
   0x0000fffff7f42884 <+4>:	stp	x29, x30, [sp, #-16]!
   0x0000fffff7f42888 <+8>:	fminnm	d0, d0, d1
   0x0000fffff7f4288c <+12>:	mov	x29, sp
   0x0000fffff7f42890 <+16>:	ldp	x29, x30, [sp], #16
   0x0000fffff7f42894 <+20>:	autiasp
   0x0000fffff7f42898 <+24>:	ret
End of assembler dump.
Comment 2 Mark Wielaard 2024-08-14 11:30:33 UTC
Note that VEX/priv/guest_arm64_toIR.c contains the following comment:

      /* ------- 0x,0111: FMINNM d_d, s_s ------- (FIXME KLUDGED) */
[...]
         case BITS4(0,1,1,1): nm = "fminnm"; iop = mkVecMINF(ty+2); break; //!!
Comment 3 Mark Wielaard 2024-08-14 11:33:41 UTC
And the following comment at the top:

/* KNOWN LIMITATIONS 2014-Nov-16

   * Correctness: FMAXNM, FMINNM are implemented the same as FMAX/FMIN.

     Also FP comparison "unordered" .. is implemented as normal FP
     comparison.

     Both should be fixed.  They behave incorrectly in the presence of
     NaNs.

     FMULX is treated the same as FMUL.  That's also not correct.

   * Floating multiply-add (etc) insns.  Are split into a multiply and 
     an add, and so suffer double rounding and hence sometimes the
     least significant mantissa bit is incorrect.  Fix: use the IR
     multiply-add IROps instead.           

   * FRINTX might be need updating to set the inexact computation FPSR flag
         
   * Ditto FCVTXN.  No idea what "round to odd" means.  This implementation
     just rounds to nearest. 
*/