Merge pull request #1950 from riscv-software-src/fix-instreth

Fix implementation of instreth writes

riscv/csrs.cc

@@ -1075,6 +1075,7 @@ bool virtualized_satp_csr_t::unlogged_write(const reg_t val) noexcept {
 wide_counter_csr_t::wide_counter_csr_t(processor_t* const proc, const reg_t addr, smcntrpmf_csr_t_p config_csr):
   csr_t(proc, addr),
   val(0),
+  written(false),
   config_csr(config_csr) {
 }
 
@@ -1083,7 +1084,15 @@ reg_t wide_counter_csr_t::read() const noexcept {
 }
 
 void wide_counter_csr_t::bump(const reg_t howmuch) noexcept {
-  if (is_counting_enabled()) {
+  if (written) {
+    // Because writing a CSR serializes the simulator, howmuch should
+    // reflect exactly one instruction: the explicit CSR write.
+    // If counting is disabled, though, howmuch will be zero.
+    assert(howmuch <= 1);
+    // The ISA mandates that explicit writes to instret take precedence
+    // over the instret, so simply skip the increment.
+    written = false;
+  } else if (is_counting_enabled()) {
     val += howmuch;  // to keep log reasonable size, don't log every bump
   }
   // Clear cached value
@@ -1091,23 +1100,15 @@ void wide_counter_csr_t::bump(const reg_t howmuch) noexcept {
 }
 
 bool wide_counter_csr_t::unlogged_write(const reg_t val) noexcept {
+  // Because writing a CSR serializes the simulator and is followed by a
+  // bump, back-to-back writes with no intervening bump should never occur.
+  assert(!written);
+  written = true;
+
   this->val = val;
-  // The ISA mandates that if an instruction writes instret, the write
-  // takes precedence over the increment to instret.  However, Spike
-  // unconditionally increments instret after executing an instruction.
-  // Correct for this artifact by decrementing instret here.
-  // Ensure that Smctrpmf hasn't disabled counting.
-  if (is_counting_enabled()) {
-    this->val--;
-  }
   return true;
 }
 
-reg_t wide_counter_csr_t::written_value() const noexcept {
-  // Re-adjust for upcoming bump()
-  return this->val + 1;
-}
-
 // Returns true if counting is not inhibited by Smcntrpmf.
 // Note that minstretcfg / mcyclecfg / mhpmevent* share the same inhibit bits.
 bool wide_counter_csr_t::is_counting_enabled() const noexcept {

riscv/csrs.h

@@ -542,10 +542,10 @@ class wide_counter_csr_t: public csr_t {
   void bump(const reg_t howmuch) noexcept;
  protected:
   virtual bool unlogged_write(const reg_t val) noexcept override;
-  virtual reg_t written_value() const noexcept override;
  private:
   bool is_counting_enabled() const noexcept;
   reg_t val;
+  bool written;
   smcntrpmf_csr_t_p config_csr;
 };
 

riscv/execute.cc

@@ -365,12 +365,10 @@ void processor_t::step(size_t n)
       in_wfi = true;
     }
 
-    if (!(state.mcountinhibit->read() & MCOUNTINHIBIT_IR))
-      state.minstret->bump(instret);
+    state.minstret->bump((state.mcountinhibit->read() & MCOUNTINHIBIT_IR) ? 0 : instret);
 
     // Model a hart whose CPI is 1.
-    if (!(state.mcountinhibit->read() & MCOUNTINHIBIT_CY))
-      state.mcycle->bump(instret);
+    state.mcycle->bump((state.mcountinhibit->read() & MCOUNTINHIBIT_CY) ? 0 : instret);
 
     n -= instret;
   }