The float??_minnum implementation differs from Hexagon for SNaN,
it returns NaN, but Hexagon returns the other input.  So, we use
float??_minimum_number.  For double precision, we check for QNaN and
raise the invalid flag.

test cases added in a subsequent patch to more extensively test USR bits

Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
---
 target/hexagon/op_helper.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/target/hexagon/op_helper.c b/target/hexagon/op_helper.c
index 057baf9a48..daf0b0d348 100644
--- a/target/hexagon/op_helper.c
+++ b/target/hexagon/op_helper.c
@@ -948,7 +948,7 @@ float32 HELPER(sfmax)(CPUHexagonState *env, float32 RsV, float32 RtV)
 {
     float32 RdV;
     arch_fpop_start(env);
-    RdV = float32_maxnum(RsV, RtV, &env->fp_status);
+    RdV = float32_maximum_number(RsV, RtV, &env->fp_status);
     arch_fpop_end(env);
     return RdV;
 }
@@ -957,7 +957,7 @@ float32 HELPER(sfmin)(CPUHexagonState *env, float32 RsV, float32 RtV)
 {
     float32 RdV;
     arch_fpop_start(env);
-    RdV = float32_minnum(RsV, RtV, &env->fp_status);
+    RdV = float32_minimum_number(RsV, RtV, &env->fp_status);
     arch_fpop_end(env);
     return RdV;
 }
@@ -1041,8 +1041,9 @@ float64 HELPER(dfmax)(CPUHexagonState *env, float64 RssV, float64 RttV)
 {
     float64 RddV;
     arch_fpop_start(env);
-    RddV = float64_maxnum(RssV, RttV, &env->fp_status);
-    if (float64_is_any_nan(RssV) || float64_is_any_nan(RttV)) {
+    RddV = float64_maximum_number(RssV, RttV, &env->fp_status);
+    if (float64_is_quiet_nan(RssV, &env->fp_status) ||
+        float64_is_quiet_nan(RttV, &env->fp_status)) {
         float_raise(float_flag_invalid, &env->fp_status);
     }
     arch_fpop_end(env);
@@ -1053,8 +1054,9 @@ float64 HELPER(dfmin)(CPUHexagonState *env, float64 RssV, float64 RttV)
 {
     float64 RddV;
     arch_fpop_start(env);
-    RddV = float64_minnum(RssV, RttV, &env->fp_status);
-    if (float64_is_any_nan(RssV) || float64_is_any_nan(RttV)) {
+    RddV = float64_minimum_number(RssV, RttV, &env->fp_status);
+    if (float64_is_quiet_nan(RssV, &env->fp_status) ||
+        float64_is_quiet_nan(RttV, &env->fp_status)) {
         float_raise(float_flag_invalid, &env->fp_status);
     }
     arch_fpop_end(env);
-- 
2.17.1

On 2/16/22 15:39, Taylor Simpson wrote:
> The float??_minnum implementation differs from Hexagon for SNaN,
> it returns NaN, but Hexagon returns the other input.  So, we use
> float??_minimum_number.  For double precision, we check for QNaN and
> raise the invalid flag.

I'm surprised that the behaviour for double differs from single, but the docs are light on 
the subject.  Anyway,

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Wednesday, February 16, 2022 4:53 PM
> To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
> Cc: f4bug@amsat.org; ale@rev.ng; Brian Cain <bcain@quicinc.com>; Michael
> Lambert <mlambert@quicinc.com>
> Subject: Re: [PATCH v3] Hexagon (target/hexagon) properly handle NaN in
> dfmin/dfmax/sfmin/sfmax
> 
> On 2/16/22 15:39, Taylor Simpson wrote:
> > The float??_minnum implementation differs from Hexagon for SNaN, it
> > returns NaN, but Hexagon returns the other input.  So, we use
> > float??_minimum_number.  For double precision, we check for QNaN and
> > raise the invalid flag.
> 
> I'm surprised that the behaviour for double differs from single, but the docs
> are light on the subject.  Anyway,
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

The dfmin/dfmax were added in a later version of the hardware than sfmin/sfmax.  I was seeing the different behavior on the reference simulator.  I was able to get my hands on the newer hardware to run the test and found that the reference simulator behaves differently.  I'll respin this patch to match the behavior of the hardware.

Thanks,
Taylor