proteus-core / proteus

The SpinalHDL design of the Proteus core, an extensible RISC-V core.
MIT License
39 stars 9 forks source link

Dynamic Pipeline stalls when changing ibusLatency #5

Closed eyck closed 1 year ago

eyck commented 1 year ago

Hi, I wanted to run a simple hello world program in the dynamic pipeline of Proteus but the core somewhere stalls. Running the same program on the static version yields the expected results. Therefore my question before spending further time debugging: is the dynamic pipeline supposed to run properly or is it work in progress?

As reference I'll attach the elf of the program, the expected output is:

$ ./sim/build/sim ../PROT-RUN/program.bin 
Factorial is 3628800
End of execution
Progam has exited with code:0x0

hello.elf.zip

martonbognar commented 1 year ago

Hello, thanks for submitting an issue!

The dynamic pipeline is supposed to have the same functionality as the static pipeline. When converting your program to a binary file with riscv32-unknown-elf-objcopy -O binary hello.elf hello_github.bin and then running it with either simulator (both the static pipeline obtained with make -C tests CORE=riscv.CoreExtMem and the dynamic obtained with make -C tests CORE=riscv.CoreDynamicExtMem), I get the same output as you:

$ ./sim/build/static hello_github.bin 
Factorial is 3628800
End of execution
Progam has exited with code:0x0
^C
$ ./ProteusCore/sim/build/dynamic hello_github.bin 
Factorial is 3628800
End of execution
Progam has exited with code:0x0
^C

Note, however, that in both cases, I had to force cancel the simulation, it did not stop at the end of the program. Maybe the reason you don't see the output for the dynamic pipeline is related to this, and it's just a buffering issue? (The dynamic simulation can also take longer than the static pipeline, due to the complexity of the hardware.)

For stopping the simulation, see here. This should also happen automatically (as far as I know) for C programs compiled with the RISC-V toolchain, e.g., I see this inserted at the end of the compiled Newlib example:

lui     ra, 0x10000
li      sp, 4
sb      sp, 0(ra)

Let me know if you figure this out or need further assistance!

eyck commented 1 year ago

I know that the FW does not stop, it has been 'reused' from another system.

Actually I did check the waveform and saw that the dynamic pipeline did miss as function return...

What version of Scala/sbt/SpinaHDL/verilator are you using? In the past I found cases where verilator had issues simulating SpinlaHDL generated constructs...

martonbognar commented 1 year ago

Can you try using our Docker container? For me it also works with that version, and that should be easier to reproduce. For information, the versions in the container are:

root@proteus:/proteus# sbt --version
sbt version in this project: 1.8.0
sbt script version: 1.8.2
root@proteus:/proteus# verilator --version
Verilator 4.038 2020-07-11 rev v4.036-114-g0cd4a57ad

The Scala and SpinalHDL versions are stored in build.sbt.

eyck commented 1 year ago

THX for the quick reply, this helped a log to sort out potential issues and brought me to the root cause. I tried to set the ibusLatency of the fetcher to 1 which works for the static pipeline but not fopr the dynamic one. Is there any limitation on the range?

martonbognar commented 1 year ago

Great that you found the root cause! I'm not aware of any such limitation, this sounds like a bug. You're definitely very welcome to try to find a fix and I will try to help, but I'll only have time in a week or two to look into this in more detail.

martonbognar commented 1 year ago

I finally started looking into this: to me it appears that setting ibusLatency = 1 breaks both the static and the dynamic pipeline (which is not unexpected, as the Fetcher plugin is shared among them), I will investigate the cause further. Not sure if this issue is still relevant for you, but I'll let you know when it's fixed :)

martonbognar commented 1 year ago

Fixed by c7a6c62d2313240f946c5dca9118bcff9f4a8644

eyck commented 1 year ago

THX for fixing this, it is still relevant for us. I will check quite likely next week...