Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The coherence units add unnecessary latency and logic to bus requests. This PR removes the middle man and directly integrates the
bus_controller
intol1_cache
. With this change, it will be easier to debug coherence/deadlock issues, and (this is purely speculative) reduce the ability to deadlock due to interoperating FSMs (cache, coherence unit, bus). The coherence stress test continues to work fine, and riscv-tests continue to pass.One thing I want to change before merging is the hit logic for writes. Currently, there's an extremely nasty edge case where if a write request is started during a snoop hit, it will lower
proc_gen_bus_if.busy
before actually writing to the SRAM. This should only affect things which are able to lowerren
/wen
on the negative edge of the clock, however, I'm sure there's a better way to represent this logic that I can't think of right now.Performance
In merge sort, there is a ~5% speedup over the previous design. For the
global_atomic
toy test which tests atomic addition on a highly contentious variable, speedup is ~10%. There is likely still performance that can be squeezed out of the memory subsystem, however, its out of the scope of this PR.