Browse Source

Fix implementation of instreth writes

Decrementing the counter in advance of incrementing it produces
incorrect results when there's a carry-out.  Fix by getting rid of this
scheme altogher: just skip the increment if there was an expicit write.
pull/1950/head
Andrew Waterman 1 year ago
parent
commit
51835fd752
  1. 29
      riscv/csrs.cc
  2. 2
      riscv/csrs.h

29
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 {

2
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;
};

Loading…
Cancel
Save