Alec Roelke has submitted this change and it was merged. ( https://gem5-review.googlesource.com/6402 )

Change subject: arch-riscv: Fix floating-point conversion bugs
......................................................................

arch-riscv: Fix floating-point conversion bugs

Using the fetestexcept function to check for specific types of floating
point exceptions is unreliable for some kinds of
floating-point-to-integer conversion operations. RISC-V code used to
make use of them to check for some exceptional cases like overflow and
underflow, which caused incorrect output when compiler optimization is
turned on. This patch changes the use of fetestexcept to explicit checks
for those exceptional cases.

Change-Id: Id983906ea0664dc246e115a9e470d9ab7733bde1
Reviewed-on: https://gem5-review.googlesource.com/6402
Reviewed-by: Jason Lowe-Power <ja...@lowepower.com>
Maintainer: Alec Roelke <ar...@virginia.edu>
---
M src/arch/riscv/isa/decoder.isa
1 file changed, 63 insertions(+), 53 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved
  Alec Roelke: Looks good to me, approved



diff --git a/src/arch/riscv/isa/decoder.isa b/src/arch/riscv/isa/decoder.isa
index 435266c..baae581 100644
--- a/src/arch/riscv/isa/decoder.isa
+++ b/src/arch/riscv/isa/decoder.isa
@@ -1338,31 +1338,31 @@
                         if (std::isnan(fs1)) {
                             Rd_sd = numeric_limits<int32_t>::max();
                             FFLAGS |= FloatInvalid;
+                        } else if (fs1 >= numeric_limits<int32_t>::max()) {
+                            Rd_sd = numeric_limits<int32_t>::max();
+                            FFLAGS |= FloatInvalid;
+                        } else if (fs1 <= numeric_limits<int32_t>::min()) {
+                            Rd_sd = numeric_limits<int32_t>::min();
+                            FFLAGS |= FloatInvalid;
                         } else {
                             Rd_sd = (int32_t)fs1;
-                            if (fetestexcept(FE_INVALID)) {
-                                if (signbit(fs1)) {
-                                    Rd_sd = numeric_limits<int32_t>::min();
-                                } else {
-                                    Rd_sd = numeric_limits<int32_t>::max();
-                                }
-                                feclearexcept(FE_INEXACT);
-                            }
                         }
                     }}, FloatCvtOp);
                     0x1: fcvt_wu_s({{
                         uint32_t temp;
float fs1 = reinterpret_cast<float&>(temp = Fs1_bits);

-                        if (fs1 < 0.0) {
+                        if (std::isnan(fs1)) {
+                            Rd = numeric_limits<uint64_t>::max();
+                            FFLAGS |= FloatInvalid;
+                        } else if (fs1 < 0.0) {
                             Rd = 0;
                             FFLAGS |= FloatInvalid;
+                        } else if (fs1 > numeric_limits<uint32_t>::max()) {
+                            Rd = numeric_limits<uint64_t>::max();
+                            FFLAGS |= FloatInvalid;
                         } else {
                             Rd = (uint32_t)fs1;
-                            if (fetestexcept(FE_INVALID)) {
-                                Rd = numeric_limits<uint64_t>::max();
-                                feclearexcept(FE_INEXACT);
-                            }
                         }
                     }}, FloatCvtOp);
                     0x2: fcvt_l_s({{
@@ -1372,79 +1372,89 @@
                         if (std::isnan(fs1)) {
                             Rd_sd = numeric_limits<int64_t>::max();
                             FFLAGS |= FloatInvalid;
+                        } else if (fs1 > numeric_limits<int64_t>::max()) {
+                            Rd_sd = numeric_limits<int64_t>::max();
+                            FFLAGS |= FloatInvalid;
+                        } else if (fs1 < numeric_limits<int64_t>::min()) {
+                            Rd_sd = numeric_limits<int64_t>::min();
+                            FFLAGS |= FloatInvalid;
                         } else {
                             Rd_sd = (int64_t)fs1;
-                            if (fetestexcept(FE_INVALID)) {
-                                if (signbit(fs1)) {
-                                    Rd_sd = numeric_limits<int64_t>::min();
-                                } else {
-                                    Rd_sd = numeric_limits<int64_t>::max();
-                                }
-                                feclearexcept(FE_INEXACT);
-                            }
                         }
                     }}, FloatCvtOp);
                     0x3: fcvt_lu_s({{
                         uint32_t temp;
float fs1 = reinterpret_cast<float&>(temp = Fs1_bits);

-                        if (fs1 < 0.0) {
+                        if (std::isnan(fs1)) {
+                            Rd = numeric_limits<uint64_t>::max();
+                            FFLAGS |= FloatInvalid;
+                        } else if (fs1 < 0.0) {
                             Rd = 0;
                             FFLAGS |= FloatInvalid;
+                        } else if (fs1 > numeric_limits<uint64_t>::max()) {
+                            Rd = numeric_limits<uint64_t>::max();
+                            FFLAGS |= FloatInvalid;
                         } else {
                             Rd = (uint64_t)fs1;
-                            if (fetestexcept(FE_INVALID)) {
-                                Rd = numeric_limits<uint64_t>::max();
-                                feclearexcept(FE_INEXACT);
-                            }
                         }
                     }}, FloatCvtOp);
                 }
                 0x61: decode CONV_SGN {
                     0x0: fcvt_w_d({{
-                        Rd_sd = (int32_t)Fs1;
-                        if (fetestexcept(FE_INVALID)) {
-                            if (Fs1 < 0.0) {
-                                Rd_sd = numeric_limits<int32_t>::min();
-                            } else {
-                                Rd_sd = numeric_limits<int32_t>::max();
-                            }
-                            feclearexcept(FE_INEXACT);
+                        if (std::isnan(Fs1)) {
+                            Rd_sd = numeric_limits<int32_t>::max();
+                            FFLAGS |= FloatInvalid;
+                        } else if (Fs1 > numeric_limits<int32_t>::max()) {
+                            Rd_sd = numeric_limits<int32_t>::max();
+                            FFLAGS |= FloatInvalid;
+                        } else if (Fs1 < numeric_limits<int32_t>::min()) {
+                            Rd_sd = numeric_limits<int32_t>::min();
+                            FFLAGS |= FloatInvalid;
+                        } else {
+                            Rd_sd = (int32_t)Fs1;
                         }
                     }}, FloatCvtOp);
                     0x1: fcvt_wu_d({{
-                        if (Fs1 < 0.0) {
+                        if (std::isnan(Fs1)) {
+                            Rd = numeric_limits<uint64_t>::max();
+                            FFLAGS |= FloatInvalid;
+                        } else if (Fs1 < 0) {
                             Rd = 0;
                             FFLAGS |= FloatInvalid;
+                        } else if (Fs1 > numeric_limits<uint32_t>::max()) {
+                            Rd = numeric_limits<uint64_t>::max();
+                            FFLAGS |= FloatInvalid;
                         } else {
                             Rd = (uint32_t)Fs1;
-                            if (fetestexcept(FE_INVALID)) {
-                                Rd = numeric_limits<uint64_t>::max();
-                                feclearexcept(FE_INEXACT);
-                            }
                         }
                     }}, FloatCvtOp);
                     0x2: fcvt_l_d({{
-                        Rd_sd = Fs1;
-                        if (fetestexcept(FE_INVALID)) {
-                            if (Fs1 < 0.0) {
-                                Rd_sd = numeric_limits<int64_t>::min();
-                            } else {
-                                Rd_sd = numeric_limits<int64_t>::max();
-                            }
-                            feclearexcept(FE_INEXACT);
+                        if (std::isnan(Fs1)) {
+                            Rd_sd = numeric_limits<int64_t>::max();
+                            FFLAGS |= FloatInvalid;
+                        } else if (Fs1 > numeric_limits<int64_t>::max()) {
+                            Rd_sd = numeric_limits<int64_t>::max();
+                            FFLAGS |= FloatInvalid;
+                        } else if (Fs1 < numeric_limits<int64_t>::min()) {
+                            Rd_sd = numeric_limits<int64_t>::min();
+                            FFLAGS |= FloatInvalid;
+                        } else {
+                            Rd_sd = Fs1;
                         }
                     }}, FloatCvtOp);
                     0x3: fcvt_lu_d({{
-                        if (Fs1 < 0.0) {
+                        if (std::isnan(Fs1)) {
+                            Rd = numeric_limits<uint64_t>::max();
+                            FFLAGS |= FloatInvalid;
+                        } else if (Fs1 < 0) {
                             Rd = 0;
                             FFLAGS |= FloatInvalid;
+                        } else if (Fs1 > numeric_limits<uint64_t>::max()) {
+                            Rd = numeric_limits<uint64_t>::max();
+                            FFLAGS |= FloatInvalid;
                         } else {
-                            Rd = (uint64_t)Fs1;
-                            if (fetestexcept(FE_INVALID)) {
-                                Rd = numeric_limits<uint64_t>::max();
-                                feclearexcept(FE_INEXACT);
-                            }
+                            Rd = Fs1;
                         }
                     }}, FloatCvtOp);
                 }

--
To view, visit https://gem5-review.googlesource.com/6402
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id983906ea0664dc246e115a9e470d9ab7733bde1
Gerrit-Change-Number: 6402
Gerrit-PatchSet: 4
Gerrit-Owner: Alec Roelke <ar...@virginia.edu>
Gerrit-Reviewer: Alec Roelke <ar...@virginia.edu>
Gerrit-Reviewer: Jason Lowe-Power <ja...@lowepower.com>
Gerrit-Reviewer: Tuan Ta <q...@cornell.edu>
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to