Merge pull request #950 from riscv-software-src/fix-perf-regression

Fix perf regressions from CSR refactoring in FP-heavy code

riscv/csrs.cc

@@ -316,12 +316,6 @@ base_status_csr_t::base_status_csr_t(processor_t* const proc, const reg_t addr):
 }
 
 
-bool base_status_csr_t::enabled(const reg_t which) {
-  // If the field doesn't exist, it is always enabled. See #823.
-  if ((sstatus_write_mask & which) == 0) return true;
-  return (read() & which) != 0;
-}
-
 reg_t base_status_csr_t::compute_sstatus_write_mask() const noexcept {
   // If a configuration has FS bits, they will always be accessible no
   // matter the state of misa.
@@ -380,10 +374,6 @@ vsstatus_csr_t::vsstatus_csr_t(processor_t* const proc, const reg_t addr):
   val(proc->get_state()->mstatus->read() & sstatus_read_mask) {
 }
 
-reg_t vsstatus_csr_t::read() const noexcept {
-  return val;
-}
-
 bool vsstatus_csr_t::unlogged_write(const reg_t val) noexcept {
   const reg_t newval = (this->val & ~sstatus_write_mask) | (val & sstatus_write_mask);
   if (state->v) maybe_flush_tlb(newval);
@@ -393,15 +383,11 @@ bool vsstatus_csr_t::unlogged_write(const reg_t val) noexcept {
 
 
 // implement class sstatus_proxy_csr_t
-sstatus_proxy_csr_t::sstatus_proxy_csr_t(processor_t* const proc, const reg_t addr, csr_t_p mstatus):
+sstatus_proxy_csr_t::sstatus_proxy_csr_t(processor_t* const proc, const reg_t addr, mstatus_csr_t_p mstatus):
   base_status_csr_t(proc, addr),
   mstatus(mstatus) {
 }
 
-reg_t sstatus_proxy_csr_t::read() const noexcept {
-  return mstatus->read() & sstatus_read_mask;
-}
-
 bool sstatus_proxy_csr_t::unlogged_write(const reg_t val) noexcept {
   const reg_t new_mstatus = (mstatus->read() & ~sstatus_write_mask) | (val & sstatus_write_mask);
 
@@ -425,11 +411,6 @@ mstatus_csr_t::mstatus_csr_t(processor_t* const proc, const reg_t addr):
 }
 
 
-reg_t mstatus_csr_t::read() const noexcept {
-  return val;
-}
-
-
 bool mstatus_csr_t::unlogged_write(const reg_t val) noexcept {
   const bool has_mpv = proc->extension_enabled('S') && proc->extension_enabled('H');
   const bool has_gva = has_mpv;
@@ -466,29 +447,40 @@ bool mstatush_csr_t::unlogged_write(const reg_t val) noexcept {
 }
 
 // implement class sstatus_csr_t
-sstatus_csr_t::sstatus_csr_t(processor_t* const proc, base_status_csr_t_p orig, base_status_csr_t_p virt):
+sstatus_csr_t::sstatus_csr_t(processor_t* const proc, sstatus_proxy_csr_t_p orig, vsstatus_csr_t_p virt):
   virtualized_csr_t(proc, orig, virt),
   orig_sstatus(orig),
   virt_sstatus(virt) {
 }
 
 void sstatus_csr_t::dirty(const reg_t dirties) {
+  // As an optimization, return early if already dirty.
+  if ((orig_sstatus->read() & dirties) == dirties) {
+    if (likely(!state->v || (virt_sstatus->read() & dirties) == dirties))
+      return;
+  }
+
   // Catch problems like #823 where P-extension instructions were not
   // checking for mstatus.VS!=Off:
   if (!enabled(dirties)) abort();
 
-  orig_csr->write(orig_csr->read() | dirties);
+  orig_sstatus->write(orig_sstatus->read() | dirties);
   if (state->v) {
-    virt_csr->write(virt_csr->read() | dirties);
+    virt_sstatus->write(virt_sstatus->read() | dirties);
   }
 }
 
 bool sstatus_csr_t::enabled(const reg_t which) {
-  if (!orig_sstatus->enabled(which))
-    return false;
-  if (state->v && !virt_sstatus->enabled(which))
-    return false;
+  if ((orig_sstatus->read() & which) != 0) {
+    if (!state->v || (virt_sstatus->read() & which) != 0)
       return true;
+  }
+
+  // If the field doesn't exist, it is always enabled. See #823.
+  if (!orig_sstatus->field_exists(which))
+    return true;
+
+  return false;
 }
 
 

riscv/csrs.h

@@ -185,8 +185,11 @@ class cause_csr_t: public basic_csr_t {
 class base_status_csr_t: public csr_t {
  public:
   base_status_csr_t(processor_t* const proc, const reg_t addr);
-  // Return true if the specified bits are not 00 (Off)
-  bool enabled(const reg_t which);
+
+  bool field_exists(const reg_t which) {
+    return (sstatus_write_mask & which) != 0;
+  }
+
  protected:
   reg_t adjust_sd(const reg_t val) const noexcept;
   void maybe_flush_tlb(const reg_t newval) noexcept;
@@ -202,10 +205,14 @@ typedef std::shared_ptr<base_status_csr_t> base_status_csr_t_p;
 
 // For vsstatus, which is its own separate architectural register
 // (unlike sstatus)
-class vsstatus_csr_t: public base_status_csr_t {
+class vsstatus_csr_t final: public base_status_csr_t {
  public:
   vsstatus_csr_t(processor_t* const proc, const reg_t addr);
-  virtual reg_t read() const noexcept override;
+
+  reg_t read() const noexcept override {
+    return val;
+  }
+
  protected:
   virtual bool unlogged_write(const reg_t val) noexcept override;
  private:
@@ -215,21 +222,14 @@ class vsstatus_csr_t: public base_status_csr_t {
 typedef std::shared_ptr<vsstatus_csr_t> vsstatus_csr_t_p;
 
 
-class sstatus_proxy_csr_t: public base_status_csr_t {
+class mstatus_csr_t final: public base_status_csr_t {
  public:
-  sstatus_proxy_csr_t(processor_t* const proc, const reg_t addr, csr_t_p mstatus);
-  virtual reg_t read() const noexcept override;
- protected:
-  virtual bool unlogged_write(const reg_t val) noexcept override;
- private:
-  csr_t_p mstatus;
-};
+  mstatus_csr_t(processor_t* const proc, const reg_t addr);
 
+  reg_t read() const noexcept override {
+    return val;
+  }
 
-class mstatus_csr_t: public base_status_csr_t {
- public:
-  mstatus_csr_t(processor_t* const proc, const reg_t addr);
-  virtual reg_t read() const noexcept override;
  protected:
   virtual bool unlogged_write(const reg_t val) noexcept override;
  private:
@@ -252,17 +252,33 @@ class mstatush_csr_t: public csr_t {
 };
 
 
+class sstatus_proxy_csr_t final: public base_status_csr_t {
+ public:
+  sstatus_proxy_csr_t(processor_t* const proc, const reg_t addr, mstatus_csr_t_p mstatus);
+
+  reg_t read() const noexcept override {
+    return mstatus->read() & sstatus_read_mask;
+  }
+
+ protected:
+  virtual bool unlogged_write(const reg_t val) noexcept override;
+ private:
+  mstatus_csr_t_p mstatus;
+};
+
+typedef std::shared_ptr<sstatus_proxy_csr_t> sstatus_proxy_csr_t_p;
+
 class sstatus_csr_t: public virtualized_csr_t {
  public:
-  sstatus_csr_t(processor_t* const proc, base_status_csr_t_p orig, base_status_csr_t_p virt);
+  sstatus_csr_t(processor_t* const proc, sstatus_proxy_csr_t_p orig, vsstatus_csr_t_p virt);
 
   // Set FS, VS, or XS bits to dirty
   void dirty(const reg_t dirties);
   // Return true if the specified bits are not 00 (Off)
   bool enabled(const reg_t which);
  private:
-  base_status_csr_t_p orig_sstatus;
-  base_status_csr_t_p virt_sstatus;
+  sstatus_proxy_csr_t_p orig_sstatus;
+  vsstatus_csr_t_p virt_sstatus;
 };
 
 typedef std::shared_ptr<sstatus_csr_t> sstatus_csr_t_p;
@@ -605,7 +621,7 @@ class dcsr_csr_t: public csr_t {
 typedef std::shared_ptr<dcsr_csr_t> dcsr_csr_t_p;
 
 
-class float_csr_t: public masked_csr_t {
+class float_csr_t final: public masked_csr_t {
  public:
   float_csr_t(processor_t* const proc, const reg_t addr, const reg_t mask, const reg_t init);
   virtual void verify_permissions(insn_t insn, bool write) const override;
@@ -613,6 +629,8 @@ class float_csr_t: public masked_csr_t {
   virtual bool unlogged_write(const reg_t val) noexcept override;
 };
 
+typedef std::shared_ptr<float_csr_t> float_csr_t_p;
+
 
 // For a CSR like FCSR, that is actually a view into multiple
 // underlying registers.

riscv/processor.h

@@ -217,8 +217,8 @@ struct state_t
   static const int max_pmp = 16;
   pmpaddr_csr_t_p pmpaddr[max_pmp];
 
-  csr_t_p fflags;
-  csr_t_p frm;
+  float_csr_t_p fflags;
+  float_csr_t_p frm;
 
   csr_t_p menvcfg;
   csr_t_p senvcfg;