[PATCH 0/2] target/i386: Fix 0 * Inf + QNaN regression

Peter Maydell posted 2 patches 2 weeks, 5 days ago
include/fpu/softfloat-types.h    |  16 ++++-
target/i386/tcg/fpu_helper.c     |   5 +-
tests/tcg/x86_64/fma.c           | 109 +++++++++++++++++++++++++++++++
fpu/softfloat-parts.c.inc        |   5 +-
tests/tcg/x86_64/Makefile.target |   1 +
5 files changed, 130 insertions(+), 6 deletions(-)
create mode 100644 tests/tcg/x86_64/fma.c
[PATCH 0/2] target/i386: Fix 0 * Inf + QNaN regression
Posted by Peter Maydell 2 weeks, 5 days ago
This patchset fixes a regression that I introduced in my recent
refactoring of softfloat NaN handling, in commit 8adcff4ae7
("fpu: handle raising Invalid for infzero in pick_nan_muladd").
When I wrote that code I was under the impression that all
architectures would raise Invalid for the "inf * zero + NaN"
case of a fused multiply-add. However, IEEE 754-2008 makes this
impdef for QNaN: an architecture can choose whether to raise
Invalid or not.

For i386, SDM section 14.5.2 documents that for the 0 * Inf + NaN
case that it will only raise the Invalid exception when the input is
a signalling NaN, and so the behaviour change in 8adcff4ae7 that
caused it to raise Invalid also for the QNaN case is wrong.

The first commit here adds a knob to the softfloat code to
allow an architecture to disable the "raise Invalid" that is
the default, and makes x86 set that. The second commit is a
test case for x86 check-tcg that exercises this corner case.

We do not revert here the behaviour change for hppa, sh4 or tricore:
 * The sh4 manual is clear that it should signal Invalid
 * The tricore manual is a bit vague but doesn't say it shouldn't
 * The hppa manual doesn't talk about fused multiply-add corner
   cases at all

The test case also includes a disabled test for a different
x86 fma corner case; this is one that's not a regression. I've
left it in the test case code because it's the justification
for why the test harness has the support for testing fma insns
with FTZ set. I'm working on a fix for that but I don't think
it should be tangled up with fixing this regression.

thanks
-- PMM

Peter Maydell (2):
  target/i386: Do not raise Invalid for 0 * Inf + QNaN
  tests/tcg/x86_64/fma: Test some x86 fused-multiply-add cases

 include/fpu/softfloat-types.h    |  16 ++++-
 target/i386/tcg/fpu_helper.c     |   5 +-
 tests/tcg/x86_64/fma.c           | 109 +++++++++++++++++++++++++++++++
 fpu/softfloat-parts.c.inc        |   5 +-
 tests/tcg/x86_64/Makefile.target |   1 +
 5 files changed, 130 insertions(+), 6 deletions(-)
 create mode 100644 tests/tcg/x86_64/fma.c

-- 
2.34.1
Re: [PATCH 0/2] target/i386: Fix 0 * Inf + QNaN regression
Posted by Paolo Bonzini 1 week, 4 days ago
Queued, thanks.

Paolo
Re: [PATCH 0/2] target/i386: Fix 0 * Inf + QNaN regression
Posted by Peter Maydell 1 day, 22 hours ago
On Fri, 24 Jan 2025 at 17:22, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Queued, thanks.

Thanks; do you plan to send a pullreq with these in soon?
I ask because the Arm FEAT_AFP set is now ready to land
and it has a dependency on these.

thanks
-- PMM
Re: [PATCH 0/2] target/i386: Fix 0 * Inf + QNaN regression
Posted by Paolo Bonzini 1 day, 21 hours ago
On 2/3/25 12:05, Peter Maydell wrote:
> On Fri, 24 Jan 2025 at 17:22, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> Queued, thanks.
> 
> Thanks; do you plan to send a pullreq with these in soon?
> I ask because the Arm FEAT_AFP set is now ready to land
> and it has a dependency on these.
I do but if you want to send it yourself, feel free to include them.

Paolo