Alec Roelke has uploaded this change for review. (
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
---
M src/arch/riscv/isa/decoder.isa
1 file changed, 63 insertions(+), 53 deletions(-)
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: newchange
Gerrit-Change-Id: Id983906ea0664dc246e115a9e470d9ab7733bde1
Gerrit-Change-Number: 6402
Gerrit-PatchSet: 1
Gerrit-Owner: Alec Roelke <ar...@virginia.edu>
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev
