Re: [Qemu-devel] [PATCH v1 18/19] fpu/softfloat: re-factor minmax

2017-12-18 Thread Richard Henderson
On 12/11/2017 04:57 AM, Alex Bennée wrote:
> +static decomposed_parts minmax_decomposed(decomposed_parts a,
> +  decomposed_parts b,
> +  bool ismin, bool ieee, bool ismag,
> +  float_status *s)
> +{
> +if (a.cls >= float_class_qnan

Indent is off by 4.

> +if (s->default_nan_mode) {
> +a.cls = float_class_msnan;
> +return a;

float_class_dnan.  That said, can't you fall through to pick_nan_parts and have
that (and float_flag_invalid) handled already?

> +if (a.cls == float_class_zero || b.cls == float_class_zero) {
> +if (a.cls == float_class_normal) {
> +if (a.sign) {
> +return ismin ? a : b;
> +} else {
> +return ismin ? b : a;
> +}
> +} else if (b.cls == float_class_normal) {
> +if (b.sign) {
> +return ismin ? b : a;
> +} else {
> +return ismin ? a : b;
> +}
> +}
> +}

With respect to zero, normal and inf should be handled the same.
Both of those middle tests should be

  a.cls == float_class_normal || a.cls == float_class_inf

It would appear this section is not honoring ismag.

This is one case where I think it might be helpful to do

int a_exp, b_exp;
bool a_sign, b_sign;

if (nans) {
...
}

switch (a.cls) {
case float_class_normal:
a_exp = a.exp;
break;
case float_class_inf:
a_exp = INT_MAX;
break;
case float_class_zero:
a_exp = INT_MIN;
break;
}
switch (b.cls) {

}
a_sign = a.sign;
b_sign = b.sign
if (ismag) {
a_sign = b_sign = 0;
}

if (a_sign == b_sign) {
bool a_less = a_exp < b_exp;
if (a_exp == b_exp) {
a_less = a.frac < b.frac;
}
return a_sign ^ a_less ^ ismin ? b : a;
} else {
return a_sign ^ ismin ? b : a;
}


r~



[Qemu-devel] [PATCH v1 18/19] fpu/softfloat: re-factor minmax

2017-12-11 Thread Alex Bennée
Let's do the same re-factor treatment for minmax functions. I still
use the MACRO trick to expand but now all the checking code is common.

Signed-off-by: Alex Bennée 
---
 fpu/softfloat.c | 242 ++--
 include/fpu/softfloat.h |   6 ++
 2 files changed, 137 insertions(+), 111 deletions(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index b7ea56dfa5..5eba996932 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -1662,6 +1662,137 @@ float64 uint16_to_float64(uint16_t a, float_status 
*status)
 return uint64_to_float64((uint64_t) a, status);
 }
 
+/* Float Min/Max */
+/* min() and max() functions. These can't be implemented as
+ * 'compare and pick one input' because that would mishandle
+ * NaNs and +0 vs -0.
+ *
+ * minnum() and maxnum() functions. These are similar to the min()
+ * and max() functions but if one of the arguments is a QNaN and
+ * the other is numerical then the numerical argument is returned.
+ * SNaNs will get quietened before being returned.
+ * minnum() and maxnum correspond to the IEEE 754-2008 minNum()
+ * and maxNum() operations. min() and max() are the typical min/max
+ * semantics provided by many CPUs which predate that specification.
+ *
+ * minnummag() and maxnummag() functions correspond to minNumMag()
+ * and minNumMag() from the IEEE-754 2008.
+ */
+static decomposed_parts minmax_decomposed(decomposed_parts a,
+  decomposed_parts b,
+  bool ismin, bool ieee, bool ismag,
+  float_status *s)
+{
+if (a.cls >= float_class_qnan
+||
+b.cls >= float_class_qnan)
+{
+if (ieee) {
+/* Takes two floating-point values `a' and `b', one of
+ * which is a NaN, and returns the appropriate NaN
+ * result. If either `a' or `b' is a signaling NaN,
+ * the invalid exception is raised.
+ */
+if (a.cls == float_class_snan || b.cls == float_class_snan) {
+s->float_exception_flags |= float_flag_invalid;
+if (s->default_nan_mode) {
+a.cls = float_class_msnan;
+return a;
+}
+} else if (a.cls >= float_class_qnan
+   &&
+   b.cls < float_class_qnan) {
+return b;
+} else if (b.cls >= float_class_qnan
+   &&
+   a.cls < float_class_qnan) {
+return a;
+}
+}
+return pick_nan_parts(a, b, s);
+}
+
+/* Handle zero cases */
+if (a.cls == float_class_zero || b.cls == float_class_zero) {
+if (a.cls == float_class_normal) {
+if (a.sign) {
+return ismin ? a : b;
+} else {
+return ismin ? b : a;
+}
+} else if (b.cls == float_class_normal) {
+if (b.sign) {
+return ismin ? b : a;
+} else {
+return ismin ? a : b;
+}
+}
+}
+
+if (ismag) {
+/* Magnitude, ignore sign */
+bool a_less;
+if (a.exp == b.exp) {
+a_less = a.frac < b.frac;
+} else {
+a_less = a.exp < b.exp;
+}
+return a_less == ismin ? a : b;
+}
+if (a.sign != b.sign) {
+if (ismin) {
+return a.sign ? a : b;
+} else {
+return a.sign ? b : a;
+}
+} else {
+bool a_less;
+if (a.exp == b.exp) {
+a_less = a.frac < b.frac;
+} else {
+a_less = a.exp < b.exp;
+}
+if (ismin) {
+return a.sign ^ a_less ? a : b;
+} else {
+return a.sign ^ a_less ? b : a;
+}
+}
+}
+
+#define MINMAX(sz, name, ismin, isiee, ismag)   \
+float ## sz float ## sz ## _ ## name(float ## sz a, float ## sz b, 
float_status *s) \
+{   \
+decomposed_parts pa = float ## sz ## _unpack_canonical(a, s);   \
+decomposed_parts pb = float ## sz ## _unpack_canonical(b, s);   \
+decomposed_parts pr = minmax_decomposed(pa, pb, ismin, isiee, ismag, s); \
+\
+return float ## sz ## _round_pack_canonical(pr, s);\
+}
+
+MINMAX(16, min, true, false, false)
+MINMAX(16, minnum, true, true, false)
+MINMAX(16, minnummag, true, true, true)
+MINMAX(16, max, false, false, false)
+MINMAX(16, maxnum, fals