t-crest / patmos

Patmos is a time-predictable VLIW processor, and the processor for the T-CREST project
http://patmos.compute.dtu.dk
BSD 2-Clause "Simplified" License
134 stars 72 forks source link

Make-tests fail #142

Closed Emoun closed 7 months ago

Emoun commented 7 months ago

The command make test currently fails with all tests failing to run correctly.

This issue tracks getting that fixed.

Emoun commented 7 months ago

Looking at the asm/basic.s test, w get the error:

=== ISA Tests ===
basic
 Chisel trace incomplete: 6
 failed

We see that the log for emutatos output (tmp/basic.emu.out) does not include any PASSED text at the end. This is in contrast to the ouput from the hardware simulator output (tmp/basic.sim.out) which ends with:

20 - 0 15 4 3 7 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
24 - 0 15 4 3 7 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
PASSED
[success] Total time: 11 s, completed Feb 1, 2024, 2:51:49 PM
make[1]: Leaving directory '/home/user/t-crest/patmos'
EXIT 0

Looking at tools/java/src/util/CompareScala.java, we see that it expects both the emulator and simulator to outpus PASSED at the end of their output. So, why doesn't the emulator?

Looking at hardware/Patmos-harness.cpp, we see that line 936 outputs Patmos start. But nowhere does PASSED get output. Adding the output at the end solves many tests.

It seems this was an oversight when the emulator was moved to use Patmos-harness.cpp. These test keywords should have been added as part of this commit

Emoun commented 7 months ago

Next issue is with the compare test.

compare
Exception in thread "main" java.util.InputMismatchException
        at java.base/java.util.Scanner.throwFor(Scanner.java:939)
        at java.base/java.util.Scanner.next(Scanner.java:1594)
        at java.base/java.util.Scanner.nextInt(Scanner.java:2258)
        at java.base/java.util.Scanner.nextInt(Scanner.java:2212)
        at util.CompareScala.main(CompareScala.java:160)
 failed

Looking at the log (tmp/compare.sim.out), we see the error:

[error] java.lang.Exception: OpcodeExt 3 not (yet) implemented

Looking at the code in the isasim (isasim/src/main/scala/patsim/PatSim.scala), we can see that the only ALU operation supported is the register source variant. So it makes sense that as soon as an immediate add is reached, patsim complains.

schoeberl commented 7 months ago

PatSim should be taken out of any testing, as this is not finished. Maybe never will be ;-)

Emoun commented 7 months ago

I seem to have it working using pasim instead. I'd like to therefore just delete everything patsim related (including patsim). Is that ok? (If we ever need it again, that is what the git history is for)

schoeberl commented 7 months ago

Leave PatSim as it is for now. Just remove it from the tests. There is more garbage that could be deleted in this repo.

Emoun commented 7 months ago

Fixed in d4164e58f9524488fa1c3c134450733ea834b091