ucb-bar / midas

FPGA-Accelerated Simulation Framework Automatically Transforming Arbitrary RTL
Other
97 stars 15 forks source link

step() in peek poke interface in simif is deceptive #49

Closed edwardcwang closed 5 years ago

edwardcwang commented 7 years ago

step() in simif can be deceptive for users who are familiar with Chisel's PeekPokeTesters. Consider the following example:

import chisel3._

class ShiftRegister extends Module {
  val io = IO(new Bundle {
    val in1  = Input(UInt(8.W))
    val in2  = Input(UInt(8.W))
    val out = Output(UInt(8.W))
    val enable = Input(Bool())
  })
  val out = RegInit(0.U(8.W))
  io.out := out
  when (io.enable) {
    out := io.in1 + io.in2
  }
}
#include "simif.h"

class ShiftRegister_t: virtual simif_t
{
public:
  void run() {
    std::vector<uint32_t> reg(4);
    target_reset();
    poke(io_enable, 0);
    step(5);
    poke(io_enable, 1);
    step(1);
    poke(io_in1, 10);
    poke(io_in2, 20);
    step(1);
    expect(io_out, 30);
  }
};

In chisel-testers, the poke-step-expect sequence works as expected, but since simif's "step" is really "fire one cycle of targetFire", the expect line actually dequeues the old value of io_out right before the posedge, which is different from what the chisel-testers do.

Suggestions: either document this, or perhaps rename as "targetFireStep()" or some other disambiguated name, or provide step(1) as an alias for targetFireStep(2), etc.

@davidbiancolin

davidbiancolin commented 6 years ago

It'll be much easier to handle this intuitively once the new F1 pass is on dev.

davidbiancolin commented 5 years ago

Resolved in Golden Gate