[PATCH 0/2] m68k fpu fixes

[PATCH 0/2] m68k fpu fixes

[KONRAD Frederic]

Here are two fixes for the m68k targets:
  - the first one implements a fix about infinity handling when converting
    it from extended format to double or single format.
  - the second fixes the length of the floating points registers for GDB.

Changes:
  * target/m68k/cpu.c: created DEFINE_M68K_CPU_TYPE_CF and dropped
                       DEFINE_M68K_CPU_TYPE_WITH_CLASS (Suggested by Laurent)
  * target/m68k/cpu.c: reworked DEFINE_M68K_CPU_TYPE_* to guess the
                       cpu_initfn from the cpu_model (Suggested by Laurent).

Cheers,
Fred

KONRAD Frederic (2):
  softfloat: m68k: infinity is a valid encoding
  target/m68k: fix gdb for m68xxx

 configure               |  2 +-
 gdb-xml/m68k-core.xml   | 29 +++++++++++++++++++++++++++
 include/fpu/softfloat.h |  5 +++++
 target/m68k/cpu.c       | 52 +++++++++++++++++++++++++++++++++++--------------
 4 files changed, 72 insertions(+), 16 deletions(-)
 create mode 100644 gdb-xml/m68k-core.xml

-- 
1.8.3.1

[PATCH 1/2] softfloat: m68k: infinity is a valid encoding

[KONRAD Frederic]

The MC68881 say about infinities (3.2.4):

"*For the extended precision format, the most significant bit of the
mantissa (the integer bit) is a don't care."

https://www.nxp.com/docs/en/reference-manual/MC68881UM.pdf

The m68k extended format is implemented with the floatx80 and
floatx80_invalid_encoding currently treats 0x7fff00000000000000000000 as
an invalid encoding.  This patch fixes floatx80_invalid_encoding so it
accepts that the most significant bit of the mantissa can be 0.

This bug can be revealed with the following code which pushes extended
infinity on the stack as a double and then reloads it as a double.  It
should normally be converted and read back as infinity and is currently
read back as nan:

        .global _start
        .text
_start:
        lea val, %a0
        lea fp, %fp
        fmovex (%a0), %fp0
        fmoved %fp0, %fp@(-8)
        fmoved %fp@(-8), %fp0
end:
        bra end

.align 0x4
val:
        .fill 1, 4, 0x7fff0000
        .fill 1, 4, 0x00000000
        .fill 1, 4, 0x00000000
.align 0x4
        .fill 0x100, 1, 0
fp:

-------------

(gdb) tar rem :1234
Remote debugging using :1234
_start () at main.S:5
5              lea val, %a0
(gdb) display $fp0
1: $fp0 = nan(0xffffffffffffffff)
(gdb) si
6             lea fp, %fp
1: $fp0 = nan(0xffffffffffffffff)
(gdb) si
_start () at main.S:7
7              fmovex (%a0), %fp0
1: $fp0 = nan(0xffffffffffffffff)
(gdb) si
8             fmoved %fp0, %fp@(-8)
1: $fp0 = inf
(gdb) si
9             fmoved %fp@(-8), %fp0
1: $fp0 = inf
(gdb) si
end () at main.S:12
12          bra end
1: $fp0 = nan(0xfffffffffffff800)
(gdb) x/1xg $fp-8
0x40000120 <val+260>:   0x7fffffffffffffff

Signed-off-by: KONRAD Frederic <frederic.konrad@adacore.com>
---
 include/fpu/softfloat.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index ecb8ba0..dc80298 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -688,7 +688,12 @@ static inline int floatx80_is_any_nan(floatx80 a)
 *----------------------------------------------------------------------------*/
 static inline bool floatx80_invalid_encoding(floatx80 a)
 {
+#if defined(TARGET_M68K)
+    return (a.low & (1ULL << 63)) == 0 && (((a.high & 0x7FFF) != 0)
+                                           && (a.high != 0x7FFF));
+#else
     return (a.low & (1ULL << 63)) == 0 && (a.high & 0x7FFF) != 0;
+#endif
 }
 
 #define floatx80_zero make_floatx80(0x0000, 0x0000000000000000LL)
-- 
1.8.3.1

[PATCH 2/2] target/m68k: fix gdb for m68xxx

[KONRAD Frederic]

Currently "cf-core.xml" is sent to GDB when using any m68k flavor.  Thing is
it uses the "org.gnu.gdb.coldfire.core" feature name and gdb 8.3 then expects
a coldfire FPU instead of the default m68881 FPU.

This is not OK because the m68881 floats registers are 96 bits wide so it
crashes GDB with the following error message:

(gdb) target remote localhost:7960
Remote debugging using localhost:7960
warning: Register "fp0" has an unsupported size (96 bits)
warning: Register "fp1" has an unsupported size (96 bits)
...
Remote 'g' packet reply is too long (expected 148 bytes, got 180 bytes):    \
  00000000000[...]0000

With this patch: qemu-system-m68k -M none -cpu m68020 -s -S

(gdb) tar rem :1234
Remote debugging using :1234
warning: No executable has been specified and target does not support
determining executable automatically.  Try using the "file" command.
0x00000000 in ?? ()
(gdb) p $fp0
$1 = nan(0xffffffffffffffff)

Signed-off-by: KONRAD Frederic <frederic.konrad@adacore.com>
---
 configure             |  2 +-
 gdb-xml/m68k-core.xml | 29 ++++++++++++++++++++++++++++
 target/m68k/cpu.c     | 52 ++++++++++++++++++++++++++++++++++++---------------
 3 files changed, 67 insertions(+), 16 deletions(-)
 create mode 100644 gdb-xml/m68k-core.xml

diff --git a/configure b/configure
index 23b5e93..2b07b85 100755
--- a/configure
+++ b/configure
@@ -7825,7 +7825,7 @@ case "$target_name" in
   ;;
   m68k)
     bflt="yes"
-    gdb_xml_files="cf-core.xml cf-fp.xml m68k-fp.xml"
+    gdb_xml_files="cf-core.xml cf-fp.xml m68k-core.xml m68k-fp.xml"
     TARGET_SYSTBL_ABI=common
   ;;
   microblaze|microblazeel)
diff --git a/gdb-xml/m68k-core.xml b/gdb-xml/m68k-core.xml
new file mode 100644
index 0000000..5b092d2
--- /dev/null
+++ b/gdb-xml/m68k-core.xml
@@ -0,0 +1,29 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2008 Free Software Foundation, Inc.
+
+     Copying and distribution of this file, with or without modification,
+     are permitted in any medium without royalty provided the copyright
+     notice and this notice are preserved.  -->
+<!DOCTYPE feature SYSTEM "gdb-target.dtd">
+<feature name="org.gnu.gdb.m68k.core">
+  <reg name="d0" bitsize="32"/>
+  <reg name="d1" bitsize="32"/>
+  <reg name="d2" bitsize="32"/>
+  <reg name="d3" bitsize="32"/>
+  <reg name="d4" bitsize="32"/>
+  <reg name="d5" bitsize="32"/>
+  <reg name="d6" bitsize="32"/>
+  <reg name="d7" bitsize="32"/>
+  <reg name="a0" bitsize="32" type="data_ptr"/>
+  <reg name="a1" bitsize="32" type="data_ptr"/>
+  <reg name="a2" bitsize="32" type="data_ptr"/>
+  <reg name="a3" bitsize="32" type="data_ptr"/>
+  <reg name="a4" bitsize="32" type="data_ptr"/>
+  <reg name="a5" bitsize="32" type="data_ptr"/>
+  <reg name="fp" bitsize="32" type="data_ptr"/>
+  <reg name="sp" bitsize="32" type="data_ptr"/>
+
+  <reg name="ps" bitsize="32"/>
+  <reg name="pc" bitsize="32" type="code_ptr"/>
+
+</feature>
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index 9445fcd..72c5451 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -292,16 +292,38 @@ static void m68k_cpu_class_init(ObjectClass *c, void *data)
     cc->tcg_initialize = m68k_tcg_init;
 
     cc->gdb_num_core_regs = 18;
-    cc->gdb_core_xml_file = "cf-core.xml";
 
     dc->vmsd = &vmstate_m68k_cpu;
 }
 
-#define DEFINE_M68K_CPU_TYPE(cpu_model, initfn) \
-    {                                           \
-        .name = M68K_CPU_TYPE_NAME(cpu_model),  \
-        .instance_init = initfn,                \
-        .parent = TYPE_M68K_CPU,                \
+static void m68k_cpu_class_init_cf_core(ObjectClass *c, void *data)
+{
+    CPUClass *cc = CPU_CLASS(c);
+
+    cc->gdb_core_xml_file = "cf-core.xml";
+}
+
+#define DEFINE_M68K_CPU_TYPE_CF(model)               \
+    {                                                \
+        .name = M68K_CPU_TYPE_NAME(#model),          \
+        .instance_init = model##_cpu_initfn,         \
+        .parent = TYPE_M68K_CPU,                     \
+        .class_init = m68k_cpu_class_init_cf_core    \
+    }
+
+static void m68k_cpu_class_init_m68k_core(ObjectClass *c, void *data)
+{
+    CPUClass *cc = CPU_CLASS(c);
+
+    cc->gdb_core_xml_file = "m68k-core.xml";
+}
+
+#define DEFINE_M68K_CPU_TYPE_M68K(model)             \
+    {                                                \
+        .name = M68K_CPU_TYPE_NAME(#model),          \
+        .instance_init = model##_cpu_initfn,         \
+        .parent = TYPE_M68K_CPU,                     \
+        .class_init = m68k_cpu_class_init_m68k_core  \
     }
 
 static const TypeInfo m68k_cpus_type_infos[] = {
@@ -314,15 +336,15 @@ static const TypeInfo m68k_cpus_type_infos[] = {
         .class_size = sizeof(M68kCPUClass),
         .class_init = m68k_cpu_class_init,
     },
-    DEFINE_M68K_CPU_TYPE("m68000", m68000_cpu_initfn),
-    DEFINE_M68K_CPU_TYPE("m68020", m68020_cpu_initfn),
-    DEFINE_M68K_CPU_TYPE("m68030", m68030_cpu_initfn),
-    DEFINE_M68K_CPU_TYPE("m68040", m68040_cpu_initfn),
-    DEFINE_M68K_CPU_TYPE("m68060", m68060_cpu_initfn),
-    DEFINE_M68K_CPU_TYPE("m5206", m5206_cpu_initfn),
-    DEFINE_M68K_CPU_TYPE("m5208", m5208_cpu_initfn),
-    DEFINE_M68K_CPU_TYPE("cfv4e", cfv4e_cpu_initfn),
-    DEFINE_M68K_CPU_TYPE("any", any_cpu_initfn),
+    DEFINE_M68K_CPU_TYPE_M68K(m68000),
+    DEFINE_M68K_CPU_TYPE_M68K(m68020),
+    DEFINE_M68K_CPU_TYPE_M68K(m68030),
+    DEFINE_M68K_CPU_TYPE_M68K(m68040),
+    DEFINE_M68K_CPU_TYPE_M68K(m68060),
+    DEFINE_M68K_CPU_TYPE_CF(m5206),
+    DEFINE_M68K_CPU_TYPE_CF(m5208),
+    DEFINE_M68K_CPU_TYPE_CF(cfv4e),
+    DEFINE_M68K_CPU_TYPE_CF(any),
 };
 
 DEFINE_TYPES(m68k_cpus_type_infos)
-- 
1.8.3.1

Re: [PATCH 1/2] softfloat: m68k: infinity is a valid encoding

[Alex Bennée]

KONRAD Frederic <frederic.konrad@adacore.com> writes:

> The MC68881 say about infinities (3.2.4):
>
> "*For the extended precision format, the most significant bit of the
> mantissa (the integer bit) is a don't care."
>
> https://www.nxp.com/docs/en/reference-manual/MC68881UM.pdf
>
> The m68k extended format is implemented with the floatx80 and
> floatx80_invalid_encoding currently treats 0x7fff00000000000000000000 as
> an invalid encoding.  This patch fixes floatx80_invalid_encoding so it
> accepts that the most significant bit of the mantissa can be 0.
>
> This bug can be revealed with the following code which pushes extended
> infinity on the stack as a double and then reloads it as a double.  It
> should normally be converted and read back as infinity and is currently
> read back as nan:

Do you have any real HW on which you could record some .ref files for
the various multiarch float tests we have (float_convs/float_madds)?
Does this different of invalid encoding show up when you add them?

>
>         .global _start
>         .text
> _start:
>         lea val, %a0
>         lea fp, %fp
>         fmovex (%a0), %fp0
>         fmoved %fp0, %fp@(-8)
>         fmoved %fp@(-8), %fp0
> end:
>         bra end
>
> .align 0x4
> val:
>         .fill 1, 4, 0x7fff0000
>         .fill 1, 4, 0x00000000
>         .fill 1, 4, 0x00000000
> .align 0x4
>         .fill 0x100, 1, 0
> fp:
>
> -------------
>
> (gdb) tar rem :1234
> Remote debugging using :1234
> _start () at main.S:5
> 5              lea val, %a0
> (gdb) display $fp0
> 1: $fp0 = nan(0xffffffffffffffff)
> (gdb) si
> 6             lea fp, %fp
> 1: $fp0 = nan(0xffffffffffffffff)
> (gdb) si
> _start () at main.S:7
> 7              fmovex (%a0), %fp0
> 1: $fp0 = nan(0xffffffffffffffff)
> (gdb) si
> 8             fmoved %fp0, %fp@(-8)
> 1: $fp0 = inf
> (gdb) si
> 9             fmoved %fp@(-8), %fp0
> 1: $fp0 = inf
> (gdb) si
> end () at main.S:12
> 12          bra end
> 1: $fp0 = nan(0xfffffffffffff800)
> (gdb) x/1xg $fp-8
> 0x40000120 <val+260>:   0x7fffffffffffffff
>
> Signed-off-by: KONRAD Frederic <frederic.konrad@adacore.com>
> ---
>  include/fpu/softfloat.h | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
> index ecb8ba0..dc80298 100644
> --- a/include/fpu/softfloat.h
> +++ b/include/fpu/softfloat.h
> @@ -688,7 +688,12 @@ static inline int floatx80_is_any_nan(floatx80 a)
>  *----------------------------------------------------------------------------*/
>  static inline bool floatx80_invalid_encoding(floatx80 a)
>  {
> +#if defined(TARGET_M68K)
> +    return (a.low & (1ULL << 63)) == 0 && (((a.high & 0x7FFF) != 0)
> +                                           && (a.high != 0x7FFF));
> +#else
>      return (a.low & (1ULL << 63)) == 0 && (a.high & 0x7FFF) != 0;
> +#endif
>  }
>  
>  #define floatx80_zero make_floatx80(0x0000, 0x0000000000000000LL)


-- 
Alex Bennée

Re: [PATCH 1/2] softfloat: m68k: infinity is a valid encoding

[Laurent Vivier]

Le 28/04/2020 à 19:17, KONRAD Frederic a écrit :
> The MC68881 say about infinities (3.2.4):
> 
> "*For the extended precision format, the most significant bit of the
> mantissa (the integer bit) is a don't care."
> 
> https://www.nxp.com/docs/en/reference-manual/MC68881UM.pdf

As we use 68040 I refer to:

https://www.nxp.com/files-static/archives/doc/ref_manual/M68000PRM.pdf

> 
> The m68k extended format is implemented with the floatx80 and
> floatx80_invalid_encoding currently treats 0x7fff00000000000000000000 as
> an invalid encoding.  This patch fixes floatx80_invalid_encoding so it
> accepts that the most significant bit of the mantissa can be 0.
> 
> This bug can be revealed with the following code which pushes extended
> infinity on the stack as a double and then reloads it as a double.  It
> should normally be converted and read back as infinity and is currently
> read back as nan:
> 
>         .global _start
>         .text
> _start:
>         lea val, %a0
>         lea fp, %fp
>         fmovex (%a0), %fp0
>         fmoved %fp0, %fp@(-8)
>         fmoved %fp@(-8), %fp0
> end:
>         bra end
> 
> .align 0x4
> val:
>         .fill 1, 4, 0x7fff0000
>         .fill 1, 4, 0x00000000
>         .fill 1, 4, 0x00000000
> .align 0x4
>         .fill 0x100, 1, 0
> fp:
> 
> -------------
> 
> (gdb) tar rem :1234
> Remote debugging using :1234
> _start () at main.S:5
> 5              lea val, %a0
> (gdb) display $fp0
> 1: $fp0 = nan(0xffffffffffffffff)
> (gdb) si
> 6             lea fp, %fp
> 1: $fp0 = nan(0xffffffffffffffff)
> (gdb) si
> _start () at main.S:7
> 7              fmovex (%a0), %fp0
> 1: $fp0 = nan(0xffffffffffffffff)
> (gdb) si
> 8             fmoved %fp0, %fp@(-8)
> 1: $fp0 = inf
> (gdb) si
> 9             fmoved %fp@(-8), %fp0
> 1: $fp0 = inf
> (gdb) si
> end () at main.S:12
> 12          bra end
> 1: $fp0 = nan(0xfffffffffffff800)
> (gdb) x/1xg $fp-8
> 0x40000120 <val+260>:   0x7fffffffffffffff
> 
> Signed-off-by: KONRAD Frederic <frederic.konrad@adacore.com>
> ---
>  include/fpu/softfloat.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
> index ecb8ba0..dc80298 100644
> --- a/include/fpu/softfloat.h
> +++ b/include/fpu/softfloat.h
> @@ -688,7 +688,12 @@ static inline int floatx80_is_any_nan(floatx80 a)
>  *----------------------------------------------------------------------------*/
>  static inline bool floatx80_invalid_encoding(floatx80 a)
>  {
> +#if defined(TARGET_M68K)
> +    return (a.low & (1ULL << 63)) == 0 && (((a.high & 0x7FFF) != 0)
> +                                           && (a.high != 0x7FFF));
> +#else
>      return (a.low & (1ULL << 63)) == 0 && (a.high & 0x7FFF) != 0;
> +#endif
>  }
>  
>  #define floatx80_zero make_floatx80(0x0000, 0x0000000000000000LL)
> 

This is denormalized numbers and should generate an exception.

I tried something like that in the past:

https://patchew.org/QEMU/20170207005930.28327-1-laurent@vivier.eu/20170207005930.28327-3-laurent@vivier.eu/

Pierre tried recently:
https://patchew.org/QEMU/1615bbe5-3033-3b76-5cfb-52e343dc4d67@freepascal.org/

See "1.6.2 Denormalized Numbers" in M68000 FAMILY PROGRAMMER’S REFERENCE
MANUAL.

"Since the extended-precision data format has an explicit integer bit, a
number can be formatted with a nonzero exponent, less than the maximum
value, and a zero integer bit. The IEEE 754 standard does not define a
zero integer bit. Such a number is an unnormalized number. Hardware does
not directly support denormalized and unnormalized numbers, but
implicitly supports them by trapping them as unimplemented data types,
allowing efficient conversion in software."

But m68k FPU exceptions are not currently implemented in QEMU.

Thanks,
Laurent

Re: [PATCH 1/2] softfloat: m68k: infinity is a valid encoding

[Laurent Vivier]

Le 28/04/2020 à 20:43, Alex Bennée a écrit :
> 
> KONRAD Frederic <frederic.konrad@adacore.com> writes:
> 
>> The MC68881 say about infinities (3.2.4):
>>
>> "*For the extended precision format, the most significant bit of the
>> mantissa (the integer bit) is a don't care."
>>
>> https://www.nxp.com/docs/en/reference-manual/MC68881UM.pdf
>>
>> The m68k extended format is implemented with the floatx80 and
>> floatx80_invalid_encoding currently treats 0x7fff00000000000000000000 as
>> an invalid encoding.  This patch fixes floatx80_invalid_encoding so it
>> accepts that the most significant bit of the mantissa can be 0.
>>
>> This bug can be revealed with the following code which pushes extended
>> infinity on the stack as a double and then reloads it as a double.  It
>> should normally be converted and read back as infinity and is currently
>> read back as nan:
> 
> Do you have any real HW on which you could record some .ref files for
> the various multiarch float tests we have (float_convs/float_madds)?
> Does this different of invalid encoding show up when you add them?

On my side, in the past when I started to implement m68k FPU, I used
TestFloat and SoftFloat I have ported to m68k and I compare the result
in QEMU and in a Quadra 800.

https://github.com/vivier/m68k-testfloat
https://github.com/vivier/m68k-softfloat

I also used the gcc and libc testsuite to detect problems but this was a
very slow process...

I have also ported RISU to m68k, but I didn't add FPU test in it (does
it support FPU test?).

Thanks,
Laurent

Re: [PATCH 2/2] target/m68k: fix gdb for m68xxx

[Laurent Vivier]

Le 28/04/2020 à 19:17, KONRAD Frederic a écrit :
> Currently "cf-core.xml" is sent to GDB when using any m68k flavor.  Thing is
> it uses the "org.gnu.gdb.coldfire.core" feature name and gdb 8.3 then expects
> a coldfire FPU instead of the default m68881 FPU.
> 
> This is not OK because the m68881 floats registers are 96 bits wide so it
> crashes GDB with the following error message:
> 
> (gdb) target remote localhost:7960
> Remote debugging using localhost:7960
> warning: Register "fp0" has an unsupported size (96 bits)
> warning: Register "fp1" has an unsupported size (96 bits)
> ...
> Remote 'g' packet reply is too long (expected 148 bytes, got 180 bytes):    \
>   00000000000[...]0000
> 
> With this patch: qemu-system-m68k -M none -cpu m68020 -s -S
> 
> (gdb) tar rem :1234
> Remote debugging using :1234
> warning: No executable has been specified and target does not support
> determining executable automatically.  Try using the "file" command.
> 0x00000000 in ?? ()
> (gdb) p $fp0
> $1 = nan(0xffffffffffffffff)
> 
> Signed-off-by: KONRAD Frederic <frederic.konrad@adacore.com>
> ---
>  configure             |  2 +-
>  gdb-xml/m68k-core.xml | 29 ++++++++++++++++++++++++++++
>  target/m68k/cpu.c     | 52 ++++++++++++++++++++++++++++++++++++---------------
>  3 files changed, 67 insertions(+), 16 deletions(-)
>  create mode 100644 gdb-xml/m68k-core.xml

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH 1/2] softfloat: m68k: infinity is a valid encoding

[Alex Bennée]

Laurent Vivier <laurent@vivier.eu> writes:

> Le 28/04/2020 à 20:43, Alex Bennée a écrit :
>> 
>> KONRAD Frederic <frederic.konrad@adacore.com> writes:
>> 
>>> The MC68881 say about infinities (3.2.4):
>>>
>>> "*For the extended precision format, the most significant bit of the
>>> mantissa (the integer bit) is a don't care."
>>>
>>> https://www.nxp.com/docs/en/reference-manual/MC68881UM.pdf
>>>
>>> The m68k extended format is implemented with the floatx80 and
>>> floatx80_invalid_encoding currently treats 0x7fff00000000000000000000 as
>>> an invalid encoding.  This patch fixes floatx80_invalid_encoding so it
>>> accepts that the most significant bit of the mantissa can be 0.
>>>
>>> This bug can be revealed with the following code which pushes extended
>>> infinity on the stack as a double and then reloads it as a double.  It
>>> should normally be converted and read back as infinity and is currently
>>> read back as nan:
>> 
>> Do you have any real HW on which you could record some .ref files for
>> the various multiarch float tests we have (float_convs/float_madds)?
>> Does this different of invalid encoding show up when you add them?
>
> On my side, in the past when I started to implement m68k FPU, I used
> TestFloat and SoftFloat I have ported to m68k and I compare the result
> in QEMU and in a Quadra 800.

Surely TestFloat and SoftFloat is all emulation though?

Anyway if you have a Quadra 800 running Linux could you generate some
.ref files for the float_convs and float_madds test cases. The binaries
are static so you should just be able to copy them and run.

> https://github.com/vivier/m68k-testfloat
> https://github.com/vivier/m68k-softfloat

Ahh I see you have sys_float functions to compare to TestFloat. 

> I also used the gcc and libc testsuite to detect problems but this was a
> very slow process...
>
> I have also ported RISU to m68k, but I didn't add FPU test in it (does
> it support FPU test?).

There is no reason why it couldn't. The FPU support would basically be
ensuring the appropriate registers are saved out of the context. I did
similar when we expanded the aarch64 RISU to support SVE. In fact
looking at the code:

    for (i = 0; i < 8; i++) {
        if (m->fpregs.f_fpregs[i][0] != a->fpregs.f_fpregs[i][0] ||
            m->fpregs.f_fpregs[i][1] != a->fpregs.f_fpregs[i][1] ||
            m->fpregs.f_fpregs[i][2] != a->fpregs.f_fpregs[i][2]) {
            return 0;
        }
    }

it seems the fpregs are included and tested so it should be good.

That said RISU's random instruction approach means most floating point
numbers very quickly become tend to NaNs which is part of the reason I
wrote the float_convs/float_madds tests which try to be more systematic
in exercising the range of float types (normals, denormals, min and max
etc). Maybe we could improve risugen's seeding of floating point values
to better exercise FP ops rather than just dumping random stuff there.

>
> Thanks,
> Laurent


-- 
Alex Bennée

Re: [PATCH 2/2] target/m68k: fix gdb for m68xxx

[Alex Bennée]

Laurent Vivier <laurent@vivier.eu> writes:

> Le 28/04/2020 à 19:17, KONRAD Frederic a écrit :
>> Currently "cf-core.xml" is sent to GDB when using any m68k flavor.  Thing is
>> it uses the "org.gnu.gdb.coldfire.core" feature name and gdb 8.3 then expects
>> a coldfire FPU instead of the default m68881 FPU.
>> 
>> This is not OK because the m68881 floats registers are 96 bits wide so it
>> crashes GDB with the following error message:
>> 
>> (gdb) target remote localhost:7960
>> Remote debugging using localhost:7960
>> warning: Register "fp0" has an unsupported size (96 bits)
>> warning: Register "fp1" has an unsupported size (96 bits)
>> ...
>> Remote 'g' packet reply is too long (expected 148 bytes, got 180 bytes):    \
>>   00000000000[...]0000
>> 
>> With this patch: qemu-system-m68k -M none -cpu m68020 -s -S
>> 
>> (gdb) tar rem :1234
>> Remote debugging using :1234
>> warning: No executable has been specified and target does not support
>> determining executable automatically.  Try using the "file" command.
>> 0x00000000 in ?? ()
>> (gdb) p $fp0
>> $1 = nan(0xffffffffffffffff)
>> 
>> Signed-off-by: KONRAD Frederic <frederic.konrad@adacore.com>
>> ---
>>  configure             |  2 +-
>>  gdb-xml/m68k-core.xml | 29 ++++++++++++++++++++++++++++
>>  target/m68k/cpu.c     | 52 ++++++++++++++++++++++++++++++++++++---------------
>>  3 files changed, 67 insertions(+), 16 deletions(-)
>>  create mode 100644 gdb-xml/m68k-core.xml
>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Are you going to take this through your tree or do you want me to add it
to my small pile of gdbstub fixes?

-- 
Alex Bennée

Re: [PATCH 2/2] target/m68k: fix gdb for m68xxx

[Laurent Vivier]

Le 29/04/2020 à 11:28, Alex Bennée a écrit :
> 
> Laurent Vivier <laurent@vivier.eu> writes:
> 
>> Le 28/04/2020 à 19:17, KONRAD Frederic a écrit :
>>> Currently "cf-core.xml" is sent to GDB when using any m68k flavor.  Thing is
>>> it uses the "org.gnu.gdb.coldfire.core" feature name and gdb 8.3 then expects
>>> a coldfire FPU instead of the default m68881 FPU.
>>>
>>> This is not OK because the m68881 floats registers are 96 bits wide so it
>>> crashes GDB with the following error message:
>>>
>>> (gdb) target remote localhost:7960
>>> Remote debugging using localhost:7960
>>> warning: Register "fp0" has an unsupported size (96 bits)
>>> warning: Register "fp1" has an unsupported size (96 bits)
>>> ...
>>> Remote 'g' packet reply is too long (expected 148 bytes, got 180 bytes):    \
>>>   00000000000[...]0000
>>>
>>> With this patch: qemu-system-m68k -M none -cpu m68020 -s -S
>>>
>>> (gdb) tar rem :1234
>>> Remote debugging using :1234
>>> warning: No executable has been specified and target does not support
>>> determining executable automatically.  Try using the "file" command.
>>> 0x00000000 in ?? ()
>>> (gdb) p $fp0
>>> $1 = nan(0xffffffffffffffff)
>>>
>>> Signed-off-by: KONRAD Frederic <frederic.konrad@adacore.com>
>>> ---
>>>  configure             |  2 +-
>>>  gdb-xml/m68k-core.xml | 29 ++++++++++++++++++++++++++++
>>>  target/m68k/cpu.c     | 52 ++++++++++++++++++++++++++++++++++++---------------
>>>  3 files changed, 67 insertions(+), 16 deletions(-)
>>>  create mode 100644 gdb-xml/m68k-core.xml
>>
>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> 
> Are you going to take this through your tree or do you want me to add it
> to my small pile of gdbstub fixes?
> 

Please add it to your pile.

Thanks,
Laurent

Re: [PATCH 1/2] softfloat: m68k: infinity is a valid encoding

[Laurent Vivier]

Le 29/04/2020 à 11:26, Alex Bennée a écrit :
> 
> Laurent Vivier <laurent@vivier.eu> writes:
> 
>> Le 28/04/2020 à 20:43, Alex Bennée a écrit :
>>>
>>> KONRAD Frederic <frederic.konrad@adacore.com> writes:
>>>
>>>> The MC68881 say about infinities (3.2.4):
>>>>
>>>> "*For the extended precision format, the most significant bit of the
>>>> mantissa (the integer bit) is a don't care."
>>>>
>>>> https://www.nxp.com/docs/en/reference-manual/MC68881UM.pdf
>>>>
>>>> The m68k extended format is implemented with the floatx80 and
>>>> floatx80_invalid_encoding currently treats 0x7fff00000000000000000000 as
>>>> an invalid encoding.  This patch fixes floatx80_invalid_encoding so it
>>>> accepts that the most significant bit of the mantissa can be 0.
>>>>
>>>> This bug can be revealed with the following code which pushes extended
>>>> infinity on the stack as a double and then reloads it as a double.  It
>>>> should normally be converted and read back as infinity and is currently
>>>> read back as nan:
>>>
>>> Do you have any real HW on which you could record some .ref files for
>>> the various multiarch float tests we have (float_convs/float_madds)?
>>> Does this different of invalid encoding show up when you add them?
>>
>> On my side, in the past when I started to implement m68k FPU, I used
>> TestFloat and SoftFloat I have ported to m68k and I compare the result
>> in QEMU and in a Quadra 800.
> 
> Surely TestFloat and SoftFloat is all emulation though?
> 
> Anyway if you have a Quadra 800 running Linux could you generate some
> .ref files for the float_convs and float_madds test cases. The binaries
> are static so you should just be able to copy them and run.
>
Is there any HOWTO somewhere? Or should I dig into the code as usual?

Thanks,
Laurent

Re: [PATCH 1/2] softfloat: m68k: infinity is a valid encoding

[Alex Bennée]

Laurent Vivier <laurent@vivier.eu> writes:

> Le 29/04/2020 à 11:26, Alex Bennée a écrit :
>> 
>> Laurent Vivier <laurent@vivier.eu> writes:
>> 
>>> Le 28/04/2020 à 20:43, Alex Bennée a écrit :
>>>>
>>>> KONRAD Frederic <frederic.konrad@adacore.com> writes:
>>>>
>>>>> The MC68881 say about infinities (3.2.4):
>>>>>
>>>>> "*For the extended precision format, the most significant bit of the
>>>>> mantissa (the integer bit) is a don't care."
>>>>>
>>>>> https://www.nxp.com/docs/en/reference-manual/MC68881UM.pdf
>>>>>
>>>>> The m68k extended format is implemented with the floatx80 and
>>>>> floatx80_invalid_encoding currently treats 0x7fff00000000000000000000 as
>>>>> an invalid encoding.  This patch fixes floatx80_invalid_encoding so it
>>>>> accepts that the most significant bit of the mantissa can be 0.
>>>>>
>>>>> This bug can be revealed with the following code which pushes extended
>>>>> infinity on the stack as a double and then reloads it as a double.  It
>>>>> should normally be converted and read back as infinity and is currently
>>>>> read back as nan:
>>>>
>>>> Do you have any real HW on which you could record some .ref files for
>>>> the various multiarch float tests we have (float_convs/float_madds)?
>>>> Does this different of invalid encoding show up when you add them?
>>>
>>> On my side, in the past when I started to implement m68k FPU, I used
>>> TestFloat and SoftFloat I have ported to m68k and I compare the result
>>> in QEMU and in a Quadra 800.
>> 
>> Surely TestFloat and SoftFloat is all emulation though?
>> 
>> Anyway if you have a Quadra 800 running Linux could you generate some
>> .ref files for the float_convs and float_madds test cases. The binaries
>> are static so you should just be able to copy them and run.
>>
> Is there any HOWTO somewhere? Or should I dig into the code as usual?

On your known good HW:

  ./float_convs > float_convs.ref

and copy the file into tests/tcg/m68k and the:

  run-float_%: float_%
          $(call run-test,$<, $(QEMU) $(QEMU_OPTS) $<,"$< on $(TARGET_NAME)")
          $(call conditional-diff-out,$<,$(SRC_PATH)/tests/tcg/$(TARGET_NAME)/$<.ref)

should automatically include the diff on the next run.

>
> Thanks,
> Laurent


-- 
Alex Bennée

Re: [PATCH 2/2] target/m68k: fix gdb for m68xxx

[KONRAD Frederic]

Le 4/29/20 à 11:38 AM, Laurent Vivier a écrit :
> Le 29/04/2020 à 11:28, Alex Bennée a écrit :
>>
>> Laurent Vivier <laurent@vivier.eu> writes:
>>
>>> Le 28/04/2020 à 19:17, KONRAD Frederic a écrit :
>>>> Currently "cf-core.xml" is sent to GDB when using any m68k flavor.  Thing is
>>>> it uses the "org.gnu.gdb.coldfire.core" feature name and gdb 8.3 then expects
>>>> a coldfire FPU instead of the default m68881 FPU.
>>>>
>>>> This is not OK because the m68881 floats registers are 96 bits wide so it
>>>> crashes GDB with the following error message:
>>>>
>>>> (gdb) target remote localhost:7960
>>>> Remote debugging using localhost:7960
>>>> warning: Register "fp0" has an unsupported size (96 bits)
>>>> warning: Register "fp1" has an unsupported size (96 bits)
>>>> ...
>>>> Remote 'g' packet reply is too long (expected 148 bytes, got 180 bytes):    \
>>>>    00000000000[...]0000
>>>>
>>>> With this patch: qemu-system-m68k -M none -cpu m68020 -s -S
>>>>
>>>> (gdb) tar rem :1234
>>>> Remote debugging using :1234
>>>> warning: No executable has been specified and target does not support
>>>> determining executable automatically.  Try using the "file" command.
>>>> 0x00000000 in ?? ()
>>>> (gdb) p $fp0
>>>> $1 = nan(0xffffffffffffffff)
>>>>
>>>> Signed-off-by: KONRAD Frederic <frederic.konrad@adacore.com>
>>>> ---
>>>>   configure             |  2 +-
>>>>   gdb-xml/m68k-core.xml | 29 ++++++++++++++++++++++++++++
>>>>   target/m68k/cpu.c     | 52 ++++++++++++++++++++++++++++++++++++---------------
>>>>   3 files changed, 67 insertions(+), 16 deletions(-)
>>>>   create mode 100644 gdb-xml/m68k-core.xml
>>>
>>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>>
>> Are you going to take this through your tree or do you want me to add it
>> to my small pile of gdbstub fixes?
>>
> 
> Please add it to your pile.
> 
> Thanks,
> Laurent
> 

Thanks!

Re: [PATCH 1/2] softfloat: m68k: infinity is a valid encoding

[KONRAD Frederic]

Le 4/29/20 à 10:42 AM, Laurent Vivier a écrit :
> Le 28/04/2020 à 19:17, KONRAD Frederic a écrit :
>> The MC68881 say about infinities (3.2.4):
>>
>> "*For the extended precision format, the most significant bit of the
>> mantissa (the integer bit) is a don't care."
>>
>> https://www.nxp.com/docs/en/reference-manual/MC68881UM.pdf
> 
> As we use 68040 I refer to:
> 
> https://www.nxp.com/files-static/archives/doc/ref_manual/M68000PRM.pdf
> 

[...]

> 
> This is denormalized numbers and should generate an exception.
> 
> I tried something like that in the past:
> 
> https://patchew.org/QEMU/20170207005930.28327-1-laurent@vivier.eu/20170207005930.28327-3-laurent@vivier.eu/
> 
> Pierre tried recently:
> https://patchew.org/QEMU/1615bbe5-3033-3b76-5cfb-52e343dc4d67@freepascal.org/

Arg, yes that's almost the same!  Sorry Pierre I missed this one :(.

> 
> See "1.6.2 Denormalized Numbers" in M68000 FAMILY PROGRAMMER’S REFERENCE
> MANUAL.
> 
> "Since the extended-precision data format has an explicit integer bit, a
> number can be formatted with a nonzero exponent, less than the maximum
> value, and a zero integer bit. The IEEE 754 standard does not define a
> zero integer bit. Such a number is an unnormalized number. Hardware does
> not directly support denormalized and unnormalized numbers, but
> implicitly supports them by trapping them as unimplemented data types,
> allowing efficient conversion in software."
> 
> But m68k FPU exceptions are not currently implemented in QEMU.

Hmm ok, I don't have any m68k with an FPU at hand for testing.  I just tested
with an other simulator and it seems to trap when I load the value in the
register.  So I'm probably chasing the wrong bug here.

Thanks for the tips Laurent!
Fred

> 
> Thanks,
> Laurent
> 

Re: [PATCH 1/2] softfloat: m68k: infinity is a valid encoding

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 1758 bytes --]

Le 29/04/2020 à 11:26, Alex Bennée a écrit :
> 
> Laurent Vivier <laurent@vivier.eu> writes:
> 
>> Le 28/04/2020 à 20:43, Alex Bennée a écrit :
>>>
>>> KONRAD Frederic <frederic.konrad@adacore.com> writes:
>>>
>>>> The MC68881 say about infinities (3.2.4):
>>>>
>>>> "*For the extended precision format, the most significant bit of the
>>>> mantissa (the integer bit) is a don't care."
>>>>
>>>> https://www.nxp.com/docs/en/reference-manual/MC68881UM.pdf
>>>>
>>>> The m68k extended format is implemented with the floatx80 and
>>>> floatx80_invalid_encoding currently treats 0x7fff00000000000000000000 as
>>>> an invalid encoding.  This patch fixes floatx80_invalid_encoding so it
>>>> accepts that the most significant bit of the mantissa can be 0.
>>>>
>>>> This bug can be revealed with the following code which pushes extended
>>>> infinity on the stack as a double and then reloads it as a double.  It
>>>> should normally be converted and read back as infinity and is currently
>>>> read back as nan:
>>>
>>> Do you have any real HW on which you could record some .ref files for
>>> the various multiarch float tests we have (float_convs/float_madds)?
>>> Does this different of invalid encoding show up when you add them?
>>
>> On my side, in the past when I started to implement m68k FPU, I used
>> TestFloat and SoftFloat I have ported to m68k and I compare the result
>> in QEMU and in a Quadra 800.
> 
> Surely TestFloat and SoftFloat is all emulation though?
> 
> Anyway if you have a Quadra 800 running Linux could you generate some
> .ref files for the float_convs and float_madds test cases. The binaries
> are static so you should just be able to copy them and run.
> 
>

Here are the files I have generated on Q800.

Thanks,
Laurent


[-- Attachment #2: float_madds.ref.gz --]
[-- Type: application/gzip, Size: 2859 bytes --]

[-- Attachment #3: float_convs.ref.gz --]
[-- Type: application/gzip, Size: 1117 bytes --]

Re: [PATCH 1/2] softfloat: m68k: infinity is a valid encoding

[Alex Bennée]

Laurent Vivier <laurent@vivier.eu> writes:

> Le 29/04/2020 à 11:26, Alex Bennée a écrit :
>> 
>> Laurent Vivier <laurent@vivier.eu> writes:
>> 
>>> Le 28/04/2020 à 20:43, Alex Bennée a écrit :
>>>>
>>>> KONRAD Frederic <frederic.konrad@adacore.com> writes:
>>>>
>>>>> The MC68881 say about infinities (3.2.4):
>>>>>
>>>>> "*For the extended precision format, the most significant bit of the
>>>>> mantissa (the integer bit) is a don't care."
>>>>>
>>>>> https://www.nxp.com/docs/en/reference-manual/MC68881UM.pdf
>>>>>
>>>>> The m68k extended format is implemented with the floatx80 and
>>>>> floatx80_invalid_encoding currently treats 0x7fff00000000000000000000 as
>>>>> an invalid encoding.  This patch fixes floatx80_invalid_encoding so it
>>>>> accepts that the most significant bit of the mantissa can be 0.
>>>>>
>>>>> This bug can be revealed with the following code which pushes extended
>>>>> infinity on the stack as a double and then reloads it as a double.  It
>>>>> should normally be converted and read back as infinity and is currently
>>>>> read back as nan:
>>>>
>>>> Do you have any real HW on which you could record some .ref files for
>>>> the various multiarch float tests we have (float_convs/float_madds)?
>>>> Does this different of invalid encoding show up when you add them?
>>>
>>> On my side, in the past when I started to implement m68k FPU, I used
>>> TestFloat and SoftFloat I have ported to m68k and I compare the result
>>> in QEMU and in a Quadra 800.
>> 
>> Surely TestFloat and SoftFloat is all emulation though?
>> 
>> Anyway if you have a Quadra 800 running Linux could you generate some
>> .ref files for the float_convs and float_madds test cases. The binaries
>> are static so you should just be able to copy them and run.
>> 
>>
>
> Here are the files I have generated on Q800.

So running those with:

  run-float_convs: QEMU_OPTS += -cpu m68040
  run-float_madds: QEMU_OPTS += -cpu m68040

We see the m68k float needs a fair bit of work from the get go:

      Reference                                            qemu-m68k -cou m68040

  ### Rounding to nearest						### Rounding to nearest
                                                                >	from single: f32(-nan:0xffbfffff)
                                                                >	  to double: f64(-nan:0x00fff7ffffe0000000) (OK)
                                                                >	   to int32: 2147483647 (OK)
                                                                >	   to int64: 9223372034707292159 (OK)
                                                                >	  to uint32: 2147483647 (OK)
                                                                >	  to uint64: 9223372034707292159 (OK)
  from single: f32(-nan:0xffffffff)				from single: f32(-nan:0xffffffff)
    to double: f64(-nan:0x00ffffffffe0000000) (OK)		  to double: f64(-nan:0x00ffffffffe0000000) (OK)
     to int32: 2147483392 (INVALID)			      |	   to int32: 2147483647 (OK)
     to int64: 9223370939490631424 (INVALID)		      |	   to int64: 9223372034707292159 (OK)
    to uint32: 2147483392 (INVALID)			      |	  to uint32: 2147483647 (OK)
    to uint64: 9223370939490631424 (INVALID)		      |	  to uint64: 9223372034707292159 (OK)
  from single: f32(-nan:0xffffffff)			      |	from single: f32(nan:0x7fffffff)
    to double: f64(-nan:0x00ffffffffe0000000) (OK)	      |	  to double: f64(nan:0x007fffffffe0000000) (OK)
     to int32: 2147483392 (INVALID)			      |	   to int32: 2147483647 (OK)
     to int64: 9223370939490631424 (INVALID)		      |	   to int64: 9223372034707292159 (OK)
    to uint32: 2147483392 (INVALID)			      |	  to uint32: 2147483647 (OK)
    to uint64: 9223370939490631424 (INVALID)		      |	  to uint64: 9223372034707292159 (OK)
  from single: f32(-inf:0xff800000)			      <
    to double: f64(-inf:0x00fff0000000000000) (OK)	      <
     to int32: -2147483648 (INVALID)			      <
     to int64: 1 (INVALID)				      <
    to uint32: -2147483648 (INVALID)			      <
    to uint64: -9223372034707292160 (INVALID)		      <
  from single: f32(-0x1.fffffe00000000000000p+127:0xff7fffff)	from single: f32(-0x1.fffffe00000000000000p+127:0xff7fffff)
    to double: f64(-0x1.fffffe00000000000000p+127:0x00c7efffffe	  to double: f64(-0x1.fffffe00000000000000p+127:0x00c7efffffe
     to int32: -2147483648 (INVALID)			      |	   to int32: -2147483648 (OK)
     to int64: 1 (INVALID)				      |	   to int64: 1 (OK)
    to uint32: -2147483648 (INVALID)			      |	  to uint32: -2147483648 (OK)
    to uint64: -9223372034707292160 (INVALID)		      |	  to uint64: -9223372034707292160 (OK)

snipped a bunch more.

-- 
Alex Bennée

Re: [PATCH 1/2] softfloat: m68k: infinity is a valid encoding

[Laurent Vivier]

Le 28/04/2020 à 19:17, KONRAD Frederic a écrit :
> The MC68881 say about infinities (3.2.4):
> 
> "*For the extended precision format, the most significant bit of the
> mantissa (the integer bit) is a don't care."
> 
> https://www.nxp.com/docs/en/reference-manual/MC68881UM.pdf
> 
> The m68k extended format is implemented with the floatx80 and
> floatx80_invalid_encoding currently treats 0x7fff00000000000000000000 as
> an invalid encoding.  This patch fixes floatx80_invalid_encoding so it
> accepts that the most significant bit of the mantissa can be 0.
> 
> This bug can be revealed with the following code which pushes extended
> infinity on the stack as a double and then reloads it as a double.  It
> should normally be converted and read back as infinity and is currently
> read back as nan:
> 
>         .global _start
>         .text
> _start:
>         lea val, %a0
>         lea fp, %fp
>         fmovex (%a0), %fp0
>         fmoved %fp0, %fp@(-8)
>         fmoved %fp@(-8), %fp0
> end:
>         bra end
> 
> .align 0x4
> val:
>         .fill 1, 4, 0x7fff0000
>         .fill 1, 4, 0x00000000
>         .fill 1, 4, 0x00000000
> .align 0x4
>         .fill 0x100, 1, 0
> fp:
> 
> -------------
> 
> (gdb) tar rem :1234
> Remote debugging using :1234
> _start () at main.S:5
> 5              lea val, %a0
> (gdb) display $fp0
> 1: $fp0 = nan(0xffffffffffffffff)
> (gdb) si
> 6             lea fp, %fp
> 1: $fp0 = nan(0xffffffffffffffff)
> (gdb) si
> _start () at main.S:7
> 7              fmovex (%a0), %fp0
> 1: $fp0 = nan(0xffffffffffffffff)
> (gdb) si
> 8             fmoved %fp0, %fp@(-8)
> 1: $fp0 = inf
> (gdb) si
> 9             fmoved %fp@(-8), %fp0
> 1: $fp0 = inf
> (gdb) si
> end () at main.S:12
> 12          bra end
> 1: $fp0 = nan(0xfffffffffffff800)
> (gdb) x/1xg $fp-8
> 0x40000120 <val+260>:   0x7fffffffffffffff
> 
> Signed-off-by: KONRAD Frederic <frederic.konrad@adacore.com>
> ---
>  include/fpu/softfloat.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
> index ecb8ba0..dc80298 100644
> --- a/include/fpu/softfloat.h
> +++ b/include/fpu/softfloat.h
> @@ -688,7 +688,12 @@ static inline int floatx80_is_any_nan(floatx80 a)
>  *----------------------------------------------------------------------------*/
>  static inline bool floatx80_invalid_encoding(floatx80 a)
>  {
> +#if defined(TARGET_M68K)
> +    return (a.low & (1ULL << 63)) == 0 && (((a.high & 0x7FFF) != 0)
> +                                           && (a.high != 0x7FFF));
> +#else
>      return (a.low & (1ULL << 63)) == 0 && (a.high & 0x7FFF) != 0;
> +#endif
>  }
>  
>  #define floatx80_zero make_floatx80(0x0000, 0x0000000000000000LL)
> 

According to "M68000 FAMILY PROGRAMMER’S REFERENCE MANUAL" the explicit
integer bit is "Don't care" for signed infinite (a.high == 0x7FFF) (this
is the case this patch manages).

But wit a zero exponent and a non zero mantissa, it's a denormal number,
and a signed zero has also a zero explicit integer bit but a zero
mantissa. (both cases are already managed in the existing code).

with a non zero exponent less than the maximum value it's an unnormal
number.

The denormal and unnormal numbers must be managed during the load
operation in the m68k TCG emulation to generate directly the FP_UNIMP
exception.

So I think, in the end, we don't have invalid number at softfloat level
and floatx80_invalid_encoding() should always return "false" for
TARGET_M68K.

Thanks,
Laurent

Re: [PATCH 1/2] softfloat: m68k: infinity is a valid encoding

[Fred Konrad]

Missed this one sorry.

Le 6/12/20 à 10:31 AM, Laurent Vivier a écrit :
> Le 28/04/2020 à 19:17, KONRAD Frederic a écrit :
>> The MC68881 say about infinities (3.2.4):
>>
>> "*For the extended precision format, the most significant bit of the
>> mantissa (the integer bit) is a don't care."
>>
>> https://www.nxp.com/docs/en/reference-manual/MC68881UM.pdf
>>
>> The m68k extended format is implemented with the floatx80 and
>> floatx80_invalid_encoding currently treats 0x7fff00000000000000000000 as
>> an invalid encoding.  This patch fixes floatx80_invalid_encoding so it
>> accepts that the most significant bit of the mantissa can be 0.
>>
>> This bug can be revealed with the following code which pushes extended
>> infinity on the stack as a double and then reloads it as a double.  It
>> should normally be converted and read back as infinity and is currently
>> read back as nan:
>>
>>          .global _start
>>          .text
>> _start:
>>          lea val, %a0
>>          lea fp, %fp
>>          fmovex (%a0), %fp0
>>          fmoved %fp0, %fp@(-8)
>>          fmoved %fp@(-8), %fp0
>> end:
>>          bra end
>>
>> .align 0x4
>> val:
>>          .fill 1, 4, 0x7fff0000
>>          .fill 1, 4, 0x00000000
>>          .fill 1, 4, 0x00000000
>> .align 0x4
>>          .fill 0x100, 1, 0
>> fp:
>>

[...]

> 
> According to "M68000 FAMILY PROGRAMMER’S REFERENCE MANUAL" the explicit
> integer bit is "Don't care" for signed infinite (a.high == 0x7FFF) (this
> is the case this patch manages).
> 
> But wit a zero exponent and a non zero mantissa, it's a denormal number,
> and a signed zero has also a zero explicit integer bit but a zero
> mantissa. (both cases are already managed in the existing code).
> 
> with a non zero exponent less than the maximum value it's an unnormal
> number.
> 
> The denormal and unnormal numbers must be managed during the load
> operation in the m68k TCG emulation to generate directly the FP_UNIMP
> exception.

Is this already handled in the TCG code?

Thanks,
Fred

> 
> So I think, in the end, we don't have invalid number at softfloat level
> and floatx80_invalid_encoding() should always return "false" for
> TARGET_M68K.
> 
> Thanks,
> Laurent
> 

Re: [PATCH 1/2] softfloat: m68k: infinity is a valid encoding

[Laurent Vivier]

Le 15/06/2020 à 17:59, Fred Konrad a écrit :
> Missed this one sorry.
> 
> Le 6/12/20 à 10:31 AM, Laurent Vivier a écrit :
>> Le 28/04/2020 à 19:17, KONRAD Frederic a écrit :
>>> The MC68881 say about infinities (3.2.4):
>>>
>>> "*For the extended precision format, the most significant bit of the
>>> mantissa (the integer bit) is a don't care."
>>>
>>> https://www.nxp.com/docs/en/reference-manual/MC68881UM.pdf
>>>
>>> The m68k extended format is implemented with the floatx80 and
>>> floatx80_invalid_encoding currently treats 0x7fff00000000000000000000 as
>>> an invalid encoding.  This patch fixes floatx80_invalid_encoding so it
>>> accepts that the most significant bit of the mantissa can be 0.
>>>
>>> This bug can be revealed with the following code which pushes extended
>>> infinity on the stack as a double and then reloads it as a double.  It
>>> should normally be converted and read back as infinity and is currently
>>> read back as nan:
>>>
>>>          .global _start
>>>          .text
>>> _start:
>>>          lea val, %a0
>>>          lea fp, %fp
>>>          fmovex (%a0), %fp0
>>>          fmoved %fp0, %fp@(-8)
>>>          fmoved %fp@(-8), %fp0
>>> end:
>>>          bra end
>>>
>>> .align 0x4
>>> val:
>>>          .fill 1, 4, 0x7fff0000
>>>          .fill 1, 4, 0x00000000
>>>          .fill 1, 4, 0x00000000
>>> .align 0x4
>>>          .fill 0x100, 1, 0
>>> fp:
>>>
> 
> [...]
> 
>>
>> According to "M68000 FAMILY PROGRAMMER’S REFERENCE MANUAL" the explicit
>> integer bit is "Don't care" for signed infinite (a.high == 0x7FFF) (this
>> is the case this patch manages).
>>
>> But wit a zero exponent and a non zero mantissa, it's a denormal number,
>> and a signed zero has also a zero explicit integer bit but a zero
>> mantissa. (both cases are already managed in the existing code).
>>
>> with a non zero exponent less than the maximum value it's an unnormal
>> number.
>>
>> The denormal and unnormal numbers must be managed during the load
>> operation in the m68k TCG emulation to generate directly the FP_UNIMP
>> exception.
> 
> Is this already handled in the TCG code?

No, I have a skeleton with a workaround but if we enable the exception
the kernel crashes because the size of the frame saved in the stack by
fsave is not the one expected by the kernel (we save an IDLE frame and
not the UNIMP frame).

https://github.com/vivier/qemu-m68k/commit/c1297f61db283ccd592333f56907bd2961f1843c

I've also sent a patch similar to yours but disabling totally the
floatx80_invalid_encoding() check.

https://patchew.org/QEMU/20200612140400.2130118-1-laurent@vivier.eu/

Thanks,
Laurent

Re: [PATCH 1/2] softfloat: m68k: infinity is a valid encoding

[Andreas Schwab]

On Apr 29 2020, Laurent Vivier wrote:

> "Since the extended-precision data format has an explicit integer bit, a
> number can be formatted with a nonzero exponent, less than the maximum
> value, and a zero integer bit. The IEEE 754 standard does not define a
> zero integer bit. Such a number is an unnormalized number. Hardware does
> not directly support denormalized and unnormalized numbers, but
> implicitly supports them by trapping them as unimplemented data types,
> allowing efficient conversion in software."

This is supposed to be handled transparently by fpsp040, to be
compatible with the 68881.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."