1. Patchew
  2. QEMU
  3. View series

[PATCH v2] Hexagon: fix F2_conv_* instructions for negative zero

Matheus Tavares Bernardino posted 1 patch 3 months, 3 weeks ago 
target/hexagon/op_helper.c | 18 +++++++++---------
tests/tcg/hexagon/usr.c    | 12 +++++++++++-
2 files changed, 20 insertions(+), 10 deletions(-)
[PATCH v2] Hexagon: fix F2_conv_* instructions for negative zero
Posted by Matheus Tavares Bernardino 3 months, 3 weeks ago 
The implementation for these instructions handles -0 as an invalid float
point value, whereas the Hexagon hardware considers it the same as +0
(which is valid). Let's fix that and add a regression test.

Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
Reviewed-by: Brian Cain <bcain@quicinc.com>
Reviewed-by: Taylor Simpson <ltaylorsimpson@gmail.com>
---
v2: updated the copyright year

 target/hexagon/op_helper.c | 18 +++++++++---------
 tests/tcg/hexagon/usr.c    | 12 +++++++++++-
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/target/hexagon/op_helper.c b/target/hexagon/op_helper.c
index ae5a605513..90e7aaa097 100644
--- a/target/hexagon/op_helper.c
+++ b/target/hexagon/op_helper.c
@@ -1,5 +1,5 @@
 /*
- *  Copyright(c) 2019-2023 Qualcomm Innovation Center, Inc. All Rights Reserved.
+ *  Copyright(c) 2019-2024 Qualcomm Innovation Center, Inc. All Rights Reserved.
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -683,7 +683,7 @@ uint32_t HELPER(conv_sf2uw)(CPUHexagonState *env, float32 RsV)
     uint32_t RdV;
     arch_fpop_start(env);
     /* Hexagon checks the sign before rounding */
-    if (float32_is_neg(RsV) && !float32_is_any_nan(RsV)) {
+    if (float32_is_neg(RsV) && !float32_is_any_nan(RsV) && !float32_is_zero(RsV)) {
         float_raise(float_flag_invalid, &env->fp_status);
         RdV = 0;
     } else {
@@ -713,7 +713,7 @@ uint64_t HELPER(conv_sf2ud)(CPUHexagonState *env, float32 RsV)
     uint64_t RddV;
     arch_fpop_start(env);
     /* Hexagon checks the sign before rounding */
-    if (float32_is_neg(RsV) && !float32_is_any_nan(RsV)) {
+    if (float32_is_neg(RsV) && !float32_is_any_nan(RsV) && !float32_is_zero(RsV)) {
         float_raise(float_flag_invalid, &env->fp_status);
         RddV = 0;
     } else {
@@ -743,7 +743,7 @@ uint32_t HELPER(conv_df2uw)(CPUHexagonState *env, float64 RssV)
     uint32_t RdV;
     arch_fpop_start(env);
     /* Hexagon checks the sign before rounding */
-    if (float64_is_neg(RssV) && !float64_is_any_nan(RssV)) {
+    if (float64_is_neg(RssV) && !float64_is_any_nan(RssV) && !float64_is_zero(RssV)) {
         float_raise(float_flag_invalid, &env->fp_status);
         RdV = 0;
     } else {
@@ -773,7 +773,7 @@ uint64_t HELPER(conv_df2ud)(CPUHexagonState *env, float64 RssV)
     uint64_t RddV;
     arch_fpop_start(env);
     /* Hexagon checks the sign before rounding */
-    if (float64_is_neg(RssV) && !float64_is_any_nan(RssV)) {
+    if (float64_is_neg(RssV) && !float64_is_any_nan(RssV) && !float64_is_zero(RssV)) {
         float_raise(float_flag_invalid, &env->fp_status);
         RddV = 0;
     } else {
@@ -803,7 +803,7 @@ uint32_t HELPER(conv_sf2uw_chop)(CPUHexagonState *env, float32 RsV)
     uint32_t RdV;
     arch_fpop_start(env);
     /* Hexagon checks the sign before rounding */
-    if (float32_is_neg(RsV) && !float32_is_any_nan(RsV)) {
+    if (float32_is_neg(RsV) && !float32_is_any_nan(RsV) && !float32_is_zero(RsV)) {
         float_raise(float_flag_invalid, &env->fp_status);
         RdV = 0;
     } else {
@@ -833,7 +833,7 @@ uint64_t HELPER(conv_sf2ud_chop)(CPUHexagonState *env, float32 RsV)
     uint64_t RddV;
     arch_fpop_start(env);
     /* Hexagon checks the sign before rounding */
-    if (float32_is_neg(RsV) && !float32_is_any_nan(RsV)) {
+    if (float32_is_neg(RsV) && !float32_is_any_nan(RsV) && !float32_is_zero(RsV)) {
         float_raise(float_flag_invalid, &env->fp_status);
         RddV = 0;
     } else {
@@ -863,7 +863,7 @@ uint32_t HELPER(conv_df2uw_chop)(CPUHexagonState *env, float64 RssV)
     uint32_t RdV;
     arch_fpop_start(env);
     /* Hexagon checks the sign before rounding */
-    if (float64_is_neg(RssV) && !float64_is_any_nan(RssV)) {
+    if (float64_is_neg(RssV) && !float64_is_any_nan(RssV) && !float64_is_zero(RssV)) {
         float_raise(float_flag_invalid, &env->fp_status);
         RdV = 0;
     } else {
@@ -893,7 +893,7 @@ uint64_t HELPER(conv_df2ud_chop)(CPUHexagonState *env, float64 RssV)
     uint64_t RddV;
     arch_fpop_start(env);
     /* Hexagon checks the sign before rounding */
-    if (float64_is_neg(RssV) && !float64_is_any_nan(RssV)) {
+    if (float64_is_neg(RssV) && !float64_is_any_nan(RssV) && !float64_is_zero(RssV)) {
         float_raise(float_flag_invalid, &env->fp_status);
         RddV = 0;
     } else {
diff --git a/tests/tcg/hexagon/usr.c b/tests/tcg/hexagon/usr.c
index 92bc86a213..f0b23d312b 100644
--- a/tests/tcg/hexagon/usr.c
+++ b/tests/tcg/hexagon/usr.c
@@ -1,5 +1,5 @@
 /*
- *  Copyright(c) 2022-2023 Qualcomm Innovation Center, Inc. All Rights Reserved.
+ *  Copyright(c) 2022-2024 Qualcomm Innovation Center, Inc. All Rights Reserved.
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -1007,6 +1007,11 @@ int main()
     TEST_P_OP_R(conv_sf2d_chop,   SF_QNaN,  0xffffffffffffffffULL,  USR_FPINVF);
     TEST_P_OP_R(conv_sf2d_chop,   SF_SNaN,  0xffffffffffffffffULL,  USR_FPINVF);
 
+    TEST_R_OP_R(conv_sf2uw,       SF_zero_neg,  0, USR_CLEAR);
+    TEST_R_OP_R(conv_sf2uw_chop,  SF_zero_neg,  0, USR_CLEAR);
+    TEST_P_OP_R(conv_sf2ud,       SF_zero_neg,  0, USR_CLEAR);
+    TEST_P_OP_R(conv_sf2ud_chop,  SF_zero_neg,  0, USR_CLEAR);
+
     TEST_R_OP_P(conv_df2sf,       DF_QNaN,  SF_HEX_NaN,             USR_CLEAR);
     TEST_R_OP_P(conv_df2sf,       DF_SNaN,  SF_HEX_NaN,             USR_FPINVF);
     TEST_R_OP_P(conv_df2uw,       DF_QNaN,  0xffffffff,             USR_FPINVF);
@@ -1020,6 +1025,11 @@ int main()
     TEST_R_OP_P(conv_df2uw_chop,  DF_QNaN,  0xffffffff,             USR_FPINVF);
     TEST_R_OP_P(conv_df2uw_chop,  DF_SNaN,  0xffffffff,             USR_FPINVF);
 
+    TEST_R_OP_P(conv_df2uw,       DF_zero_neg,  0, USR_CLEAR);
+    TEST_R_OP_P(conv_df2uw_chop,  DF_zero_neg,  0, USR_CLEAR);
+    TEST_P_OP_P(conv_df2ud,       DF_zero_neg,  0, USR_CLEAR);
+    TEST_P_OP_P(conv_df2ud_chop,  DF_zero_neg,  0, USR_CLEAR);
+
     /* Test for typo in HELPER(conv_df2uw_chop) */
     TEST_R_OP_P(conv_df2uw_chop, 0xffffff7f00000001ULL, 0xffffffff, USR_FPINVF);
 
-- 
2.37.2
Re: [PATCH v2] Hexagon: fix F2_conv_* instructions for negative zero
Posted by Brian Cain 3 months, 3 weeks ago 
On 7/29/2024 2:24 PM, Matheus Tavares Bernardino wrote:
> The implementation for these instructions handles -0 as an invalid float
> point value, whereas the Hexagon hardware considers it the same as +0
> (which is valid). Let's fix that and add a regression test.
>
> Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
> Reviewed-by: Brian Cain <bcain@quicinc.com>
> Reviewed-by: Taylor Simpson <ltaylorsimpson@gmail.com>
> ---
> v2: updated the copyright year


Queued. thanks!


>   target/hexagon/op_helper.c | 18 +++++++++---------
>   tests/tcg/hexagon/usr.c    | 12 +++++++++++-
>   2 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/target/hexagon/op_helper.c b/target/hexagon/op_helper.c
> index ae5a605513..90e7aaa097 100644
> --- a/target/hexagon/op_helper.c
> +++ b/target/hexagon/op_helper.c
> @@ -1,5 +1,5 @@
>   /*
> - *  Copyright(c) 2019-2023 Qualcomm Innovation Center, Inc. All Rights Reserved.
> + *  Copyright(c) 2019-2024 Qualcomm Innovation Center, Inc. All Rights Reserved.
>    *
>    *  This program is free software; you can redistribute it and/or modify
>    *  it under the terms of the GNU General Public License as published by
> @@ -683,7 +683,7 @@ uint32_t HELPER(conv_sf2uw)(CPUHexagonState *env, float32 RsV)
>       uint32_t RdV;
>       arch_fpop_start(env);
>       /* Hexagon checks the sign before rounding */
> -    if (float32_is_neg(RsV) && !float32_is_any_nan(RsV)) {
> +    if (float32_is_neg(RsV) && !float32_is_any_nan(RsV) && !float32_is_zero(RsV)) {
>           float_raise(float_flag_invalid, &env->fp_status);
>           RdV = 0;
>       } else {
> @@ -713,7 +713,7 @@ uint64_t HELPER(conv_sf2ud)(CPUHexagonState *env, float32 RsV)
>       uint64_t RddV;
>       arch_fpop_start(env);
>       /* Hexagon checks the sign before rounding */
> -    if (float32_is_neg(RsV) && !float32_is_any_nan(RsV)) {
> +    if (float32_is_neg(RsV) && !float32_is_any_nan(RsV) && !float32_is_zero(RsV)) {
>           float_raise(float_flag_invalid, &env->fp_status);
>           RddV = 0;
>       } else {
> @@ -743,7 +743,7 @@ uint32_t HELPER(conv_df2uw)(CPUHexagonState *env, float64 RssV)
>       uint32_t RdV;
>       arch_fpop_start(env);
>       /* Hexagon checks the sign before rounding */
> -    if (float64_is_neg(RssV) && !float64_is_any_nan(RssV)) {
> +    if (float64_is_neg(RssV) && !float64_is_any_nan(RssV) && !float64_is_zero(RssV)) {
>           float_raise(float_flag_invalid, &env->fp_status);
>           RdV = 0;
>       } else {
> @@ -773,7 +773,7 @@ uint64_t HELPER(conv_df2ud)(CPUHexagonState *env, float64 RssV)
>       uint64_t RddV;
>       arch_fpop_start(env);
>       /* Hexagon checks the sign before rounding */
> -    if (float64_is_neg(RssV) && !float64_is_any_nan(RssV)) {
> +    if (float64_is_neg(RssV) && !float64_is_any_nan(RssV) && !float64_is_zero(RssV)) {
>           float_raise(float_flag_invalid, &env->fp_status);
>           RddV = 0;
>       } else {
> @@ -803,7 +803,7 @@ uint32_t HELPER(conv_sf2uw_chop)(CPUHexagonState *env, float32 RsV)
>       uint32_t RdV;
>       arch_fpop_start(env);
>       /* Hexagon checks the sign before rounding */
> -    if (float32_is_neg(RsV) && !float32_is_any_nan(RsV)) {
> +    if (float32_is_neg(RsV) && !float32_is_any_nan(RsV) && !float32_is_zero(RsV)) {
>           float_raise(float_flag_invalid, &env->fp_status);
>           RdV = 0;
>       } else {
> @@ -833,7 +833,7 @@ uint64_t HELPER(conv_sf2ud_chop)(CPUHexagonState *env, float32 RsV)
>       uint64_t RddV;
>       arch_fpop_start(env);
>       /* Hexagon checks the sign before rounding */
> -    if (float32_is_neg(RsV) && !float32_is_any_nan(RsV)) {
> +    if (float32_is_neg(RsV) && !float32_is_any_nan(RsV) && !float32_is_zero(RsV)) {
>           float_raise(float_flag_invalid, &env->fp_status);
>           RddV = 0;
>       } else {
> @@ -863,7 +863,7 @@ uint32_t HELPER(conv_df2uw_chop)(CPUHexagonState *env, float64 RssV)
>       uint32_t RdV;
>       arch_fpop_start(env);
>       /* Hexagon checks the sign before rounding */
> -    if (float64_is_neg(RssV) && !float64_is_any_nan(RssV)) {
> +    if (float64_is_neg(RssV) && !float64_is_any_nan(RssV) && !float64_is_zero(RssV)) {
>           float_raise(float_flag_invalid, &env->fp_status);
>           RdV = 0;
>       } else {
> @@ -893,7 +893,7 @@ uint64_t HELPER(conv_df2ud_chop)(CPUHexagonState *env, float64 RssV)
>       uint64_t RddV;
>       arch_fpop_start(env);
>       /* Hexagon checks the sign before rounding */
> -    if (float64_is_neg(RssV) && !float64_is_any_nan(RssV)) {
> +    if (float64_is_neg(RssV) && !float64_is_any_nan(RssV) && !float64_is_zero(RssV)) {
>           float_raise(float_flag_invalid, &env->fp_status);
>           RddV = 0;
>       } else {
> diff --git a/tests/tcg/hexagon/usr.c b/tests/tcg/hexagon/usr.c
> index 92bc86a213..f0b23d312b 100644
> --- a/tests/tcg/hexagon/usr.c
> +++ b/tests/tcg/hexagon/usr.c
> @@ -1,5 +1,5 @@
>   /*
> - *  Copyright(c) 2022-2023 Qualcomm Innovation Center, Inc. All Rights Reserved.
> + *  Copyright(c) 2022-2024 Qualcomm Innovation Center, Inc. All Rights Reserved.
>    *
>    *  This program is free software; you can redistribute it and/or modify
>    *  it under the terms of the GNU General Public License as published by
> @@ -1007,6 +1007,11 @@ int main()
>       TEST_P_OP_R(conv_sf2d_chop,   SF_QNaN,  0xffffffffffffffffULL,  USR_FPINVF);
>       TEST_P_OP_R(conv_sf2d_chop,   SF_SNaN,  0xffffffffffffffffULL,  USR_FPINVF);
>   
> +    TEST_R_OP_R(conv_sf2uw,       SF_zero_neg,  0, USR_CLEAR);
> +    TEST_R_OP_R(conv_sf2uw_chop,  SF_zero_neg,  0, USR_CLEAR);
> +    TEST_P_OP_R(conv_sf2ud,       SF_zero_neg,  0, USR_CLEAR);
> +    TEST_P_OP_R(conv_sf2ud_chop,  SF_zero_neg,  0, USR_CLEAR);
> +
>       TEST_R_OP_P(conv_df2sf,       DF_QNaN,  SF_HEX_NaN,             USR_CLEAR);
>       TEST_R_OP_P(conv_df2sf,       DF_SNaN,  SF_HEX_NaN,             USR_FPINVF);
>       TEST_R_OP_P(conv_df2uw,       DF_QNaN,  0xffffffff,             USR_FPINVF);
> @@ -1020,6 +1025,11 @@ int main()
>       TEST_R_OP_P(conv_df2uw_chop,  DF_QNaN,  0xffffffff,             USR_FPINVF);
>       TEST_R_OP_P(conv_df2uw_chop,  DF_SNaN,  0xffffffff,             USR_FPINVF);
>   
> +    TEST_R_OP_P(conv_df2uw,       DF_zero_neg,  0, USR_CLEAR);
> +    TEST_R_OP_P(conv_df2uw_chop,  DF_zero_neg,  0, USR_CLEAR);
> +    TEST_P_OP_P(conv_df2ud,       DF_zero_neg,  0, USR_CLEAR);
> +    TEST_P_OP_P(conv_df2ud_chop,  DF_zero_neg,  0, USR_CLEAR);
> +
>       /* Test for typo in HELPER(conv_df2uw_chop) */
>       TEST_R_OP_P(conv_df2uw_chop, 0xffffff7f00000001ULL, 0xffffffff, USR_FPINVF);
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.