qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <philmd@linaro.org>
To: Peter Maydell <peter.maydell@linaro.org>, qemu-devel@nongnu.org
Cc: "Alex Bennée" <alex.bennee@linaro.org>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Eduardo Habkost" <eduardo@habkost.net>,
	"Laurent Vivier" <laurent@vivier.eu>
Subject: Re: [PATCH 00/10] fpu: Remove remaining target ifdefs and build only once
Date: Fri, 21 Feb 2025 15:41:03 +0100	[thread overview]
Message-ID: <93307c7c-c29f-4061-94ae-461bf196ff41@linaro.org> (raw)
In-Reply-To: <20250217125055.160887-1-peter.maydell@linaro.org>

On 17/2/25 13:50, Peter Maydell wrote:

> (1) floatx80 behaviours
> 
> Two QEMU targets implement floatx80: x86 and m68k. (PPC also has one
> use in the xsrqpxp round-to-80-bit-precision operation, and the
> Linux-user NWFPE emulation nominally supports it, but these are
> minor.) x86 and m68k disagree about some of the corner cases of
> floatx80 where the value has the explicit Integer bit wrongly set.  At
> the moment the fpu code defaults to "floatx80 behaves like x86", with
> TARGET_M68K ifdefs to get the other option.
> 
> The first six patches in this series remove those ifdefs, replacing
> them with a floatx80_behaviour field in float_status which can have
> various flags set to select the individual behaviours. The default is
> "like x86", which allows us to set these only for m68k and not worry
> about the minor "technically makes some use of floatx80" cases.


> Peter Maydell (10):
>    fpu: Make targets specify floatx80 default Inf at runtime
>    target/m68k: Avoid using floatx80_infinity global const
>    target/i386: Avoid using floatx80_infinity global const

Bothering again, we can add the floatx80_default_inf() refactor as the
first patch:

-- >8 --
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index 09a40b43106..afae3906024 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -963,0 +964 @@ extern const floatx80 floatx80_infinity;
+floatx80 floatx80_default_inf(bool zSign, float_status *status);
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index f4fed9bfda9..f56ae886c53 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -5147,3 +5147 @@ floatx80 roundAndPackFloatx80(FloatX80RoundPrec 
roundingPrecision, bool zSign,
-            return packFloatx80(zSign,
-                                floatx80_infinity_high,
-                                floatx80_infinity_low);
+            return floatx80_default_inf(zSign, status);
diff --git a/target/i386/tcg/fpu_helper.c b/target/i386/tcg/fpu_helper.c
index f112c6c6737..741af09f908 100644
--- a/target/i386/tcg/fpu_helper.c
+++ b/target/i386/tcg/fpu_helper.c
@@ -1835 +1835 @@ void helper_fxtract(CPUX86State *env)
-        ST1 = floatx80_infinity;
+        ST1 = floatx80_default_inf(0, &env->fp_status);
@@ -2361,3 +2361,2 @@ void helper_fscale(CPUX86State *env)
-                ST0 = (floatx80_is_neg(ST0) ?
-                       floatx80_chs(floatx80_infinity) :
-                       floatx80_infinity);
+                ST0 = floatx80_default_inf(floatx80_is_neg(ST0),
+                                           &env->fp_status);
diff --git a/target/m68k/softfloat.c b/target/m68k/softfloat.c
index 02dcc03d15d..d1f150e641f 100644
--- a/target/m68k/softfloat.c
+++ b/target/m68k/softfloat.c
@@ -145,2 +145 @@ floatx80 floatx80_scale(floatx80 a, floatx80 b, 
float_status *status)
-        return packFloatx80(aSign, floatx80_infinity.high,
-                            floatx80_infinity.low);
+        return floatx80_default_inf(aSign, status);
@@ -248 +247 @@ floatx80 floatx80_lognp1(floatx80 a, float_status *status)
-        return packFloatx80(0, floatx80_infinity.high, 
floatx80_infinity.low);
+        return floatx80_default_inf(0, status);
@@ -258,2 +257 @@ floatx80 floatx80_lognp1(floatx80 a, float_status *status)
-            return packFloatx80(aSign, floatx80_infinity.high,
-                                floatx80_infinity.low);
+            return floatx80_default_inf(aSign, status);
@@ -445,2 +443 @@ floatx80 floatx80_logn(floatx80 a, float_status *status)
-            return packFloatx80(0, floatx80_infinity.high,
-                                floatx80_infinity.low);
+            return floatx80_default_inf(0, status);
@@ -455,2 +452 @@ floatx80 floatx80_logn(floatx80 a, float_status *status)
-            return packFloatx80(1, floatx80_infinity.high,
-                                floatx80_infinity.low);
+            return floatx80_default_inf(1, status);
@@ -613,2 +609 @@ floatx80 floatx80_log10(floatx80 a, float_status *status)
-            return packFloatx80(0, floatx80_infinity.high,
-                                floatx80_infinity.low);
+            return floatx80_default_inf(0, status);
@@ -620,2 +615 @@ floatx80 floatx80_log10(floatx80 a, float_status *status)
-        return packFloatx80(1, floatx80_infinity.high,
-                            floatx80_infinity.low);
+        return floatx80_default_inf(1, status);
@@ -671,2 +665 @@ floatx80 floatx80_log2(floatx80 a, float_status *status)
-            return packFloatx80(0, floatx80_infinity.high,
-                                floatx80_infinity.low);
+            return floatx80_default_inf(0, status);
@@ -679,2 +672 @@ floatx80 floatx80_log2(floatx80 a, float_status *status)
-            return packFloatx80(1, floatx80_infinity.high,
-                                floatx80_infinity.low);
+            return floatx80_default_inf(1, status);
@@ -743,2 +735 @@ floatx80 floatx80_etox(floatx80 a, float_status *status)
-        return packFloatx80(0, floatx80_infinity.high,
-                            floatx80_infinity.low);
+        return floatx80_default_inf(0, status);
@@ -927,2 +918 @@ floatx80 floatx80_twotox(floatx80 a, float_status *status)
-        return packFloatx80(0, floatx80_infinity.high,
-                            floatx80_infinity.low);
+        return floatx80_default_inf(0, status);
@@ -1078,2 +1068 @@ floatx80 floatx80_tentox(floatx80 a, float_status 
*status)
-        return packFloatx80(0, floatx80_infinity.high,
-                            floatx80_infinity.low);
+        return floatx80_default_inf(0, status);
@@ -2263,2 +2252 @@ floatx80 floatx80_atanh(floatx80 a, float_status 
*status)
-            return packFloatx80(aSign, floatx80_infinity.high,
-                                floatx80_infinity.low);
+            return floatx80_default_inf(aSign, status);
@@ -2323,2 +2311 @@ floatx80 floatx80_etoxm1(floatx80 a, float_status 
*status)
-        return packFloatx80(0, floatx80_infinity.high,
-                            floatx80_infinity.low);
+        return floatx80_default_inf(0, status);
@@ -2690,2 +2677 @@ floatx80 floatx80_sinh(floatx80 a, float_status *status)
-        return packFloatx80(aSign, floatx80_infinity.high,
-                            floatx80_infinity.low);
+        return floatx80_default_inf(aSign, status);
@@ -2777,2 +2763 @@ floatx80 floatx80_cosh(floatx80 a, float_status *status)
-        return packFloatx80(0, floatx80_infinity.high,
-                            floatx80_infinity.low);
+        return floatx80_default_inf(0, status);
diff --git a/fpu/softfloat-specialize.c.inc b/fpu/softfloat-specialize.c.inc
index cbbbab52ba3..403f7a9d69d 100644
--- a/fpu/softfloat-specialize.c.inc
+++ b/fpu/softfloat-specialize.c.inc
@@ -239,0 +240,5 @@ const floatx80 floatx80_infinity
+floatx80 floatx80_default_inf(bool zSign, float_status *status)
+{
+    return packFloatx80(zSign, floatx80_infinity_high, 
floatx80_infinity_low);
+}
+
---

The single non-obvious thing to mention is the x86 floatx80_chs()
removal.

This effectively absorb patch #3 (x86).

Then patch #2 (m68k) becomes:

-- >8 --
diff --git a/include/fpu/softfloat-helpers.h 
b/include/fpu/softfloat-helpers.h
index 8983c2748ec..90862f5cd22 100644
--- a/include/fpu/softfloat-helpers.h
+++ b/include/fpu/softfloat-helpers.h
@@ -77,0 +78,6 @@ static inline void 
set_floatx80_rounding_precision(FloatX80RoundPrec val,
+static inline void set_floatx80_behaviour(FloatX80Behaviour b,
+                                          float_status *status)
+{
+    status->floatx80_behaviour = b;
+}
+
@@ -153,0 +160,6 @@ get_floatx80_rounding_precision(const float_status 
*status)
+static inline FloatX80Behaviour
+get_floatx80_behaviour(const float_status *status)
+{
+    return status->floatx80_behaviour;
+}
+
diff --git a/include/fpu/softfloat-types.h b/include/fpu/softfloat-types.h
index 53d5eb85210..dd22ecdbe60 100644
--- a/include/fpu/softfloat-types.h
+++ b/include/fpu/softfloat-types.h
@@ -322,0 +323,12 @@ typedef enum __attribute__((__packed__)) {
+/*
+ * floatx80 is primarily used by x86 and m68k, and there are
+ * differences in the handling, largely related to the explicit
+ * Integer bit which floatx80 has and the other float formats do not.
+ * These flag values allow specification of the target's requirements
+ * and can be ORed together to set floatx80_behaviour.
+ */
+typedef enum __attribute__((__packed__)) {
+    /* In the default Infinity value, is the Integer bit 0 ? */
+    floatx80_default_inf_int_bit_is_zero = 1,
+} FloatX80Behaviour;
+
@@ -333,0 +346 @@ typedef struct float_status {
+    FloatX80Behaviour floatx80_behaviour;
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index f56ae886c53..b12ad2b42a9 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -1863 +1863,2 @@ static floatx80 
floatx80_round_pack_canonical(FloatParts128 *p,
-        frac = floatx80_infinity_low;
+        frac = s->floatx80_behaviour & 
floatx80_default_inf_int_bit_is_zero ?
+               0 : (1ULL << 63);
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index 41dfdf58045..df66e8ba22a 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -109,0 +110,6 @@ static void m68k_cpu_reset_hold(Object *obj, 
ResetType type)
+    /*
+     * m68k-specific floatx80 behaviour:
+     *  * default Infinity values have a zero Integer bit
+     */
+    set_floatx80_behaviour(floatx80_default_inf_int_bit_is_zero,
+                           &env->fp_status);
diff --git a/fpu/softfloat-specialize.c.inc b/fpu/softfloat-specialize.c.inc
index 403f7a9d69d..77a43f46597 100644
--- a/fpu/softfloat-specialize.c.inc
+++ b/fpu/softfloat-specialize.c.inc
@@ -242 +242,6 @@ floatx80 floatx80_default_inf(bool zSign, float_status 
*status)
-    return packFloatx80(zSign, floatx80_infinity_high, 
floatx80_infinity_low);
+    /*
+     * Whether the Integer bit is set in the default Infinity is
+     * target dependent.
+     */
+    bool z = status->floatx80_behaviour & 
floatx80_default_inf_int_bit_is_zero;
+    return packFloatx80(zSign, 0x7fff, z ? 0 : (1ULL << 63));
---


  parent reply	other threads:[~2025-02-21 14:41 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-17 12:50 [PATCH 00/10] fpu: Remove remaining target ifdefs and build only once Peter Maydell
2025-02-17 12:50 ` [PATCH 01/10] fpu: Make targets specify floatx80 default Inf at runtime Peter Maydell
2025-02-17 18:09   ` Richard Henderson
2025-02-21 14:42   ` Philippe Mathieu-Daudé
2025-02-21 15:16     ` Peter Maydell
2025-02-17 12:50 ` [PATCH 02/10] target/m68k: Avoid using floatx80_infinity global const Peter Maydell
2025-02-17 18:10   ` Richard Henderson
2025-02-21 13:51   ` Philippe Mathieu-Daudé
2025-02-17 12:50 ` [PATCH 03/10] target/i386: " Peter Maydell
2025-02-17 18:10   ` Richard Henderson
2025-02-21 13:40   ` Philippe Mathieu-Daudé
2025-02-17 12:50 ` [PATCH 04/10] fpu: Make targets specify whether floatx80 Inf can have Int bit clear Peter Maydell
2025-02-17 18:13   ` Richard Henderson
2025-02-21 13:12   ` Philippe Mathieu-Daudé
2025-02-17 12:50 ` [PATCH 05/10] fpu: Make floatx80 invalid encoding settable at runtime Peter Maydell
2025-02-17 18:45   ` Richard Henderson
2025-02-21 13:14   ` Philippe Mathieu-Daudé
2025-02-17 12:50 ` [PATCH 06/10] fpu: Move m68k_denormal fmt flag into floatx80_behaviour Peter Maydell
2025-02-17 19:14   ` Richard Henderson
2025-02-20 17:12     ` Peter Maydell
2025-02-20 18:39       ` Richard Henderson
2025-02-20 18:54         ` Peter Maydell
2025-02-21 14:14           ` Philippe Mathieu-Daudé
2025-02-21 22:24           ` Richard Henderson
2025-02-17 12:50 ` [PATCH 07/10] fpu: Always decide no_signaling_nans() at runtime Peter Maydell
2025-02-17 13:13   ` Philippe Mathieu-Daudé
2025-02-17 19:25   ` Richard Henderson
2025-02-17 12:50 ` [PATCH 08/10] fpu: Always decide snan_bit_is_one() " Peter Maydell
2025-02-17 13:15   ` Philippe Mathieu-Daudé
2025-02-17 19:26   ` Richard Henderson
2025-02-17 12:50 ` [PATCH 09/10] fpu: Don't compile-time disable hardfloat for PPC targets Peter Maydell
2025-02-17 19:27   ` Richard Henderson
2025-02-17 12:50 ` [PATCH 10/10] fpu: Build only once Peter Maydell
2025-02-17 19:28   ` Richard Henderson
2025-02-20  8:48 ` [PATCH 00/10] fpu: Remove remaining target ifdefs and build " Philippe Mathieu-Daudé
2025-02-20  9:00   ` Philippe Mathieu-Daudé
2025-02-21 10:48   ` Philippe Mathieu-Daudé
2025-02-21 13:05 ` Philippe Mathieu-Daudé
2025-02-21 13:28   ` Peter Maydell
2025-02-21 13:48     ` Philippe Mathieu-Daudé
2025-02-21 14:41 ` Philippe Mathieu-Daudé [this message]
2025-02-21 15:19   ` Peter Maydell
2025-02-21 16:21     ` Philippe Mathieu-Daudé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=93307c7c-c29f-4061-94ae-461bf196ff41@linaro.org \
    --to=philmd@linaro.org \
    --cc=alex.bennee@linaro.org \
    --cc=eduardo@habkost.net \
    --cc=laurent@vivier.eu \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).