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
135 stars 72 forks source link

The method cache is filled on every call and return, even if the target is already cached #112

Closed michael-platzer closed 2 years ago

michael-platzer commented 2 years ago

If I interpret the OCP port signals correctly, it appears that Patmos is filling the method cache on every call and return instruction, regardless of whether the fetched method is already cached.

Steps to observe this behavior:

  1. Save the following test program as test.c:
    
    #include <machine/spm.h>

int fib(int n) { if (n < 2) { return n; } return fib(n-1) + fib(n-2); }

int main() { volatile _SPM int led = (volatile _SPM int ) 0xF0090000; led = 1; int res = fib(3); led = 0; return 0; }

2. Compile, emulate and trace the program with the following commands:

patmos-clang test.c patemu -v a.out

3. Disassemble the generated binary and dump the content to a file. Locate the function `fib` (for me it starts at address 0x20bc4).

patmos-llvm-objdump -d a.out > dump.S


4. Open the trace file `Patmos.vcd` with GTKWave or a similar program. The traces of interest are `TOP/Patmos/Leds/ledReg` (helps with locating the relevant section), `TOP/Patmos/cores_0/icache/io_ocp_port_M_cmd` and `TOP/Patmos/cores_0/icache/io_ocp_port_M_Addr`.

As can be seen in the screenshot below, shortly after `ledReg` is assigned 1, the instruction cache starts fetching addresses 0x20bc0 through 0x20c70, which corresponds to the function `fib`, as expected. However, after a brief break the cache starts fetching the same addresses again.

![Screenshot from 2022-01-03 21-07-42](https://user-images.githubusercontent.com/9446837/147976912-a27ac6a3-0839-4381-ad39-d36db6b6a24e.png)

Zooming out, we can see that the instructions of `fib` are fetched 9 times in total, after which the instruction cache fetches the `main` function (address 0x20c84). Hence, the function `fib` is fetched every time it is called (5 times in total) and every time a recursive call returns to `fib` (4 times). This is redundant, the function is already in the cache when called recursively.

![Screenshot from 2022-01-03 21-07-10](https://user-images.githubusercontent.com/9446837/147976934-5a79981b-034b-4a8a-8683-08842fce80cd.png)

This appears to have a significant impact on the performance of Patmos. For algorithms repeatedly calling small leaf methods the execution time is reduced by up to an order of magnitude when inlining these calls in order to avoid redundant cache refills.
schoeberl commented 2 years ago

Thank you for checking this out! Very bad :-( Would it be possible to do the same test with the Chisel 2 version of Patmos. So we can see if this is a regression due to the port from Chisel 2 to 3.

michael-platzer commented 2 years ago

Commit f50e132, which seems to predate most of the Chisel 3 work, is not affected. Here the function fib is only fetched once, and the main function is not fetched again when returning to it.

Screenshot from 2022-01-04 15-52-11

schoeberl commented 2 years ago

Thanks for checking this out! This actually gives some hope, as the method cache is not broken (in general), but during the port there was a bug introduced. I guess it has to do with the tag memory and/or a valid item.

schoeberl commented 2 years ago

Thank you very much for this super detailed description of how to reproduce the issue! This is an excellent description. I am trying to reproduce this on my side to get started with debugging. However, I am running into trouble with the Scala compiler (Wolfgang did some tricks to use the Scala compiler at runtime). Did you have the same issue and could you solve it? This is the error I get:

[init] error: error while loading Object, Missing dependency 'object scala in compiler mirror', required by /usr/lib/jvm/java-8-openjdk-amd64/jre/lib/rt.jar(java/lang/Object.class)

Failed to initialize compiler: object scala in compiler mirror not found.
** Note that as of 2.8 scala does not assume use of the java classpath.
** For the old behavior pass -usejavacp to scala, or if using a Settings
** object programmatically, settings.usejavacp.value = true.
[error] (run-main-0) scala.reflect.internal.MissingRequirementError: object scala in compiler mirror not found.
[error] scala.reflect.internal.MissingRequirementError: object scala in compiler mirror not found.
schoeberl commented 2 years ago

Never mind, I found a later version that does just a single cache load and does not depend on the compiler at runtime. However, it is irritating that I cannot build an older version of Patmos. I tried two different Ubuntu versions (16.04 and 20.04), no chance.

michael-platzer commented 2 years ago

I also had issues when trying to build older versions but I happened to have a build of commit f50e132 on another computer.

schoeberl commented 2 years ago

This is still good (Chisel 2) and compiles: 86c1c4503de This has the error: 8241f0ad6f Inbetween are commits that don't compile.