riscv-software-src / riscof

BSD 3-Clause "New" or "Revised" License
61 stars 39 forks source link

Weird behaviour while running riscof run #38

Closed aignacio closed 2 years ago

aignacio commented 2 years ago

Hey,

I'm trying to run the riscof with my design but what I'm observing is that sometimes the tool does not wait for my simulation to run/end or either it doesn't compile the test program, thus it calls the simulator with an inexistent my.elf file. Then if I enter in the folder and run the target manually, it compiles and run the test program as expected generating the proper signature. This was working before with the old setup of riscv-arch-tests. Also, as you can check in the picture it fails in the compilation of some tests with the redefinition of some macros.

image

In the image below for instance, the signature was being dumped correctly (right- reference) when the tool just stopped the sim run as you can check in the left hand side.

image
pawks commented 2 years ago

Hi @aignacio , The warning messages regarding the redefinition of macros you see aren't a problem. They are just warnings thrown by the compiler due to the TEST_CASE_1 macro being defined in the environment files of the tests. The tests still compile properly. This was done to ensure that the tests can run on the older framework. As for the early exit of the simulator, the execution of the tests is controlled by the plugins. You will need to make the change in your plugin. If you are using the makefile based flow provided by riscof ensure that the timeout value specified to the execute_all command is high enough to let your simulation end properly. The documentation for the same can be found here. The default value is set to 300. You can specify timeout = None to not exit until the forked process exits. I wouldn't advise on doing that. Instead you can specify timeout = 500 or some higher number while calling the function.

Example for the sail_cSim plugin: This line here will change to:

        make.execute_all(self.work_dir,timeout=500)
aignacio commented 2 years ago

Hey @pawks,

you're right, just changed the timeout and started working as expected, had to increase to 10000. Thanks for the help!, is there a way to add waivers to the run? like in the case where your design doesn't support compressed but you want to have the output of the run without any errors?

pawks commented 2 years ago

The isa.yaml of your model specifies the configuration and riscof filters the tests to run based on the same information. A detailed specification of all the fields in the yaml can be found here. In your case, the isa node in the yaml should not contain the C letter.

aignacio commented 2 years ago

Hey @pawks, I understand that aspect you mentioned but if the RV core cannot handle compressed extension, there're some base I tests that will fail anyway, I'd like to find a way to withdrawn or mask these errors off to have full clean output run, for instance take a look in the image here, if I'm not wrong, designs that cannot support C, must fail according to the description here. It'd be good to have these tests removed in case there's no C added in the isa.yaml [as it's my case] as you mentioned. image image

pawks commented 2 years ago

That disclaimer is only applicable to the references uploaded in the arch-test repository. In riscof , if the C extension is not implemented, the reference model is also run without the C extension. Hence, disclaimers 2&3 aren't applicable while running with riscof. Eventually the reference model will also support misaligned load-stores natively and those test failures will go away. There is no way of filtering out the results of a few tests from a riscof run. It is intentional as these reflect shortcomings(of the RTL, tests, reference or the framework). You can get waivers for each of these tests in order to claim architectural compatibility as per the RISCV policy.

aignacio commented 2 years ago

Hey @pawks,

if you take a look at my core cfg, there's no mention to the C extension so I was not expecting to see failures while running with RISCOF as the sail c emulator should not have these ones too.

hart_ids: [0]
hart0:
  ISA: RV32I
  physical_addr_sz: 32
  User_Spec_Version: '2.3'
  Privilege_Spec_Version: '1.11'
  hw_data_misaligned_support: true
....

Also, correct me if I'm wrong but according to the QuickStart tutorial, you still use the tests in the arch-test (there's also a typo in the image if you run this cmd) so why the disclaimers wouldn't be applicable exactly? image Can you elaborate more here? "Eventually the reference model will also support misaligned load-stores natively and those test failures will go away"

pawks commented 2 years ago

The tests which require the trap handler are run only if the Zicsr extension is present in the isa string. If you look at the test list generated for the model, you will find that most of the tests mentioned under point 2 of the disclaimer are missing. You can add the extension to run those tests. However, those tests will still pass on your model while using riscof.

Also, correct me if I'm wrong but according to the QuickStart tutorial, you still use the tests in the arch-test (there's also a typo in the image if you run this cmd) so why the disclaimers wouldn't be applicable exactly?

In the old framework(the makefile based one in riscv-arch-test), the signatures generated by the DUT are checked against the statically checked in signatures in the repo. The tests themselves are equipped to handle both scenarios, its only the signatures which change. In riscof, the reference signatures are generated dynamically by running the reference model(with the appropriate configuration) and then verifying that the DUT also produces the same signature. This ensures that the reference model closely resembles the DUT(at the ISA level).

Can you elaborate more here? "Eventually the reference model will also support misaligned load-stores natively and those test failures will go away" The reason you see the tests failing right now is because the reference model traps on a misaligned memory access while the DUT loads the value appropriately. This behaviour influences the signature and causes a mismatch. Once the reference model has the ability to support misaligned accesses, it can also load the value correctly without trapping in which case the signatures will be identical.

aignacio commented 2 years ago

Thanks @pawks, it seems clear now. Is there a plan for the Sail C-emulator to support misaligned loads & stores? or in this case it'd be better to use Spike as the reference for signature comparison? (not sure if Spike supports misaligned l/s)

pawks commented 2 years ago

To my knowledge, SAIL should eventually have support for misaligned memory operations. I think the respective repositories are a better place for more clarifications on that front.

aignacio commented 2 years ago

Just for reference, to enable misaligned support in the SAIL RISC-V simulator, we need to add the --enable-misaligned switch to the riscof_sail_cSim.py, like this:

...
            execute += self.sail_exe[self.xlen] + ' --enable-misaligned --test-signature={0} {1} > {2}.log 2>&1;'.format(sig_file, elf, test_name)