proc_dff: bit-granularity optimizations and refactoring

[georgerennie]

proc_dff converts processes (sets of sync rules) into flip-flops through what is essentially structural pattern matching. As part of this it tries to make some optimizations to the sync rules so that it can produce simpler flip-flops (e.g. $adff and $dff instead of $aldff and $dffsr). These optimizations were previously applied in a fairly adhoc manner sprinkled throughout the inference which made it a pain to improve and have confidence in the correctness, as well as limiting the extent to which the optimizations could be applied. They were applied on full signals as found in the lhs of sync rules and thus could miss potential for optimization where different parts of a signal are best matched by different flip-flops.

Motivated by a pattern I saw with sv2v where a struct is lowered to one wire and may be partially reset even though the whole thing gets assigned at once, this example was being lowered to an $aldff, even though actually it is just the combination of a $dffe and an $adff in different parts of it.

module top(input wire clk, input wire rst, output reg [7:0] q, input wire [7:0] d);
always @(posedge clk or posedge rst) begin
	if (rst) q[3:0] <= '0;
	else     q <= d;
end
endmodule

This pr refactors proc_dff into three parts that are iterated on whilst there are still signals needing DFFs: extracting the relevant sync rules from the process, optimizing them and then generating flip-flop cells. The optimizations narrow the width of the signal that the flop is currently being generated for to the largest range of bits starting at the LSB that can have all the same optimizations applied as the LSB. This means that range is as optimized as it can be. The bits that are removed doing this are not deleted from the process and so are considered as a target in the next iteration. It is probably easiest to see the optimizations and choices of flip-flops by looking at the code which should be fairly well documented. For standard use-cases this should give basically the same results as before this change, it just allows supporting more corner cases.

This pr also fixes an issue in opt_dff where sigmap wasn't being used so it would fail to fold some muxes into enable signals. This caused test failures with the proc_dff changes. It also adds test cases for these new proc_dff optimizations.

To test this, it would be good to try running reasonable size verilog designs (ideally with async resets) through proc and checking the inferred flops are not a regression from previous Yosys. I believe Amaranth doesn't use sync processes so read_verilog and yosys-slang is probably the main interfaces affected by this.

[georgerennie]

As a side note, I think there are other bits of proc that could do with a bit of tidying up and being adapted to cover more general patterns. Maybe I'll have a look at proc_arst at somepoint...

[jix]

As far as I can tell, the only thing missing is some more testing with representative async reset verilog designs to rule out regressions. @georgerennie have you been using this PR or done any more testing using it in the meantime?

[georgerennie]

I have been using a build of yosys with this with a number of test designs (quite a few converted from SV2V which is what this was meant to address). I didn't find issues with it yet, can add some more tests and I'll also have a look over the code again - it's been long enough since I wrote it that I think I am almost approaching it with the eyes of a fresh reviewer!

[georgerennie]

There's a mild performance degradation (a few percent for just this pass maybe) - I think perf can be improved by precalculating the set of disjoint lvalues rather than going through and calculating them for each dff which requires a lot of sorting the sync rule lvalues. I am working on a patch for that but its probably easier to put it in a different pr.

[povik]

I believe Amaranth doesn't use sync processes so read_verilog and yosys-slang are probably the main interfaces affected by this.

Let me leave a note yosys-slang has retired the use of sync processes.

[jix]

I am working on a patch for that but its probably easier to put it in a different pr.

Does that mean that what you just pushed to this PR is ready to be merged? Or are you still in the process of reviewing it and adding tests? (It's fine if you need more time, I just want to avoid this PR sitting around for months again in case it actually is ready.)

[georgerennie]

Does that mean that what you just pushed to this PR is ready to be merged? Or are you still in the process of reviewing it and adding tests? (It's fine if you need more time, I just want to avoid this PR sitting around for months again in case it actually is ready.)

In the process of trying to improvev that I've found what I think is a better way of approaching the whole optimizations anyway that happens to improve proc_dff speed 4-7x on the designs I ran it on - therefore I think actually I will put it in this PR but there is no point reviewing until I do so. Will mark it draft until I push that to make this clear.