riscv-perf-model icon indicating copy to clipboard operation
riscv-perf-model copied to clipboard

Improve Load/Store Unit

Open ghost opened this issue 2 years ago • 6 comments
trafficstars

The load/store unit in the RISC-V Performance Model is a simple, single, in-order, pipeline (sparta::Pipeline) with a fixed number of stages: MMU Lookup, Cache lookup, and completion.

To enhance this unit, or rather to make it more extensible, the taker of this GH issue must understand the current limitations of the block.

There is no unit test for the load/store unit

  • [X] Add a unit test similar to Rename and Dispatch
  • [ ] Add testing for various loads and stores; ensure timing looks sane for given parameters

The pipeline stages are hardcoded

See https://github.com/riscv-software-src/riscv-perf-model/blob/f7c17dcb8b10f80650fa50cbc1affe31e3a0624f/core/LSU.hpp#L98-L104 with fixed latencies and length

  • [X] Fix 1: Remove the enum and replace with load/store parameters; verify current behavior is unchanged.

The MMU can only handle 1 outstanding miss

  • [X] Add support to allow the MMU to service multiple outstanding misses; consider merging multiple misses into 1 access

The cache can only handle 1 outstanding miss

  • [ ] Add support to allow multiple cache misses; consider merging multiple misses to the same line into 1 miss

There is only support for 1 load/store pipeline

  • [ ] #150

Arbitration for load/store pipe is simple

  • [ ] Add support for a smarter arbitration: loads take priority over stores; consider age, etc

Separate loads from stores

  • [ ] Stores are typically not important. Add support to move them off to the side

Other ideas welcomed...

ghost avatar Jun 26 '23 15:06 ghost

Additional improvements:

  • [ ] Support merging of loads/stores that fall into the same cache line into existing/outstanding transactions
  • [ ] Document existing behaviors and planned behaviors (on the wiki)
  • [ ] Disconnect the LS from the L1 cache -- instead, have the LS communicate with a MSS-like unit that contains the L1 (and/or L2, and/or L3, etc). The MSS unit is documented in #13

ghost avatar Jul 07 '23 15:07 ghost

Any reason why #91 was closed? It'll allow the DCache to handle more than 1 miss at a time?

danbone avatar Jan 08 '24 12:01 danbone

#91 was merged and adds some of the requested support listed above. More to come!

klingaard avatar Jan 08 '24 15:01 klingaard

I think there must have been an error in the merge or something, as none of the DCache changes* from #91 made it into #92 or onto master?

*haven't checked other files.

danbone avatar Jan 08 '24 18:01 danbone

Ugh, I misread the history of #91 -- you're right, they didn't get merged directly. I thought that #92 was supposed to include them. @h0lyalg0rithm do you mind getting to the bottom of this?

klingaard avatar Jan 08 '24 20:01 klingaard

Hi Knute I am ooo at the moment. But I assumed Vineeth message you privately letting you know that his PR is still not merged as it doesn't connect to the new L2 cache. I had a quick message with today and he mentioned that he is working on it.

I don't have access to a laptop atm, will look into it on Saturday. Apologize for the delay

On Mon, Jan 8, 2024, 8:19 PM Knute @.***> wrote:

Ugh, I misread the history of #91 https://github.com/riscv-software-src/riscv-perf-model/pull/91 -- you're right, they didn't get merged directly. I thought that #92 https://github.com/riscv-software-src/riscv-perf-model/pull/92 was supposed to include them. @h0lyalg0rithm https://github.com/h0lyalg0rithm do you mind getting to the bottom of this?

— Reply to this email directly, view it on GitHub https://github.com/riscv-software-src/riscv-perf-model/issues/57#issuecomment-1881761567, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFFFG2B5JOVCP4HAORTHJ3YNRIFTAVCNFSM6AAAAAAZUJXCBSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBRG43DCNJWG4 . You are receiving this because you were mentioned.Message ID: @.***>

h0lyalg0rithm avatar Jan 09 '24 15:01 h0lyalg0rithm