Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/45521 )

Change subject: arch-riscv: Decode more of the CSR instructions at decode time.
......................................................................

arch-riscv: Decode more of the CSR instructions at decode time.

Figure out more about what the CSR instructions are supposed to do at
decode/instruction construction time, instead of at run time. An
instruction will usually be constructed many fewer times than it will be
executed, so we can perform the work once and then use it many times.

Change-Id: I9941bb2555e67a6c738aa3dfdca1b4857427b71c
---
M src/arch/riscv/insts/standard.hh
M src/arch/riscv/isa/formats/standard.isa
2 files changed, 45 insertions(+), 35 deletions(-)



diff --git a/src/arch/riscv/insts/standard.hh b/src/arch/riscv/insts/standard.hh
index 9f9cba4..1e9fcb7 100644
--- a/src/arch/riscv/insts/standard.hh
+++ b/src/arch/riscv/insts/standard.hh
@@ -89,11 +89,30 @@
     uint64_t csr;
     uint64_t uimm;

+    bool valid = false;
+    RegIndex midx = 0;
+    std::string csrName;
+    uint64_t maskVal = 0;
+
     /// Constructor
     CSROp(const char *mnem, MachInst _machInst, OpClass __opClass)
         : RiscvStaticInst(mnem, _machInst, __opClass),
             csr(FUNCT12), uimm(CSRIMM)
     {
+        auto csr_data_it = CSRData.find(csr);
+        if (csr_data_it == CSRData.end()) {
+            valid = false;
+        } else {
+            valid = true;
+            midx = csr_data_it->second.physIndex;
+            csrName = csr_data_it->second.name;
+            auto mask_it = CSRMasks.find(csr);
+            if (mask_it == CSRMasks.end())
+                maskVal = mask(64);
+            else
+                maskVal = mask_it->second;
+        }
+
         if (csr == CSR_SATP) {
             flags[IsSquashAfter] = true;
         }
diff --git a/src/arch/riscv/isa/formats/standard.isa b/src/arch/riscv/isa/formats/standard.isa
index c31682d..e0b270b 100644
--- a/src/arch/riscv/isa/formats/standard.isa
+++ b/src/arch/riscv/isa/formats/standard.isa
@@ -279,15 +279,17 @@
     %(class_name)s::execute(ExecContext *xc,
         Trace::InstRecord *traceData) const
     {
+        if (!valid) {
+            return std::make_shared<IllegalInstFault>(
+                    csprintf("Illegal CSR index %#x\n", csr), machInst);
+        }
+
         %(op_decl)s;
         %(op_rd)s;

         RegVal data, olddata;
+
         switch (csr) {
-          case CSR_FCSR:
-            olddata = xc->readMiscReg(MISCREG_FFLAGS) |
-                      (xc->readMiscReg(MISCREG_FRM) << FRM_OFFSET);
-            break;
           case CSR_SATP: {
             auto pm = (PrivilegeMode)xc->readMiscReg(MISCREG_PRV);
             STATUS status = xc->readMiscReg(MISCREG_STATUS);
@@ -295,8 +297,6 @@
                 return std::make_shared<IllegalInstFault>(
                         "SATP access in user mode or with TVM enabled\n",
                         machInst);
-            } else {
-                olddata = xc->readMiscReg(CSRData.at(csr).physIndex);
             }
             break;
           }
@@ -306,48 +306,40 @@
                 return std::make_shared<IllegalInstFault>(
                         "MSTATUS is only accessibly in machine mode\n",
                         machInst);
-            } else {
-                olddata = xc->readMiscReg(CSRData.at(csr).physIndex);
             }
             break;
           }
           default:
-            if (CSRData.find(csr) != CSRData.end()) {
-                olddata = xc->readMiscReg(CSRData.at(csr).physIndex);
-            } else {
-                return std::make_shared<IllegalInstFault>(
- csprintf("Illegal CSR index %#x\n", csr), machInst);
-            }
             break;
         }
-        auto mask = CSRMasks.find(csr);
+
+        if (csr == CSR_FCSR) {
+            olddata = xc->readMiscReg(MISCREG_FFLAGS) |
+                      (xc->readMiscReg(MISCREG_FRM) << FRM_OFFSET);
+        } else {
+            olddata = xc->readMiscReg(midx);
+        }
         auto olddata_all = olddata;
-        if (mask != CSRMasks.end())
-            olddata &= mask->second;
-        DPRINTF(RiscvMisc, "Reading CSR %s: %#x\n", CSRData.at(csr).name,
-                olddata);
+
+        olddata &= maskVal;
+        DPRINTF(RiscvMisc, "Reading CSR %s: %#x\n", csrName, olddata);
         data = olddata;

         %(code)s;
-        if (mask != CSRMasks.end())
-            data &= mask->second;
+
+        data &= maskVal;
         if (data != olddata) {
             if (bits(csr, 11, 10) == 0x3) {
                 return std::make_shared<IllegalInstFault>(
-                        csprintf("CSR %s is read-only\n",
-                            CSRData.at(csr).name), machInst);
+ csprintf("CSR %s is read-only\n", csrName), machInst);
             }
             auto newdata_all = data;
-            if (mask != CSRMasks.end()) {
-                // we must keep those original bits not in mask
-                // olddata and data only contain thebits visable
-                // in current privilige level
-                newdata_all = (olddata_all & (~mask->second))
-                              | data;
-            }
+            // We must keep those original bits not in mask.
+            // olddata and data only contain the bits visable
+            // in current privilige level.
+            newdata_all = (olddata_all & ~maskVal) | data;
             DPRINTF(RiscvMisc, "Writing %#x to CSR %s.\n",
-                    newdata_all,
-                    CSRData.at(csr).name);
+                    newdata_all, csrName);
             switch (csr) {
               case CSR_FCSR:
                 xc->setMiscReg(MISCREG_FFLAGS, bits(data, 4, 0));
@@ -358,8 +350,7 @@
               case CSR_UIP: case CSR_UIE:
               case CSR_MSTATUS: case CSR_SSTATUS: case CSR_USTATUS:
                 if (newdata_all != olddata_all) {
-                    xc->setMiscReg(CSRData.at(csr).physIndex,
-                                   newdata_all);
+                    xc->setMiscReg(midx, newdata_all);
                 } else {
                     return std::make_shared<IllegalInstFault>(
                             "Only bits in mask are allowed to be set\n",
@@ -367,7 +358,7 @@
                 }
                 break;
               default:
-                xc->setMiscReg(CSRData.at(csr).physIndex, data);
+                xc->setMiscReg(midx, data);
                 break;
             }
         }

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/45521
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I9941bb2555e67a6c738aa3dfdca1b4857427b71c
Gerrit-Change-Number: 45521
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <gabe.bl...@gmail.com>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to