Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vcs: add dramsim on emu run #302

Merged
merged 2 commits into from
Mar 13, 2024
Merged

Conversation

xiaokamikami
Copy link
Contributor

Support for enabling dramsim3 programs to run on Paradine. DRAMSIM3_HOME Note: Recompile dramsim3 on Paladin and set DRAMSIM3_HOME, then add DRAMSIM3_HOME to LD_LIBRARY_PATH so that it links to the dynamic library correctly

@xiaokamikami xiaokamikami force-pushed the master branch 2 times, most recently from 6f67cd2 to 8ec5cae Compare March 7, 2024 06:12
@@ -440,6 +440,8 @@ void dramsim3_init() {

assert(dram == NULL);
dram = new ComplexCoDRAMsim3(DRAMSIM3_CONFIG, DRAMSIM3_OUTDIR);
if (dram == NULL)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary? Seems we call dramsim same as emu. No need to change the common interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prevents references to null Pointers from causing Paladin to crash,If you don't think this is necessary, I can add a macro definition to enable checks on Paladin

@@ -199,6 +205,9 @@ extern "C" void simv_nstep(uint8_t step) {
extern "C" uint8_t simv_nstep(uint8_t step) {
if (difftest == NULL)
return 0;
#ifdef WITH_DRAMSIM3
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to trigger simv_tick every cycle. Sperate it for single interface, rather than leave it everywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@xiaokamikami xiaokamikami force-pushed the master branch 5 times, most recently from c58f10e to ff28020 Compare March 7, 2024 08:32
@xiaokamikami xiaokamikami force-pushed the master branch 3 times, most recently from 6f695c2 to 2cfa5dc Compare March 13, 2024 09:09
static uint8_t simv_result = SIMV_RUN;
#ifdef WITH_DRAMSIM3
extern "C" void simv_tick() {
#ifdef CONFIG_DIFFTEST_DEFERRED_RESULT
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to check simv_result and difftest here. You can check whether dram is NULL in dramsim3_step

@@ -190,7 +222,6 @@ void simv_finish() {
simMemory = nullptr;
}

static uint8_t simv_result = SIMV_RUN;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to change here.

@@ -152,6 +162,14 @@ extern "C" uint8_t simv_step() {
if (max_instrs != 0) { // 0 for no limit
auto trap = difftest[0]->get_trap_event();
if (max_instrs < trap->instrCnt) {
if (ckpt_item != NULL) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems unexpected changes for dramsim? May you only push master branch on your fork repo

@xiaokamikami xiaokamikami force-pushed the master branch 2 times, most recently from f021b6d to 766c123 Compare March 13, 2024 09:52
@poemonsense poemonsense merged commit 965fcd3 into OpenXiangShan:master Mar 13, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants