[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH 1/3] target/ppc: Bugfix fadd/fsub result with OE/UE set
|
From: |
Lucas Mateus Martins Araujo e Castro |
|
Subject: |
Re: [RFC PATCH 1/3] target/ppc: Bugfix fadd/fsub result with OE/UE set |
|
Date: |
Wed, 3 Aug 2022 15:42:27 -0300 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 |
On 03/08/2022 15:16, Richard Henderson
wrote:
On 8/3/22 10:45, Lucas Mateus Martins Araujo e Castro wrote:
On 03/08/2022 13:18, Richard Henderson wrote:
On 8/3/22 05:22, Lucas Mateus Castro(alqotel) wrote:
From: "Lucas Mateus Castro (alqotel)"
<lucas.araujo@eldorado.org.br>
As mentioned in the functions float_overflow_excp and
float_underflow_excp, the result should be adjusted as
mentioned in the
ISA (subtracted 192/1536 from the exponent of the
intermediate result if
an overflow occurs with OE set and added 192/1536 to the
exponent of the
intermediate result if an underflow occurs with UE set), but
at those
functions the result has already been rounded so it is not
possible to
add/subtract from the intermediate result anymore.
This patch creates a new function that receives the value
that should be
subtracted/added from the exponent if an overflow/underflow
happens, to
not leave some arbitrary numbers from the PowerISA in the
middle of the
FPU code. If these numbers are 0 the new functions just call
the old
ones.
I used 2 values here for overflow and underflow, maybe it'd
be better to
just use the same ones, any thoughts?
Signed-off-by: Lucas Mateus Castro (alqotel)
<lucas.araujo@eldorado.org.br>
---
An alternative I've thought was to always return the value
adjusted if a
overflow or underflow occurs and in float_underflow_excp and
float_overflow_excp adjust it to inf/den/0 if OE/UE is 0,
but I didn't
saw many advantages to that approach.
---
fpu/softfloat.c | 75
+++++++++++++++++++++++++++++++++++++++++
include/fpu/softfloat.h | 2 ++
target/ppc/fpu_helper.c | 10 ++++--
3 files changed, 85 insertions(+), 2 deletions(-)
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 4a871ef2a1..a407129dcb 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -268,6 +268,8 @@ typedef bool
(*f64_check_fn)(union_float64 a, union_float64 b);
typedef float32 (*soft_f32_op2_fn)(float32 a, float32 b,
float_status *s);
typedef float64 (*soft_f64_op2_fn)(float64 a, float64 b,
float_status *s);
+typedef float64 (*soft_f64_op2_int2_fn)(float64 a, float64
b, int c, int d,
+ float_status *s);
typedef float (*hard_f32_op2_fn)(float a, float b);
typedef double (*hard_f64_op2_fn)(double a, double b);
@@ -401,6 +403,19 @@ float64_gen2(float64 xa, float64 xb,
float_status *s,
return soft(ua.s, ub.s, s);
}
+static inline float64
+float64_gen2_excp(float64 xa, float64 xb, int xc, int xd,
float_status *s,
+ hard_f64_op2_fn hard, soft_f64_op2_fn
soft,
+ soft_f64_op2_int2_fn soft_excp,
f64_check_fn pre,
+ f64_check_fn post)
+{
+ if (xc || xd) {
+ return soft_excp(xa, xb, xc, xd, s);
+ } else {
+ return float64_gen2(xa, xb, s, hard, soft, pre,
post);
+ }
+}
+
/*
* Classify a floating point number. Everything above
float_class_qnan
* is a NaN so cls >= float_class_qnan is any NaN.
@@ -1929,6 +1944,39 @@ static double hard_f64_sub(double a,
double b)
return a - b;
}
+static float64 QEMU_SOFTFLOAT_ATTR
+soft_f64_addsub_excp_en(float64 a, float64 b, int oe_sub,
int ue_sum,
+ float_status *status, bool
subtract)
+{
+ FloatParts64 pa, pb, *pr;
+
+ float64_unpack_canonical(&pa, a, status);
+ float64_unpack_canonical(&pb, b, status);
+ pr = parts_addsub(&pa, &pb, status, subtract);
+
+ if (unlikely(oe_sub && (pr->exp > 1023)))
{
+ pr->exp -= oe_sub;
+ float_raise(float_flag_overflow, status);
+ } else if (unlikely(ue_sum && (pr->exp <
-1022))) {
+ pr->exp += ue_sum;
+ float_raise(float_flag_underflow, status);
+ }
+
+ return float64_round_pack_canonical(pr, status);
This is incorrect, because the exponent is not fixed until the
middle of
round_pack_canonical.
I think you should not add new functions like this, with new
parameters, but instead add
fields to float_status, which would then be checked at the
places currently setting
underflow and overflow.
So add overflow_correction and underflow_correction in
'partsN(uncanon_normal)' so that:
if (exp >= exp_max) {
if (overflow_correction != 0) {
exp -= overflow_correction;
}
}
And the equivalent for underflow, or a bool ppc_overflow_enable
that uses a fixed value like:
if (exp >= exp_max) {
if (ppc_overflow_enable) {
exp -= ((fmt->exp_bias + 1) + (fmt->exp_bias +
1)/2);
}
}
(and the equivalent for underflow) ?
Something like that.
I would suggest pre-computing that adjustment into fmt, via
FLOAT_PARAMS.
Naming is always hard, but how about exp_re_bias?
The flag(s) should not contain "ppc" in the name. But perhaps
s->rebias_overflow
s->rebias_underflow
rebias_* sounds good to me.
Also I imagine that these bools would be set by mtfsf, mtfsfi,
mtfsb0 and mtfsb1, in which case it'd make these patches
significantly shorter. I'll send a v2 with these changes
r~
[RFC PATCH 2/3] target/ppc: Bugfix fmul result with OE/UE set, Lucas Mateus Castro(alqotel), 2022/08/03
[RFC PATCH 3/3] target/ppc: Bugfix fdiv result with OE/UE set, Lucas Mateus Castro(alqotel), 2022/08/03
[PATCH] tests/tcg/ppc64le: Added OE/UE enabled exception test, Lucas Mateus Castro(alqotel), 2022/08/03