Optimize out self-assignment from DFF reset patterns

[georgerennie]

Consider the below where a DFF is unassigned at reset only under some parameters. yosys-slang currently converts this to an $aldff with an asynchronous load of q, which introduces an error in check as there is a comb loop from Q to AD. Yosys proc_dff optimizes out asynchronous self assignment conditions from DFFs so creates a $dff here, and I think yosys-slang also does this on some other patterns (e.g. if q is unconditionally assigned itself in the reset branch) but it would be good to be able to handle these more general patterns

module top #(
	parameter bit RST_Q = 0
) (
	input logic clk,
	input logic rst,
	input logic d,
	output logic q
);

always_ff @(posedge clk or posedge rst) begin
	if (rst) begin
		if (RST_Q)
			q <= '0;
	end else begin
		q <= d;
	end
end

endmodule

[povik]

Do you agree your example should be handled equivalent to what's below?

always_ff @(posedge clk) begin
	if (!rst) begin
		q <= d;
	end
end

[georgerennie]

Yeah exactly, as the rst branch can be considered to be q <= RST_Q ? '0 : q which becomes q <= q for known RST_Q and the flop is not actually synchronous to rst. This is still to some extent a bad coding pattern in the RTL as it implies an enable based on !rst, but this is still better than an $aldff.

[povik]

I think yosys-slang already infers flops with enable from the pattern

always_ff @(posedge clk or posedge rst) begin
	if (rst) begin
	end else begin
		q <= d;
	end
end

but when processing your example the if (RST_Q) branch isn't eliminated early enough, so one gets a loopback async load instead. I will see about making sure dead branches are eliminated earlier.

While we are here, do you know of other patterns from real-world designs which need improved handling?

[georgerennie]

Not off the top of my head, this came from playing with loading CVA6 earlier this week. Using the filelist from bender seemed to just work and the only issues I hit were this one and YosysHQ/yosys#4896.

The other pattern I know of like this is the ones in YosysHQ/yosys#4781 (which I need to come back to and tidy up but have been busy lately) where different bits of a signal infer different flop types. This was particularly an issue with how sv2v lowered structs though and I think is less likely to be an issue for yosys-slang.

[povik]

where different bits of a signal infer different flop types.

You will probably not be reading sv2v output with yosys-slang but FWIW the inference it performs should be independent per each bit of a signal.