Skip to content

Commit

Permalink
Support global enable control for DPI-C (#166)
Browse files Browse the repository at this point in the history
This commit adds an optional global enable bit for DPI-C function calls
to avoid useless DPI-C calls and state synchronization during simulation.
  • Loading branch information
poemonsense authored Sep 21, 2023
1 parent 8aa9275 commit 630b4fe
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 7 deletions.
11 changes: 11 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,17 @@ jobs:
./build/emu -e 0 -i ./ready-to-run/microbench.bin --diff ./ready-to-run/riscv64-nemu-interpreter-so --dump-footprints microbench.bin
./build/emu -e 0 -i microbench.bin --diff ./ready-to-run/riscv64-nemu-interpreter-so --as-footprints
- name: Difftest with Global DPI-C Enable
run: |
cd $GITHUB_WORKSPACE/../xs-env
source ./env.sh
cd $GITHUB_WORKSPACE/../xs-env/NutShell
source ./env.sh
make clean
sed -i 's/hasGlobalEnable: Boolean = false/hasGlobalEnable: Boolean = true/' difftest/src/main/scala/DPIC.scala
make emu
./build/emu -b 0 -e 0 -i ./ready-to-run/microbench.bin --diff ./ready-to-run/riscv64-nemu-interpreter-so
- name: Difftest with Coverage-Guided Fuzzer
run: |
git clone https://github.com/OpenXiangShan/riscv-isa-sim.git
Expand Down
30 changes: 26 additions & 4 deletions src/main/scala/DPIC.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package difftest.dpic
import chisel3._
import chisel3.util._
import chisel3.experimental.{ChiselAnnotation, DataMirror, ExtModule}
import chisel3.util.experimental.BoringUtils
import difftest._

import java.nio.charset.StandardCharsets
Expand All @@ -28,6 +29,7 @@ class DPIC[T <: DifftestBundle](gen: T) extends ExtModule
with HasExtModuleInline
with DifftestModule[T] {
val clock = IO(Input(Clock()))
val enable = IO(Input(Bool()))
val io = IO(Input(gen))

def getDirectionString(data: Data): String = {
Expand Down Expand Up @@ -60,16 +62,17 @@ class DPIC[T <: DifftestBundle](gen: T) extends ExtModule
override def desiredName: String = gen.desiredModuleName
val dpicFuncName: String = s"v_difftest_${desiredName.replace("Difftest", "")}"
val dpicFuncArgsWithClock: Seq[Seq[(String, Data)]] = {
val common = Seq(Seq(("clock", clock)), Seq(("enable", enable)))
// ExtModule implicitly adds io_* prefix to the IOs (because the IO val is named as io).
// This is different from BlackBoxes.
Seq(("clock", clock)) +: io.elements.toSeq.reverse.map{ case (name, data) =>
common ++ io.elements.toSeq.reverse.map{ case (name, data) =>
data match {
case vec: Vec[_] => vec.zipWithIndex.map { case (v, i) => (s"io_${name}_$i", v) }
case _ => Seq((s"io_$name", data))
}
}
}
val dpicFuncArgs: Seq[Seq[(String, Data)]] = dpicFuncArgsWithClock.tail
val dpicFuncArgs: Seq[Seq[(String, Data)]] = dpicFuncArgsWithClock.drop(2)
val dpicFuncAssigns: Seq[String] = {
val filters: Seq[(DifftestBundle => Boolean, Seq[String])] = Seq(
((_: DifftestBundle) => true, Seq("io_coreid")),
Expand Down Expand Up @@ -125,6 +128,7 @@ class DPIC[T <: DifftestBundle](gen: T) extends ExtModule
|`ifdef DIFFTEST
|$dpicDecl
| always @(posedge clock) begin
| if (enable)
| $dpicFuncName (${dpicFuncArgs.flatten.map(_._1).mkString(", ")});
| end
|`endif
Expand All @@ -137,32 +141,50 @@ class DPIC[T <: DifftestBundle](gen: T) extends ExtModule
setInline(s"$desiredName.v", moduleBody)
}

private class DummyDPICWrapper[T <: DifftestBundle](gen: T) extends Module
private class DummyDPICWrapper[T <: DifftestBundle](gen: T, hasGlobalEnable: Boolean) extends Module
with DifftestModule[T] {
val io = IO(Input(gen))

val dpic = Module(new DPIC(gen))
dpic.clock := clock
val enable = WireInit(true.B)
dpic.enable := enable
if (hasGlobalEnable) {
BoringUtils.addSink(enable, "dpic_global_enable")
}
dpic.io := io
}

object DPIC {
val interfaces = ListBuffer.empty[(String, String, String)]
private val hasGlobalEnable: Boolean = false
private var enableBits = 0

def apply[T <: DifftestBundle](gen: T): T = {
val module = Module(new DummyDPICWrapper(gen))
val module = Module(new DummyDPICWrapper(gen, hasGlobalEnable))
val dpic = module.dpic
if (!interfaces.map(_._1).contains(dpic.dpicFuncName)) {
val interface = (dpic.dpicFuncName, dpic.dpicFuncProto, dpic.dpicFunc)
interfaces += interface
}
if (hasGlobalEnable && module.io.needUpdate.isDefined) {
BoringUtils.addSource(WireInit(module.io.needUpdate.get), s"dpic_global_enable_$enableBits")
enableBits += 1
}
module.io
}

def collect(): Unit = {
if (interfaces.isEmpty) {
return
}
if (hasGlobalEnable) {
val global_en = WireInit(0.U.asTypeOf(Vec(enableBits, Bool())))
for (i <- 0 until enableBits) {
BoringUtils.addSink(global_en(i), s"dpic_global_enable_$i")
}
BoringUtils.addSource(WireInit(global_en.asUInt.orR), "dpic_global_enable")
}
val interfaceCpp = ListBuffer.empty[String]
interfaceCpp += "#ifndef __DIFFTEST_DPIC_H__"
interfaceCpp += "#define __DIFFTEST_DPIC_H__"
Expand Down
2 changes: 2 additions & 0 deletions src/main/scala/Difftest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ abstract class DifftestBundle extends Bundle
case _ => true.B
}
}
def needUpdate: Option[Bool] = if (withValid) Some(getValid) else None
def isFlatten: Boolean = this.isInstanceOf[DifftestWithAddress] &&
this.asInstanceOf[DifftestWithAddress].needFlatten
def numFlattenElements: Int = this.asInstanceOf[DifftestWithAddress].numElements
Expand Down Expand Up @@ -187,6 +188,7 @@ class DiffTrapEvent extends DifftestBundle {
val pc = UInt(64.W)

override val desiredCppName: String = "trap"
override def needUpdate: Option[Bool] = Some(hasTrap || hasWFI)
}

class DiffCSRState extends DifftestBundle {
Expand Down
14 changes: 11 additions & 3 deletions src/test/csrc/difftest/difftest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ int Difftest::step() {
#ifdef CONFIG_DIFFTEST_LRSCEVENT
// sync lr/sc reg microarchitectural status to the REF
if (dut.lrsc.valid) {
dut.lrsc.valid = 0;
struct SyncState sync;
sync.sc_fail = !dut.lrsc.success;
proxy->uarchstatus_sync((uint64_t*)&sync);
Expand All @@ -159,6 +160,8 @@ int Difftest::step() {
if (dut.event.valid) {
// interrupt has a higher priority than exception
dut.event.interrupt ? do_interrupt() : do_exception();
dut.event.valid = 0;
dut.commit[0].valid = 0;
} else {
for (int i = 0; i < CONFIG_DIFF_COMMIT_WIDTH && dut.commit[i].valid; i++) {
if (do_instr_commit(i)) {
Expand Down Expand Up @@ -487,15 +490,19 @@ int Difftest::do_store_check() {
// 8 -> icache mainPipe port1 read PIQ
int Difftest::do_refill_check(int cacheid) {
#ifdef CONFIG_DIFFTEST_REFILLEVENT
auto dut_refill = &(dut.refill[cacheid]);
if (!dut_refill->valid) {
return 0;
}
dut_refill->valid = 0;
static int delay = 0;
delay = delay * 2;
if (delay > 16) { return 1; }
static uint64_t last_valid_addr = 0;
char buf[512];
auto dut_refill = &(dut.refill[cacheid]);
uint64_t realpaddr = dut_refill->addr;
dut_refill->addr = dut_refill->addr - dut_refill->addr % 64;
if (dut_refill->valid == 1 && dut_refill->addr != last_valid_addr) {
if (dut_refill->addr != last_valid_addr) {
last_valid_addr = dut_refill->addr;
if(!in_pmem(dut_refill->addr)){
// speculated illegal mem access should be ignored
Expand Down Expand Up @@ -551,6 +558,7 @@ int Difftest::do_l1tlb_check() {
if (!dut.l1tlb[i].valid) {
continue;
}
dut.l1tlb[i].valid = 0;
PTE pte;
uint64_t paddr;
uint8_t difftest_level;
Expand Down Expand Up @@ -583,7 +591,7 @@ int Difftest::do_l2tlb_check() {
if (!dut.l2tlb[i].valid) {
continue;
}

dut.l2tlb[i].valid = 0;
for (int j = 0; j < 8; j++) {
if (dut.l2tlb[i].valididx[j]) {
PTE pte;
Expand Down

0 comments on commit 630b4fe

Please sign in to comment.