Basically did
set(CMAKE_CXX_CLANG_TIDY local/clang-tidy -checks=-*,llvm-use-new-mlir-op-builder -fix)
and then fixed cases where temporary OpBuilders were used (as passed in
by reference now).
This is a fix for an issue introduced by llvm-project:a21abc7 via
b895069. The alignment in LLVM was changed from 8 to 16 bytes but in Arc
we still allocated memory according to the old alignment, thus leading
to overlapping memory accesses (a store to a HW memory could modify the
state of a register stored after it in memory).
Currently, all the pass base classes are included in all files
implementing one specific pass. This aims to reduce the amount of
unnecessarily included code.
As mentioned in the removed TODO, `AllocateState` currently has an expensive combination of `moveBefore` and `isBeforeInBlock` calls. This applies a very similar fix to the one used in `LowerState` to avoid forcing the built in operation ordering to be repeatedly reconstructed.
* Remove the optional enable and clock operands from `arc.memory_read_port` making it a combinatorial read port. Latencies can be added using the `arc.state` operation.
* Adjust InferMemories accordingly
* Make `arc.memory_read_port` pure to allow it to be moved into `arc.define` ops. This means the memory value has to be passed via argument. This will be considered to pass the whole memory as a value (like `hw.array`) at this stage of the lowering. The LegalizeStateUpdates pass should make it easy to switch to pointer semantics for memories.
* Remove top-level `sv.macro.decl` ops in StripSV (sorry for this unrelated change, but it's just 4 lines :) )
* Remove the optional stride parameter from the `!arc.memory` type and add a member function to conveniently compute the stride on-demand. Not removing it led to some issues in the lowering to LLVM which would have required bigger changes to AllocateState otherwise.
* Fix a bug in the ClockDomainOp canonicalizer revealed by the change to the regression test required by the removal of the `memory_read_port` operands
* Change the memory read port lowering in LowerState such that it does not reorder read ops anymore, but just converts them and also lowers Arcs that contain reads to functions early on because arcs don't allow ops with memory side effects. Alternatively we could split the arcs and pull the read ops out again.
* Add the memory read/write reordering to LegalizeStateUpdates instead. This is just a temporary implementation and fails on more complex cases, but we need to re-implement this pass anyways as it is very slow (~4 sec on rocket, 100s of sec on largeBoom)
* Add support for `func.call` op lowering in LowerArcToLLVM and lower arcs with LLVM internal linkage such that LLVM can remove those functions when getting inlined to reduce the objfile size to almost a third for rocket.
Add three passes that implement state allocation. The passes take the
abstract state allocation ops, compute a memory layout for the overall
state of the model, and replace the allocation ops with simple pointer
getter ops that access the allocated piece of memory. The passes operate
as follows:
- `LegalizeStateUpdate` detects read-after-write hazards and introduces
temporary storage locations that allow the read and write ops to
occur without infringing on each other.
- `AllocateState` computes the overall memory layout and replaces
allocation ops with simple accessor ops.
- `PrintStateInfo` emits the memory layout as a JSON file. This allows
other tools to reason about the exact memory layout of the design.
State update legalization is still lacking proper handling of memory
reads and writes, which are significantly more involved than the simple
scalar registers. Follow-up work.
Co-authored-by: Martin Erhart <maerhart@outlook.com>
Co-authored-by: Zachary Yedidia <zyedidia@gmail.com>
Jacques Pienaar