From 630b4fe3061e17fc0533adcc0f7150c7245196ea Mon Sep 17 00:00:00 2001 From: Yinan Xu Date: Thu, 21 Sep 2023 12:18:55 +0800 Subject: [PATCH] Support global enable control for DPI-C (#166) This commit adds an optional global enable bit for DPI-C function calls to avoid useless DPI-C calls and state synchronization during simulation. --- .github/workflows/main.yml | 11 +++++++++++ src/main/scala/DPIC.scala | 30 +++++++++++++++++++++++++---- src/main/scala/Difftest.scala | 2 ++ src/test/csrc/difftest/difftest.cpp | 14 +++++++++++--- 4 files changed, 50 insertions(+), 7 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index b5b15ff89..f31eddd8b 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -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 diff --git a/src/main/scala/DPIC.scala b/src/main/scala/DPIC.scala index 13bfa3cce..433c55653 100644 --- a/src/main/scala/DPIC.scala +++ b/src/main/scala/DPIC.scala @@ -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 @@ -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 = { @@ -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")), @@ -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 @@ -137,25 +141,36 @@ 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 } @@ -163,6 +178,13 @@ object DPIC { 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__" diff --git a/src/main/scala/Difftest.scala b/src/main/scala/Difftest.scala index 438239962..78ed9da06 100644 --- a/src/main/scala/Difftest.scala +++ b/src/main/scala/Difftest.scala @@ -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 @@ -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 { diff --git a/src/test/csrc/difftest/difftest.cpp b/src/test/csrc/difftest/difftest.cpp index ebd825647..d2bfa3d55 100644 --- a/src/test/csrc/difftest/difftest.cpp +++ b/src/test/csrc/difftest/difftest.cpp @@ -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); @@ -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)) { @@ -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 @@ -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; @@ -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;