[PATCH 00/21] softfloat: Set 2-NaN propagation rule in float_status, not at compile time

Peter Maydell posted 21 patches 4 weeks, 1 day ago
include/fpu/softfloat-helpers.h   |  11 +++
include/fpu/softfloat-types.h     |  38 ++++++++
target/i386/cpu.h                 |   3 +
target/mips/fpu_helper.h          |  22 +++++
target/xtensa/cpu.h               |   6 ++
linux-user/arm/nwfpe/fpa11.c      |  18 ++++
target/alpha/cpu.c                |  11 +++
target/arm/cpu.c                  |  25 +++--
target/hppa/fpu_helper.c          |   6 ++
target/i386/cpu.c                 |   4 +
target/i386/tcg/fpu_helper.c      |  40 ++++++++
target/loongarch/tcg/fpu_helper.c |   1 +
target/m68k/cpu.c                 |  16 +++
target/m68k/fpu_helper.c          |   1 +
target/m68k/helper.c              |   4 +-
target/microblaze/cpu.c           |  10 +-
target/mips/cpu.c                 |   2 +-
target/mips/msa.c                 |  17 ++++
target/openrisc/cpu.c             |   6 ++
target/ppc/cpu_init.c             |   8 ++
target/rx/cpu.c                   |   7 ++
target/s390x/cpu.c                |   1 +
target/sparc/cpu.c                |  10 +-
target/sparc/fop_helper.c         |  10 +-
target/xtensa/cpu.c               |   2 +-
target/xtensa/fpu_helper.c        |  35 ++++---
tests/fp/fp-bench.c               |   2 +
tests/fp/fp-test-log2.c           |   1 +
tests/fp/fp-test.c                |   2 +
fpu/softfloat-specialize.c.inc    | 156 +++++++++++-------------------
30 files changed, 346 insertions(+), 129 deletions(-)
[PATCH 00/21] softfloat: Set 2-NaN propagation rule in float_status, not at compile time
Posted by Peter Maydell 4 weeks, 1 day ago
IEEE 758 does not define a fixed rule for which NaN to pick as the
result if both operands of a 2-operand operation are NaNs.  As a
result different architectures have ended up with different rules for
propagating NaNs.

QEMU currently hardcodes the NaN propagation logic into the binary,
because pickNaN() has an ifdef ladder for different targets.  We want
to make the propagation rule instead be selectable at runtime,
because:
 * this will let us have multiple targets in one QEMU binary
 * the Arm FEAT_AFP architectural feature includes letting the guest
   select a NaN propagation rule at runtime
 * x86 specifies different propagation rules for x87 FPU ops and for
   SSE ops, and specifying the rule in the float_status would let us
   emulate this, instead of wrongly using the x87 rules everywhere
 * xtensa allows selection of NaN propagation rule at runtime;
   currently we implement this with the use_first_nan flag which
   is only used by xtensa; providing a general facility will let us
   remove this limited-purpose handling
                                                           
This series switches pickNaN away from the ifdef ladder. Instead
targets must always set a 2-NaN propagation rule via the new
set_float_2nan_prop_rule(), unless they are using default_nan_mode.
Patch 1 implements this in core softfloat (with a transitional
fallback to "use the old ifdef ladder logic" if the target doesn't
specify the propagation rule). Then subsequent patches convert
all the targets to set the rule explicitly. The final patch then
removes the remnants of the transitional logic.

I have included a couple of minor fixes for sparc, xtensa, m68k.
This is intended as a no-behaviour-change patchset, so although there
are a few places where I have noticed that the current behaviour
is not correct I have left these as TODO comments. A summary of
those TODOs and other oddities I have noticed but not tried to
tackle is:

 * hppa really ought to implement a CPU reset method
 * alpha also doesn't implement CPU reset
 * ppc sets tininess_before_rounding on fp_status but not on
   vec_status, so its behaviour there will vary between
   guest instructions; unclear to me if this is intended
 * x86 should set the float_2nan_prop_ab rule on env->sse_status
   (and maybe env->mmx_status, though 3DNow! insns don't handle
   NaNs in a documented way anyway)
 * alpha never had a case in the ifdef ladder so it uses the
   x87 propagation rule. It ought to use float_2nan_prop_ba.
 * openrisc didn't have an ifdef and also uses the x87 rule;
   this is probably wrong but I couldn't find anything documenting
   what it actually does
 * rx didn't have an ifdef and also uses the x87 rule;
   again, probably wrong

The next stage after this patchset, obviously, is to do the
same thing with pickNaNMulAdd(). That is a little trickier
because currently we ask it to do two things:
 * handle the "infinity * zero + NaN" corner case
 * pick a NaN when more than one operand is a NaN
My intention is to separate these out so that targets specify
both separately. This gives more orthogonality (e.g. Arm,
MIPS with !snan_bit_is_one and loongarch64 all have the same
"prefer SNaN over QNaN, prefer C over A over B" NaN selection
logic but they have different ideas about the infzero case).

(Once pickNaNMulAdd() is converted we will be able to remove
the current xtensa-specific use_first_nan flag.)

thanks
-- PMM

Peter Maydell (21):
  softfloat: Allow 2-operand NaN propagation rule to be set at runtime
  tests/fp: Explicitly set 2-NaN propagation rule
  target/arm: Explicitly set 2-NaN propagation rule
  target/mips: Explicitly set 2-NaN propagation rule
  target/loongarch: Explicitly set 2-NaN propagation rule
  target/hppa: Explicitly set 2-NaN propagation rule
  target/s390x: Explicitly set 2-NaN propagation rule
  target/ppc: Explicitly set 2-NaN propagation rule
  target/m68k: Explicitly set 2-NaN propagation rule
  target/m68k: Initialize float_status fields in gdb set/get functions
  target/sparc: Move cpu_put_fsr(env, 0) call to reset
  target/sparc: Explicitly set 2-NaN propagation rule
  target/xtensa: Factor out calls to set_use_first_nan()
  target/xtensa: Explicitly set 2-NaN propagation rule
  target/i386: Set 2-NaN propagation rule explicitly
  target/alpha: Explicitly set 2-NaN propagation rule
  target/microblaze: Move setting of float rounding mode to reset
  target/microblaze: Explicitly set 2-NaN propagation rule
  target/openrisc: Explicitly set 2-NaN propagation rule
  target/rx: Explicitly set 2-NaN propagation rule
  softfloat: Remove fallback rule from pickNaN()

 include/fpu/softfloat-helpers.h   |  11 +++
 include/fpu/softfloat-types.h     |  38 ++++++++
 target/i386/cpu.h                 |   3 +
 target/mips/fpu_helper.h          |  22 +++++
 target/xtensa/cpu.h               |   6 ++
 linux-user/arm/nwfpe/fpa11.c      |  18 ++++
 target/alpha/cpu.c                |  11 +++
 target/arm/cpu.c                  |  25 +++--
 target/hppa/fpu_helper.c          |   6 ++
 target/i386/cpu.c                 |   4 +
 target/i386/tcg/fpu_helper.c      |  40 ++++++++
 target/loongarch/tcg/fpu_helper.c |   1 +
 target/m68k/cpu.c                 |  16 +++
 target/m68k/fpu_helper.c          |   1 +
 target/m68k/helper.c              |   4 +-
 target/microblaze/cpu.c           |  10 +-
 target/mips/cpu.c                 |   2 +-
 target/mips/msa.c                 |  17 ++++
 target/openrisc/cpu.c             |   6 ++
 target/ppc/cpu_init.c             |   8 ++
 target/rx/cpu.c                   |   7 ++
 target/s390x/cpu.c                |   1 +
 target/sparc/cpu.c                |  10 +-
 target/sparc/fop_helper.c         |  10 +-
 target/xtensa/cpu.c               |   2 +-
 target/xtensa/fpu_helper.c        |  35 ++++---
 tests/fp/fp-bench.c               |   2 +
 tests/fp/fp-test-log2.c           |   1 +
 tests/fp/fp-test.c                |   2 +
 fpu/softfloat-specialize.c.inc    | 156 +++++++++++-------------------
 30 files changed, 346 insertions(+), 129 deletions(-)

-- 
2.34.1
Re: [PATCH 00/21] softfloat: Set 2-NaN propagation rule in float_status, not at compile time
Posted by Helge Deller 4 weeks, 1 day ago
On 10/25/24 16:12, Peter Maydell wrote:
> A summary of those TODOs and other oddities I have noticed but not
> tried to tackle is:>
>   * hppa really ought to implement a CPU reset method
>   * alpha also doesn't implement CPU reset

I used the alpha code as template to implement the hppa machine.
That's probably why the CPU reset method is missing for both :-)

The TODO about implementing: ?
	resettable_class_set_parent_phases(rc, NULL, hppa_cpu_reset_hold, NULL,...)...

Helge
Re: [PATCH 00/21] softfloat: Set 2-NaN propagation rule in float_status, not at compile time
Posted by Peter Maydell 4 weeks, 1 day ago
On Fri, 25 Oct 2024 at 15:49, Helge Deller <deller@gmx.de> wrote:
>
>
> On 10/25/24 16:12, Peter Maydell wrote:
> > A summary of those TODOs and other oddities I have noticed but not
> > tried to tackle is:>
> >   * hppa really ought to implement a CPU reset method
> >   * alpha also doesn't implement CPU reset
>
> I used the alpha code as template to implement the hppa machine.
> That's probably why the CPU reset method is missing for both :-)
>
> The TODO about implementing: ?
>         resettable_class_set_parent_phases(rc, NULL, hppa_cpu_reset_hold, NULL,...)...

Yes, basically. The reset method should restore all the
state of the CPU to what it was when QEMU started.
Typically you put an end_reset_fields marker in the
CPU state struct so you can memset() everything up to
that point to zero, and then manually reset anything
else that needs special handling.

thanks
-- PMM
Re: [PATCH 00/21] softfloat: Set 2-NaN propagation rule in float_status, not at compile time
Posted by Helge Deller 4 weeks ago
On 10/25/24 16:59, Peter Maydell wrote:
> On Fri, 25 Oct 2024 at 15:49, Helge Deller <deller@gmx.de> wrote:
>>
>>
>> On 10/25/24 16:12, Peter Maydell wrote:
>>> A summary of those TODOs and other oddities I have noticed but not
>>> tried to tackle is:>
>>>    * hppa really ought to implement a CPU reset method
>>>    * alpha also doesn't implement CPU reset
>>
>> I used the alpha code as template to implement the hppa machine.
>> That's probably why the CPU reset method is missing for both :-)
>>
>> The TODO about implementing: ?
>>          resettable_class_set_parent_phases(rc, NULL, hppa_cpu_reset_hold, NULL,...)...
>
> Yes, basically. The reset method should restore all the
> state of the CPU to what it was when QEMU started.
> Typically you put an end_reset_fields marker in the
> CPU state struct so you can memset() everything up to
> that point to zero, and then manually reset anything
> else that needs special handling.

Thanks for the explanation!

I've just sent a patch to the mailing list:
[PATCH] target/hppa: Add CPU reset method

I tested it and it does work for me.
If OK for you, I'd happy if you could include it in your patch
series and adjust as needed to fix the FP initialization.

Helge