vcs, palldium: support warmup, set difftest_perfCtrl_clean by DPI-C #506

xiaokamikami · 2024-11-15T09:16:47Z

No description provided.

klin02 · 2024-11-19T03:39:02Z

wait until passing perfclean to helper. And we will add ci test for this feature

klin02 · 2024-11-20T15:16:03Z

Previous we use tapAndRead to hidden IO ports for read-only probe like some Difftest Bundle.

However, chisel does not support hierarchical wiring for writable probe, so we must addSource first, which requires us to createTopIO before generating DUT. By the way, addSink/addSource will also be deprecated later, and latest bore API require us to addSource before addSink.

Also, chisel's probe seems can only accessed top-to-bottom. For example, we can use SimTop.xx.yy.zz in SimTop, but can not use SimTop.aa in Module SimTop.xx.yy. That means even we addSource first and use tapAndRead, explicit IO port will be generated to pass timer and so on.

In next step, seems we should still try isVisible to reduce number of LogPerfHelper

klin02 · 2024-11-21T01:23:18Z

Seems we can use Builder.currentModule and tap to get LogPerfelper instantiate in same parent Module. I'm trying but some helper instantiated in when scope can not accessed from even same module.

klin02 · 2024-11-21T06:05:19Z

src/test/csrc/vcs/vcs_main.cpp

@@ -44,6 +42,8 @@ static bool enable_difftest = true;
 static uint64_t max_instrs = 0;
 static char *workload_list = NULL;
 static uint32_t overwrite_nbytes = 0xe00;
+static uint64_t warmup_instrs = 0;
+static svScope wamupScope;


warmup instead of wamup

klin02 · 2024-11-21T06:05:35Z

src/test/csrc/vcs/vcs_main.cpp

@@ -57,6 +57,8 @@ enum {
  SIMV_FAIL,
 } simv_state;

+extern "C" void claer_perfcnt();


clear instead of claer

klin02 · 2024-11-21T06:08:35Z

src/test/csrc/vcs/vcs_main.cpp

@@ -84,6 +86,17 @@ extern "C" void set_overwrite_autoset() {
  fclose(fp);
 }

+// The workload warms up and clears the instruction counter
+extern "C" void set_warmup_insts(uint64_t warmup_inst, uint64_t cmn_inst) {


Why cmn_inst here? We only need to pass warmup from vcs_top, and set difftest_clean/dump by dpic

klin02 · 2024-11-21T06:14:21Z

src/test/vsrc/vcs/DifftestEndpoint.v

 assign difftest_uart_in_ch = 8'hff;

+export "DPI-C" task claer_perfcnt;


MissSpelling here too.

klin02 · 2024-11-21T06:16:30Z

src/test/vsrc/vcs/DifftestEndpoint.v

+  dpi_perfCtrl_clean <= 1'b1;
+endtask
+
+reg clear_perf_cnt;


Maybe we can simplify logic here. Use RegNext(just example) as mask to ensure clean raise only 1 cycle.

klin02 · 2024-11-21T11:12:41Z

src/test/csrc/vcs/vcs_main.cpp

+// The workload warms up and clears the instruction counter
+extern "C" void set_warmup_insts(uint64_t warmup_inst, uint64_t cmn_inst) {
+  warmup_instrs = warmup_inst + cmn_inst;
+  wamupScope = svGetScope();


warmup here.

klin02 · 2024-11-21T11:13:04Z

src/test/csrc/vcs/vcs_main.cpp

+    printf("Error: Could not retrieve wamup scope, set first\n");
+    assert(wamupScope);
+  }
+  printf("set warmp insts %ld, cmn insts %ld\n", warmup_inst, cmn_inst);


warmup here.

klin02 · 2024-11-21T11:14:15Z

src/test/csrc/vcs/vcs_main.cpp

@@ -57,6 +57,8 @@ enum {
  SIMV_FAIL,
 } simv_state;

+extern "C" void clear_perfcnt();


Also use set_[signalName] for consistent API style.

klin02 · 2024-11-21T11:16:03Z

src/test/csrc/vcs/vcs_main.cpp

+    auto trap = difftest[i]->get_trap_event();
+    if (warmup_instrs != 0 && trap->instrCnt > warmup_instrs) {
+      svSetScope(wamupScope);
+      clear_perfcnt();


Wrap setScope and setSignal to same function. Don't expose Scope to avoid abuse.

klin02 · 2024-11-21T11:17:20Z

src/test/vsrc/vcs/DifftestEndpoint.v

@@ -50,6 +50,7 @@ import "DPI-C" function void set_max_instrs(longint mc);
 import "DPI-C" function longint get_stuck_limit();
 import "DPI-C" function void set_overwrite_nbytes(longint len);
 import "DPI-C" function void set_overwrite_autoset();
+import "DPI-C" context function void set_warmup_insts(longint workload_inst, longint cmn_inst);


Seems no need to add context ?

klin02 · 2024-11-21T11:19:09Z

src/test/vsrc/vcs/DifftestEndpoint.v

+  if ($test$plusargs("warmup-inst")) begin
+    $value$plusargs("warmup-inst=%d", warmup_instrs);
+  end
+  if ($test$plusargs("cmn-instrs")) begin


I think it's no need to pass cmn-instrs here.
Difftest should only process basic workload, no need to care CMN. Total warmup-insts should be calculated before passing to Difftest

klin02 · 2024-11-21T11:24:36Z

src/test/vsrc/vcs/DifftestEndpoint.v

@@ -170,12 +185,29 @@ initial begin
  end
 end

+reg dpi_perfCtrl_clean;


Use simply difftest_perfCtrl_clean_r for same coding style.

klin02 · 2024-11-21T11:26:23Z

src/test/vsrc/vcs/DifftestEndpoint.v

+  end else begin
+    if (dpi_perfCtrl_clean) begin
+        dpi_perfCtrl_clean <= 1'b0;
+        $display("claer perfCnt");


Maybe no need to add display. Code here is just to set a singal, the effect has already printed by Software

klin02 · 2024-11-25T06:58:10Z

src/test/csrc/vcs/vcs_main.cpp

@@ -84,6 +86,17 @@ extern "C" void set_overwrite_autoset() {
  fclose(fp);
 }

+// The workload warms up and clears the instruction counter
+extern "C" void set_warmup_insts(uint64_t warmup_inst, uint64_t cmn_inst) {
+  warmup_instrs = warmup_inst + cmn_inst;


Is it necessary to add CMN here? Difftest is a common verification framework cross mutli-platform, no need to add specific param here if we can add in another way, such as platform-specific scripts.

klin02 · 2024-11-25T06:59:29Z

src/test/vsrc/vcs/DifftestEndpoint.v

+  dpic_perfCtrl_clean = 1'b1;
+endtask
+
+always @(posedge clock) begin


Use difftest_perfCtrl_clean_r for same nameing style.

klin02 · 2024-11-25T07:28:03Z

src/test/csrc/vcs/vcs_main.cpp

+// The workload warms up and clears the instruction counter
+extern "C" void set_warmup_insts(uint64_t warmup_inst) {
+  warmup_instrs = warmup_inst;
+  wamupScope = svGetScope();


MissSpelling here. warmup instead of wamup

klin02 · 2024-11-25T07:33:04Z

src/test/csrc/vcs/vcs_main.cpp

+extern "C" void set_warmup_insts(uint64_t warmup_inst) {
+  warmup_instrs = warmup_inst;
+  wamupScope = svGetScope();
+  if (wamupScope == NULL) {


No need to check here, but need to check svSetSope.

klin02 · 2024-11-25T07:35:45Z

src/test/csrc/vcs/vcs_main.cpp

+  for (int i = 0; i < NUM_CORES; i++) {
+    auto trap = difftest[i]->get_trap_event();
+    if (warmup_instrs != 0 && trap->instrCnt > warmup_instrs) {
+      svSetScope(wamupScope);


Wrap setScope and setSignal to same function. Don't expose Scope to avoid abuse.
Modified as review before. Like below

extern "C" void set_squash_enable(int enable); void difftest_squash_enable(int enable) { if (squashScope == NULL) { printf("Error: Could not retrieve squash scope, set first\n"); assert(squashScope); } svSetScope(squashScope); set_squash_enable(enable); }

klin02 · 2024-11-25T07:41:09Z

src/test/csrc/vcs/vcs_main.cpp

@@ -44,6 +42,8 @@ static bool enable_difftest = true;
 static uint64_t max_instrs = 0;
 static char *workload_list = NULL;
 static uint32_t overwrite_nbytes = 0xe00;
+static uint64_t warmup_instrs = 0;
+static svScope wamupScope;


Maybe we should seperate scope to warmup. We can add a context function set_difftest_endpoint_scope and rm context from passing warmup_instrs. Later we may also use the scope to change top ports of Difftest like dump clean and so on.

klin02 · 2024-11-25T08:48:03Z

Previous commit is made by @xiaokamikami. Author is append because of rebasing.

xiaokamikami force-pushed the wamup_vcs branch from d7fde62 to 738ad3a Compare November 15, 2024 09:47

xiaokamikami marked this pull request as ready for review November 18, 2024 02:01

xiaokamikami force-pushed the wamup_vcs branch from 290db17 to 5671a64 Compare November 20, 2024 08:58

xiaokamikami force-pushed the wamup_vcs branch from 5671a64 to 6d33ca9 Compare November 21, 2024 03:03

klin02 reviewed Nov 21, 2024

View reviewed changes

xiaokamikami force-pushed the wamup_vcs branch from 6d33ca9 to 9af8f51 Compare November 21, 2024 10:36

klin02 reviewed Nov 21, 2024

View reviewed changes

xiaokamikami force-pushed the wamup_vcs branch from 0d42b8c to 73db456 Compare November 25, 2024 03:36

klin02 reviewed Nov 25, 2024

View reviewed changes

xiaokamikami force-pushed the wamup_vcs branch from 3288ad0 to 741dca1 Compare November 25, 2024 07:08

klin02 reviewed Nov 25, 2024

View reviewed changes

xiaokamikami added 5 commits November 25, 2024 16:00

diff: The hardware performance counter is cleared at the necessary wamup

0f9c7e1

diff: fix formate

86b382a

diff: fix dpi_perfCtrl_clean

7cf22e1

diff: add insrts wamup ci

493cb2f

diff: rm cmn instr set

11970f8

klin02 force-pushed the wamup_vcs branch from a884cf9 to 2f756f0 Compare November 25, 2024 08:46

klin02 changed the title ~~diff: The hardware performance counter is cleared at the necessary wamup~~ Difftest: support workload warmup for vcs/palladium Nov 25, 2024

klin02 changed the title ~~Difftest: support workload warmup for vcs/palladium~~ vcs, palldium: support workload warmup, enable signal modified by DPI-C Nov 25, 2024

klin02 force-pushed the wamup_vcs branch from 2f756f0 to 99c63ad Compare November 25, 2024 08:56

Seperate common scope for later interface, format Code

99c63ad

klin02 changed the title ~~vcs, palldium: support workload warmup, enable signal modified by DPI-C~~ vcs, palldium: support warmup, set difftest_perfCtrl_clean by DPI-C Nov 25, 2024

klin02 approved these changes Nov 25, 2024

View reviewed changes

klin02 merged commit 9f43eee into master Nov 25, 2024
5 checks passed

klin02 deleted the wamup_vcs branch November 25, 2024 11:00

Tang-Haojin mentioned this pull request Dec 2, 2024

[BUG] make difftest-so is broken. #519

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

vcs, palldium: support warmup, set difftest_perfCtrl_clean by DPI-C #506

vcs, palldium: support warmup, set difftest_perfCtrl_clean by DPI-C #506

xiaokamikami commented Nov 15, 2024

klin02 commented Nov 19, 2024

klin02 commented Nov 20, 2024 •

edited

Loading

klin02 commented Nov 21, 2024

klin02 Nov 21, 2024

klin02 Nov 21, 2024

klin02 Nov 21, 2024

klin02 Nov 21, 2024

klin02 Nov 21, 2024

klin02 Nov 21, 2024

klin02 Nov 21, 2024

klin02 Nov 21, 2024

klin02 Nov 21, 2024

klin02 Nov 21, 2024

klin02 Nov 21, 2024

klin02 Nov 21, 2024

klin02 Nov 21, 2024

klin02 Nov 25, 2024

klin02 Nov 25, 2024

klin02 Nov 25, 2024

klin02 Nov 25, 2024

klin02 Nov 25, 2024

klin02 Nov 25, 2024

klin02 commented Nov 25, 2024

		assign difftest_uart_in_ch = 8'hff;

		export "DPI-C" task claer_perfcnt;

vcs, palldium: support warmup, set difftest_perfCtrl_clean by DPI-C #506

vcs, palldium: support warmup, set difftest_perfCtrl_clean by DPI-C #506

Conversation

xiaokamikami commented Nov 15, 2024

klin02 commented Nov 19, 2024

klin02 commented Nov 20, 2024 • edited Loading

klin02 commented Nov 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klin02 commented Nov 25, 2024

klin02 commented Nov 20, 2024 •

edited

Loading