From 816ef46c78a9a4b345eab77ff52710f9036de0a1 Mon Sep 17 00:00:00 2001 From: Yu-Cheng Chang Date: Fri, 23 Feb 2024 00:33:34 +0800 Subject: [PATCH] arch-riscv: Fix fflags behavior of float inst. in O3 CPU (#868) According to the RISC-V spec [1]. Any float-point instructions accumulate FFLAGS register rather than write it to reflect the CSR behavior. In the previous implementation. We read the FFLAGS, set the exception flags, and write the result back to the FFLAGS. This works in the gem5 simple and minor CPU model as they are actually written to `regFile` after executing the instructions. However, in the gem5 O3 CPU model, it will record in the `destMiscReg` buffer until the commit stage when writing to the `miscReg` in the execution stage. The next instruction will get the old FFLAGS and cause the incorrect result. The CL introduced the `MISCREG_FFLAGS_EXE` and used the same size of `miscRegFile` because the `MISCREG_FFLAGS_EXE` and `MISCREG_FFLAGS` shared the same space. When executing the float-pointing instruction, any exception flags should be updated via `MISCREG_FFLAGS_EXE` to accumulate the FFLAGS in `setMiscReg` method. For the MISCREG_FFLAGS, it should only be called in the CSROp. [1] Syntactic Dependencies: Appendix A https://github.com/riscv/riscv-isa-manual/blob/c80ecada1ccc3120e2c8003bd7ca8e91c2e91bf9/src/mm-eplan.adoc#syntactic-dependencies-rules-9-11 gem5 issue: https://github.com/gem5/gem5/issues/755 Change-Id: Ib7f13d95b8a921c37766a54a217a5a4b1ef17c6f --- src/arch/riscv/isa.cc | 21 +++++++++++++++++---- src/arch/riscv/isa/formats/fp.isa | 5 +---- src/arch/riscv/isa/formats/vector_arith.isa | 4 +--- src/arch/riscv/regs/misc.hh | 5 +++++ 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/src/arch/riscv/isa.cc b/src/arch/riscv/isa.cc index 18e9b5fce2..da5e5fc8f1 100644 --- a/src/arch/riscv/isa.cc +++ b/src/arch/riscv/isa.cc @@ -240,6 +240,8 @@ namespace RiscvISA [MISCREG_HPMCOUNTER29H] = "HPMCOUNTER29H", [MISCREG_HPMCOUNTER30H] = "HPMCOUNTER30H", [MISCREG_HPMCOUNTER31H] = "HPMCOUNTER31H", + + [MISCREG_FFLAGS_EXE] = "FFLAGS_EXE", }}; namespace @@ -276,7 +278,7 @@ ISA::ISA(const Params &p) : BaseISA(p), p.vlen, p.elen); - miscRegFile.resize(NUM_MISCREGS); + miscRegFile.resize(NUM_PHYS_MISCREGS); clear(); } @@ -304,7 +306,7 @@ ISA::copyRegsFrom(ThreadContext *src) } // Copying Misc Regs - for (int i = 0; i < NUM_MISCREGS; i++) + for (int i = 0; i < NUM_PHYS_MISCREGS; i++) tc->setMiscRegNoEffect(i, src->readMiscRegNoEffect(i)); // Lastly copy PC/NPC @@ -409,7 +411,7 @@ RegVal ISA::readMiscRegNoEffect(RegIndex idx) const { // Illegal CSR - panic_if(idx > NUM_MISCREGS, "Illegal CSR index %#x\n", idx); + panic_if(idx > NUM_PHYS_MISCREGS, "Illegal CSR index %#x\n", idx); DPRINTF(RiscvMisc, "Reading MiscReg %s (%d): %#x.\n", MiscRegNames[idx], idx, miscRegFile[idx]); return miscRegFile[idx]; @@ -571,6 +573,10 @@ ISA::readMiscReg(RegIndex idx) (readMiscRegNoEffect(MISCREG_VXRM) << 1); } break; + case MISCREG_FFLAGS_EXE: + { + return readMiscRegNoEffect(MISCREG_FFLAGS) & FFLAGS_MASK; + } default: // Try reading HPM counters // As a placeholder, all HPM counters are just cycle counters @@ -603,7 +609,7 @@ void ISA::setMiscRegNoEffect(RegIndex idx, RegVal val) { // Illegal CSR - panic_if(idx > NUM_MISCREGS, "Illegal CSR index %#x\n", idx); + panic_if(idx > NUM_PHYS_MISCREGS, "Illegal CSR index %#x\n", idx); DPRINTF(RiscvMisc, "Setting MiscReg %s (%d) to %#x.\n", MiscRegNames[idx], idx, val); miscRegFile[idx] = val; @@ -770,6 +776,13 @@ ISA::setMiscReg(RegIndex idx, RegVal val) setMiscRegNoEffect(MISCREG_VXRM, (val & 0x6) >> 1); } break; + case MISCREG_FFLAGS_EXE: + { + RegVal new_val = readMiscRegNoEffect(MISCREG_FFLAGS); + new_val |= (val & FFLAGS_MASK); + setMiscRegNoEffect(MISCREG_FFLAGS, new_val); + } + break; default: setMiscRegNoEffect(idx, val); } diff --git a/src/arch/riscv/isa/formats/fp.isa b/src/arch/riscv/isa/formats/fp.isa index d0bd245ae4..d459a59a50 100644 --- a/src/arch/riscv/isa/formats/fp.isa +++ b/src/arch/riscv/isa/formats/fp.isa @@ -46,13 +46,10 @@ def template FloatExecute {{ %(op_decl)s; %(op_rd)s; - RegVal FFLAGS = xc->readMiscReg(MISCREG_FFLAGS); std::feclearexcept(FE_ALL_EXCEPT); %(code)s; - - FFLAGS |= softfloat_exceptionFlags; + xc->setMiscReg(MISCREG_FFLAGS_EXE, softfloat_exceptionFlags); softfloat_exceptionFlags = 0; - xc->setMiscReg(MISCREG_FFLAGS, FFLAGS); %(op_wb)s; diff --git a/src/arch/riscv/isa/formats/vector_arith.isa b/src/arch/riscv/isa/formats/vector_arith.isa index 7f87f1e163..0a4acd3db9 100644 --- a/src/arch/riscv/isa/formats/vector_arith.isa +++ b/src/arch/riscv/isa/formats/vector_arith.isa @@ -114,12 +114,10 @@ let {{ def fflags_wrapper(code): return ''' - RegVal FFLAGS = xc->readMiscReg(MISCREG_FFLAGS); std::feclearexcept(FE_ALL_EXCEPT); ''' + code + ''' - FFLAGS |= softfloat_exceptionFlags; + xc->setMiscReg(MISCREG_FFLAGS_EXE, softfloat_exceptionFlags); softfloat_exceptionFlags = 0; - xc->setMiscReg(MISCREG_FFLAGS, FFLAGS); ''' def declareVArithTemplate( diff --git a/src/arch/riscv/regs/misc.hh b/src/arch/riscv/regs/misc.hh index 837cbfacbd..cc4fe6fe55 100644 --- a/src/arch/riscv/regs/misc.hh +++ b/src/arch/riscv/regs/misc.hh @@ -246,6 +246,11 @@ enum MiscRegIndex MISCREG_HPMCOUNTER30H, MISCREG_HPMCOUNTER31H, + NUM_PHYS_MISCREGS, + + // This CSR shared the same space with MISCREG_FFLAGS + MISCREG_FFLAGS_EXE = NUM_PHYS_MISCREGS, + NUM_MISCREGS };