Skip to content

System calls should flush pipeline #1257

Closed
pavelkryukov opened this issue Mar 6, 2020 · 3 comments · Fixed by #1258
Closed

System calls should flush pipeline #1257

pavelkryukov opened this issue Mar 6, 2020 · 3 comments · Fixed by #1258
Assignees
Labels
2 Small features, tests coverage, simple laboratory works bug Fixes a bug or potential bug in simulation. S1 — Pipeline To solve the issue, you need knowledge about pipeline, data bypass, scoreboarding

Comments

@pavelkryukov
Copy link
Member

pavelkryukov commented Mar 6, 2020

@exucutional reported in: #1140 (comment)

Also I am trying to figure out where is the bug in performance simulator. Program works only with -f option.

Usually it indicates a bug in performance simulator
The different instructions are:	
Checker output: 0x1008c: {6}	addi $t1, $sp, 0	 [ $t1 = 0x1 ]
PerfSim output: 0x1008c: {6}	addi $t1, $sp, 0	 [ $t1 = 0x5 ]

The problem existed even before RISC-V implementation. The issue is that pipeline is not flushed after system-call zero time execution, so previous value of register is sticking for input system calls.

The buggy method is here. It should call set_target for system call instructions.

void Writeback<ISA>::writeback_instruction_system( Writeback<ISA>::Instr* instr, Cycle cycle)

@pavelkryukov pavelkryukov added bug Fixes a bug or potential bug in simulation. 0 This task has the owner who does not participate in scoring system. S1 — Pipeline To solve the issue, you need knowledge about pipeline, data bypass, scoreboarding labels Mar 6, 2020
@pavelkryukov pavelkryukov self-assigned this Mar 6, 2020
@pavelkryukov pavelkryukov added 2 Small features, tests coverage, simple laboratory works and removed 0 This task has the owner who does not participate in scoring system. labels Mar 6, 2020
@pavelkryukov pavelkryukov removed their assignment Mar 6, 2020
@exucutional exucutional self-assigned this Mar 6, 2020
@exucutional
Copy link
Contributor

This method is private. Should I write a test?

@pavelkryukov
Copy link
Member Author

pavelkryukov commented Mar 7, 2020

You must, but you cannot write a test for private method.

First of all, you should write a test that fails because of the wrong behavior. ("red stage")
Then, you fix the code so test passes and other tests are not affected. ("green stage").
Finally, you refactor and optimize code ("refactor stage").

For more details on TDD, please follow our Wiki page: https://github.com/MIPT-ILab/mipt-mips/wiki/Introduction-to-Test-Driven-Development. It's not the best manual, but you may look for lots of resources over the Internet.

For testing, there can be two approaches:

  • Difficult one. You write unit tests for Writeback module from scratch: writing some instructions to input ports, reading the expected output from the output ports. It goes far beyond that issue, we even need a dedicated tracker.
  • Simple one (I suggest going here). We test PerfSim as a whole system. The idea is to provide a binary input and a keyboard input which cause a error. I assume the binary input has no problem for you as it is used already (see code below), keyboard input is not so hard as well, since you may use std::istringstream as a parameter to MARSKernel class instead of std::cin.

TEST_CASE( "Torture_Test: Perf_Sim, RISC-V 32 simple trace")
{
std::ostream nullout( nullptr);
auto sim = create_mars_sim( "riscv32", TEST_PATH "/rv32ui-p-simple", nullout, false);
CHECK( sim->run_no_limit() == Trap::HALT);
CHECK( sim->get_exit_code() == 0);
}

@pavelkryukov
Copy link
Member Author

Now we can study microarchitecture in a much interactive manner! Congratulations!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
2 Small features, tests coverage, simple laboratory works bug Fixes a bug or potential bug in simulation. S1 — Pipeline To solve the issue, you need knowledge about pipeline, data bypass, scoreboarding
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants