Perf improvements

[GregAC]

Proposed performance improvements (improving SRAM throughput and Ibex single cycle multiplier), rebased on top of the pinmux changes (#309).

Timing is passing and build time looking good.

Two things were required to achieve this:

1. Increasing the optimization level (Increase optimisation level #302), though this had no clear impact on build times
2. Adding a register stage to the internal SPI loopback (see commit dd85088). The timing path this cuts is odd, there's a single level of logic between two registers with almost 25ns worth of delay. I suspect it's something to do with the SPI timing constraints causing the tool to want the register to output at a hugely skewed time. We may just be on the edge of getting away with it and these changes have pushed it over the edge so regardless of whether we take these changes we should likely cut the internal loopback path.

I've also added saving the utilization and timing reports from the FPGA CI as artifacts so they can be examined after (in particular so you can look at the timing results for any particular PR).

[GregAC]

For reference here is the timing summary from the CI run (from the implementation reports artefact)

    WNS(ns)      TNS(ns)  TNS Failing Endpoints  TNS Total Endpoints      WHS(ns)      THS(ns)  THS Failing Endpoints  THS Total Endpoints     WPWS(ns)     TPWS(ns)  TPWS Failing Endpoints  TPWS Total Endpoints  
    -------      -------  ---------------------  -------------------      -------      -------  ---------------------  -------------------     --------     --------  ----------------------  --------------------  
      0.043        0.000                      0                42250        0.066        0.000                      0                42249        1.741        0.000                       0                 15674  

Here's the utilization report

+----------------------------+-------+-------+------------+-----------+-------+
|          Site Type         |  Used | Fixed | Prohibited | Available | Util% |
+----------------------------+-------+-------+------------+-----------+-------+
| Slice LUTs                 | 22736 |     0 |          0 |     32600 | 69.74 |
|   LUT as Logic             | 22628 |     0 |          0 |     32600 | 69.41 |
|   LUT as Memory            |   108 |     0 |          0 |      9600 |  1.13 |
|     LUT as Distributed RAM |   108 |     0 |            |           |       |
|     LUT as Shift Register  |     0 |     0 |            |           |       |
| Slice Registers            | 15330 |     0 |          0 |     65200 | 23.51 |
|   Register as Flip Flop    | 15330 |     0 |          0 |     65200 | 23.51 |
|   Register as Latch        |     0 |     0 |          0 |     65200 |  0.00 |
| F7 Muxes                   |   551 |     0 |          0 |     16300 |  3.38 |
| F8 Muxes                   |   142 |     0 |          0 |      8150 |  1.74 |
+----------------------------+-------+-------+------------+-----------+-------+

Build time was 11m 30s which is in line with previous build times in CI

[elliotb-lowrisc]

I've also seen an LCD SPI path with 24 ns of delay on a single net. The hold slack on the path was only 13 ns, so I suppose it might be hold-fixing gone wrong.

[elliotb-lowrisc]

Is there a way to make this conditional on whether "top_sonata_timing_summary_postroute_physopted.rpt" exists, so we avoid looking at the wrong report when we switch to the new 2024.1-compatible post-route optimisation flow?

[GregAC]

I think easier to just remember to fix this up when we alter the tool version. It's reasonable for CI to specifically work with the currently agreed tool versions and need modest updates when we change versions.

[elliotb-lowrisc]

I have noticed the lack of reset but think it is fine. The u_spi_core/copi_shift_q driving it also lacks a reset, so this is just replicating the behaviour one cycle later.

[GregAC]

yeah I prefer to leave resets off flops that don't strictly need them. Given the SPI ignores incoming data when it's not been specifically command to clock things in/out then it won't ever actually 'see' the first bit out of reset anyway.

[elliotb-lowrisc]

I've run a bitstream build of this PR and a baseline build with the changes it is based on (up to and including the opt=2 change) using Vivado 2021.1 on my local machine. The breaking of the SPI loopback path did seem to improve the overall WNS (-1.451 -> -0.391). Though, for these two builds at least, the TNS went the other way (-8.168 -> -27.834), though the numbers are still relatively small. Build times were also increase substantially (19m -> 32m) though that is muddied by builds going on in parallel. All this is somewhat concerning (for the state of Sonata if not this PR) but not conclusive. More testing to follow

[elliotb-lowrisc]

Adding timing constraint updates (#316) and building ono showed a more reasonable build time (15m) and timing passing.

[GregAC]

Thanks for checking it out @elliotb-lowrisc. Frustrating it's giving you some failures.

I've certainly see some flakey behaviour from the implementation flow, one version of this (pre pinmux changes) actually gave a noticeable improvement in timing, taking us to 0.5 ns WNS (positive)!

I've rebased this locally now the pinmux changes are in and am doing another run, will see what results!

[GregAC]

Well rebasing and running locally gives a terrible timing result (something like -1.5 WNS) and increased build times.

So sadly I think we'll have to reject this for 1.0 :( I will the try the two changes separately and see if they work in isolation.

As @elliotb-lowrisc says I think this is more a reflection of the current state of Sonata rather than the specific things this PR is doing, all too easy to push things the wrong way with what looks like modest design changes.

[elliotb-lowrisc]

I think the SPI loopback change is definitely worth a quick PR and merge if possible

[GregAC]

I think the SPI loopback change is definitely worth a quick PR and merge if possible

Agreed I'll spin out a separate PR

[GregAC]

I have had this just passing timing on latest main but it does look very tight and was starting to push up build times.

So I think we're best off doing this post-1.0. With @elliotb-lowrisc's crossbar change and a couple of simple Ibex timing fixes I'd hope this becomes a lot more comfortable.