mbox series

[for-9.2,v3,0/6] target/sparc: emulate floating point queue when raising fp traps

Message ID 20240816072311.353234-1-richard.henderson@linaro.org
Headers show
Series target/sparc: emulate floating point queue when raising fp traps | expand

Message

Richard Henderson Aug. 16, 2024, 7:23 a.m. UTC 
Hi Carl,

While digging through the manual to figure out if we were really
doing the right thing raising the fp exception, I found the fpu
exception state machine.  I'm not sure it's worth emulating the
fp_exception_pending state, but it's certainly easy enough to
emulate the fp_executing/fp_exception states.

In addition, this simplifies the implementation of STDFQ,
restricts FQ to sparc32 system mode, and handles migration.

Would you please double-check this against your Solaris images?


r~


Carl Hauser (2):
  target/sparc: Add FQ and FSR.QNE
  target/sparc: Populate sparc32 FQ when raising fp exception

Richard Henderson (4):
  target/sparc: Restrict STQF to sparcv9
  target/sparc: Add FSR_QNE to tb_flags
  target/sparc: Implement STDFQ
  target/sparc: Add gen_trap_if_nofpu_fpexception

 target/sparc/cpu.h          |  30 ++++++++-
 target/sparc/fop_helper.c   |   4 ++
 target/sparc/int32_helper.c |  32 ++++-----
 target/sparc/machine.c      |  25 +++++++
 target/sparc/translate.c    | 126 ++++++++++++++++++++++++++----------
 target/sparc/insns.decode   |   4 +-
 6 files changed, 169 insertions(+), 52 deletions(-)

Comments

Richard Henderson Aug. 16, 2024, 10:05 p.m. UTC | #1 
On 8/17/24 07:46, Carl Hauser wrote:
> OK, I think the problem is the handling of dc->fsr_qne in trans_STDFQ, lines 4583 and 4593 
> -- the code is evaluating dc->fsr_qne at translation time and not at runtime.

That's what patch 4 does, ensure that the runtime value is available at translation time.


r~
Richard Henderson Aug. 16, 2024, 11:59 p.m. UTC | #2 
On 8/17/24 08:58, Carl Hauser wrote:
> Yes, but ...
> 
> isn't the state of dc->fsr_qne at translation time irrelevant?

No, because patch 4 made it part of the hashed TB state.
It's checked and verified, generating a new TB if state does not match.

> And changing it at 
> translation time (line 4593) is dangerous (because it pertains to runtime, not translation 
> time); i.e. why is 0 stored at both translation time (4593) and at runtime (4591)?

That keeps the translation time state in sync with the runtime state until the end of the TB.


r~
Richard Henderson Aug. 17, 2024, 12:16 a.m. UTC | #3 
On 8/17/24 09:48, Carl Hauser wrote:
> netbsd panics in the kernel trap handler; unfortunately it does not include the fsr in the 
> panic message, but I expect it will be the same as on Solaris.

Ok, I have reproduced this with netbsd 9.3.
I'll have a look.


r~
Richard Henderson Aug. 19, 2024, 2:42 a.m. UTC | #4 
On 8/18/24 10:03, Carl Hauser wrote:
> I changed translate.c:4597 from return true; to return advance_pc(dc); and it worked.

Whoops, yes indeed.  Thanks for the catch.


r~
Richard Henderson Aug. 20, 2024, 10:16 p.m. UTC | #5 
On 8/21/24 02:59, Carl Hauser wrote:
> Do you want me to submit a patch set fixing this or will you?

I will.


r~