diff mbox series

[1/3] target/s390x: fix handling of zeroes in vfmin/vfmax

Message ID 20220712015717.3602602-2-iii@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series target/s390x: vfmin/vfmax fixes | expand

Commit Message

Ilya Leoshkevich July 12, 2022, 1:57 a.m. UTC
vfmin_res() / vfmax_res() are trying to check whether a and b are both
zeroes, but in reality they check that they are the same kind of zero.
This causes incorrect results when comparing positive and negative
zeroes.

Fixes: da4807527f3b ("s390x/tcg: Implement VECTOR FP (MAXIMUM|MINIMUM)")
Co-developed-by: Ulrich Weigand <ulrich.weigand@de.ibm.com>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 target/s390x/tcg/vec_fpu_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Richard Henderson July 12, 2022, 4:23 a.m. UTC | #1
On 7/12/22 07:27, Ilya Leoshkevich wrote:
> vfmin_res() / vfmax_res() are trying to check whether a and b are both
> zeroes, but in reality they check that they are the same kind of zero.
> This causes incorrect results when comparing positive and negative
> zeroes.
> 
> Fixes: da4807527f3b ("s390x/tcg: Implement VECTOR FP (MAXIMUM|MINIMUM)")
> Co-developed-by: Ulrich Weigand<ulrich.weigand@de.ibm.com>
> Signed-off-by: Ilya Leoshkevich<iii@linux.ibm.com>
> ---
>   target/s390x/tcg/vec_fpu_helper.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

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

r~
David Hildenbrand July 12, 2022, 7:16 a.m. UTC | #2
On 12.07.22 03:57, Ilya Leoshkevich wrote:
> vfmin_res() / vfmax_res() are trying to check whether a and b are both
> zeroes, but in reality they check that they are the same kind of zero.
> This causes incorrect results when comparing positive and negative
> zeroes.
> 
> Fixes: da4807527f3b ("s390x/tcg: Implement VECTOR FP (MAXIMUM|MINIMUM)")
> Co-developed-by: Ulrich Weigand <ulrich.weigand@de.ibm.com>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>  target/s390x/tcg/vec_fpu_helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/s390x/tcg/vec_fpu_helper.c b/target/s390x/tcg/vec_fpu_helper.c
> index 2a618a1093..75cf605b9f 100644
> --- a/target/s390x/tcg/vec_fpu_helper.c
> +++ b/target/s390x/tcg/vec_fpu_helper.c
> @@ -824,7 +824,7 @@ static S390MinMaxRes vfmin_res(uint16_t dcmask_a, uint16_t dcmask_b,
>          default:
>              g_assert_not_reached();
>          }
> -    } else if (unlikely(dcmask_a & dcmask_b & DCMASK_ZERO)) {
> +    } else if (unlikely((dcmask_a & DCMASK_ZERO) && (dcmask_b & DCMASK_ZERO))) {
>          switch (type) {
>          case S390_MINMAX_TYPE_JAVA:
>              return neg_a ? S390_MINMAX_RES_A : S390_MINMAX_RES_B;
> @@ -874,7 +874,7 @@ static S390MinMaxRes vfmax_res(uint16_t dcmask_a, uint16_t dcmask_b,
>          default:
>              g_assert_not_reached();
>          }
> -    } else if (unlikely(dcmask_a & dcmask_b & DCMASK_ZERO)) {
> +    } else if (unlikely((dcmask_a & DCMASK_ZERO) && (dcmask_b & DCMASK_ZERO))) {
>          const bool neg_a = dcmask_a & DCMASK_NEGATIVE;
>  
>          switch (type) {

Thanks!

Reviewed-by: David Hildenbrand <david@redhat.com>
diff mbox series

Patch

diff --git a/target/s390x/tcg/vec_fpu_helper.c b/target/s390x/tcg/vec_fpu_helper.c
index 2a618a1093..75cf605b9f 100644
--- a/target/s390x/tcg/vec_fpu_helper.c
+++ b/target/s390x/tcg/vec_fpu_helper.c
@@ -824,7 +824,7 @@  static S390MinMaxRes vfmin_res(uint16_t dcmask_a, uint16_t dcmask_b,
         default:
             g_assert_not_reached();
         }
-    } else if (unlikely(dcmask_a & dcmask_b & DCMASK_ZERO)) {
+    } else if (unlikely((dcmask_a & DCMASK_ZERO) && (dcmask_b & DCMASK_ZERO))) {
         switch (type) {
         case S390_MINMAX_TYPE_JAVA:
             return neg_a ? S390_MINMAX_RES_A : S390_MINMAX_RES_B;
@@ -874,7 +874,7 @@  static S390MinMaxRes vfmax_res(uint16_t dcmask_a, uint16_t dcmask_b,
         default:
             g_assert_not_reached();
         }
-    } else if (unlikely(dcmask_a & dcmask_b & DCMASK_ZERO)) {
+    } else if (unlikely((dcmask_a & DCMASK_ZERO) && (dcmask_b & DCMASK_ZERO))) {
         const bool neg_a = dcmask_a & DCMASK_NEGATIVE;
 
         switch (type) {