[Arc] Improve LowerState to never produce read-after-write conflicts

[fabianschuiki]

This is a complete rewrite of the LowerState pass that makes the LegalizeStateUpdate pass obsolete.

The old implementation of LowerState produces arc.models that still contain read-after-write conflicts. This primarily happens because the pass simply emits arc.state_write operations that write updated values to simulation memory for each arc.state, and any user of arc.state would use an arc.state_read operation to retrieve the original value of the state before any writes occurred. Memories are similar. The Arc dialect considers arc.state_write and arc.memory_write operations to be deferred writes until the LegalizeStateUpdate pass runs, at which point they become immediate since the legalization inserts the necessary temporaries to resolve the read-after-write conflicts.

The previous implementation would also not handle state-to-output and state-to-side-effecting-op propagation paths correctly. When a model's eval function is called, registers are updated to their new value, and any outputs that combinatorially depend on those new values should also immediately update. Similarly, operations such as asserts or debug trackers should observe new values for states immediately after they have been written. However, since all writes are considered deferred, there is no way for LowerState to produce a mixture of operations that depend on a register's old state (because they are used to compute a register's new state), and on a new state because they are combinatorially derived values.

This new implementation of LowerState completely avoids read-after-write conflicts. It does this by changing the way modules are lowered in two ways:

Phases: The pass tracks in which phase of the simulation lifecycle a value is needed and allows for operations to have different lowerings in different phases. An arc.state operation for example requires its inputs, enable, and reset to be computed based on the old value they had, i.e. the value the end of the previous call to the model's eval function. The clock however has to be computed based on the new value it has in the current call to eval. Therefore, the ops defining the inputs, enable, and reset are lowered in the old phase, while the ops defining the clock are lowered in the new phase. The arc.state op lowering will then write its new value to simulation storage.

This phase tracking allows registers to be used as the clock for other registers: since the clocks require new values, registers serving as clock to others are lowered first, such that the dependent registers can immediately react to the updated clock. It also allows for module outputs and side-effecting ops based on arc.states to be scheduled after the states have been updated, since they depend on the state's new value.

The pass also covers the situation where an operation depends on a module input and a state, and feeds into a module output as well as another state. In this situation that operation has to be lowered twice: once for the old phase to serve as input to the subsequent state, and once for the new phase to compute the new module output.

In addition to the old and new phases representing the previous and current call to eval, the pass also models an initial and final phase. These are used for seq.initial and llhd.final ops, and in order to compute the initial values for states. If an arc.state op has an initial value operand it is lowered in the initial phase. Similarly for the ops in llhd.final. The pass places all ops lowered in the initial and final phases into corresponding arc.initial and arc.final ops. At a later point we may want to generate the *_initial, *_eval, and *_final functions directly.

No more clock trees: The new implementation also no longer generates arc.clock_tree and arc.passthrough operations. These were a holdover from the early days of the Arc dialect, where no eval function would be generated. Instead, the user was required to directly call clock functions. This was never able to model clocks changing at the exact same moment, or having clocks derived from registers and other combinatorial operations. Since Arc has since switched to generating an eval function that can accurately interleave the effects of different clocks, grouping ops by clock tree is no longer needed. In fact, removing the clock tree ops allows for the pass to more efficiently interleave the operations from different clock domains.

The Rocket core in the circt/arc-tests repository still works with this new implementation of LowerState. In combination with the MergeIfs pass the performance stays the same. (Actually 10% speedup on my workstation.)

I have renamed the implementation and test files to make the git diffs easier to read. The names will be changed back in a follow-up commit.

Fixes #6390.

[uenoku]

Is this change required to run initialization?

[fabianschuiki]

Yeah. In its current form, a call to the initial function only executes seq.initial blocks and initializes the storage for states. It does not propagate those changes to outputs and side-effecting ops though. I wasn't sure if the initial function should also call eval once after initialization (potentially triggering first state updates and other things to run immediately), or if the user might want more fine grained control over when the first eval happens. WDYT?

[uenoku]

Yeah I see that makes sense. I agree that we want to avoid implicitly calling eval from initial

[maerhart]

Thanks a lot for implementing all this and sorry for my late review!

This makes a lot of sense to me because the state legalization has always been a pain point, is quite tricky to do properly for memories, and optimizations in between these two passes will probably do at least some degree of memory operation analysis anyway to avoid insertion of absurd amount of temporaries by state legalization.

I wonder if we can make this pass more modular in some way in the future such that arcilator can support new CIRCT core for frontend ops without having to modify this pass. I was thinking maybe a Op or dialect interface could work?

This also fixes #6390

[maerhart]

Unnecessary since it is now handled in getBitWidth?

[fabianschuiki]

Not in hw::getBitWidth though 😢

[maerhart]

Ah right, you're calling hw::getBitWidth. Why not StateType::getBitWidth though?

[fabianschuiki]

Because that is not available at this point 😢 (it takes the type from the this parameter instead of a function argument). I should just factor this out into a static helper.

[fabianschuiki]

Let me fix that post-merge.

[maerhart]

You're already skipping the TapOp here because it'd be too much work to add it, and we want to move to using the debug dialect anyway? Don't we rely on it when tracing is enabled in arc-tests?

[fabianschuiki]

The TapOp creates an AllocStateOp directly further down, where it also sets the name properly. This code up here is only for state-like ops that have to create an allocation when their def or a use is encountered for the first time.