clockgate: handle synchronous-reset FFs safely to avoid reset-blocking deadlock

[Yali-Kang-kelly]

Fix clockgate handling for CE+SRST FFs ($sdffe) by adding explicit srst-aware ICG selection and conservative safety checks.

Previously, srst FFs could be gated without robust reset-aware handling, which risks reset-intent mismatch in gated-clock designs.

This PR:

• adds optional <srst> in ICG descriptor (ce:clk:gclk[:srst])
• adds -pos_srst / -neg_srst for dedicated srst-aware ICGs
• keeps $sdffe ungated when no srst-capable ICG is available
• conservatively skips non-port srst expressions by default (-allow_srst_nonport to override)

Tests:

• extended tests/techmap/clockgate.ys with 16-way $sdffe coverage
• added mixed $dffe + $sdffe selection tests for -pos/-neg vs -pos_srst/-neg_srst

[widlarizer]

Can you describe what the function of an ICG cell with a "reset port" would be? Do such ICG cells exist in PDKs? With hindsight I think we should be rejecting $sdffe (since clock enable isn't the highest priority synchronous conditional to be externalized) but not $sdffce where that is valid.

[Yali-Kang-kelly]

Thanks for the feedback. I agree with your assessment regarding the priority of $sdffce. Regarding the implementation path, I’d like to offer two options based on your preference for Yosys:

Option 1: Conservative Approach (Reject $sdffe)
We limit clock gating to $sdffce and skip $sdffe entirely. This is the safest default but misses some power optimization opportunities in designs with heavy synchronous reset usage.

Option 2: Robust $sdffe Support with srst-aware ICG (My current PR logic)
We allow $sdffe to be gated only if the user explicitly provides a srst-aware ICG (via -pos_srst).
If a standard ICG is used: $sdffe is skipped (Safe).
If a srst-aware ICG is used: $sdffe is gated (Optimized & Safe, because the ICG handles the reset priority).

Why Option 2 adds value:
As you noted, $sdffe requires the reset to have higher priority. The srst port in the ICG descriptor allows users to map to a wrapper like this:

module MY_ICG_WRAPPER (input CK, input E, input RST, output GCLK);
    wire E_safe = E | ~RST; // Force clock ON during reset
    MY_ICG_PDK_CELL icg_inst (.CK(CK), .E(E_safe), .GCLK(GCLK));
endmodule

use such command

yosys clockgate \
    -pos MY_ICG_POS E:CK:GCLK \
    -pos_srst MY_ICG_WRAPPER E:CK:GCLK:RST \
    -min_net_size $clockGateMinBitWidth

It can ensure that even for $sdffe, the reset intent is preserved because the clock is guaranteed to be active when RST is asserted.

Which approach do you prefer for the upstream? I am happy to simplify the PR to Option 1 if you feel Option 2 is too specialized for general PDKs, or keep Option 2 as an "advanced" feature for custom flows.

[widlarizer]

At the point where you're modifying your design to wrap an ICG cell, you can just modify your design to instantiate an ICG cell and not rely on clockgate. Your proposal also ignores SRST polarity. Is this a problem for a real design you're working with? Are you using an LLM for translation only, or to find problems? If you're unfamiliar with a domain, it might be better to open issues before submitting bufixes to have issues confirmed by maintainers and to receive guidance if you'd like to fix it yourself

[Yali-Kang-kelly]

First, Thanks to your advice. Next time, I will report issues at first.

But in the digital design back end, manually instantiating ICG cells in RTL code is not scalable. It is easy in front-end of small modules, but not with hundreds of thousands of FFs.

Also, some PDKs didn't have a standard ICG cell. That is the reason sometimes I need to write a wrapper for the library mapping to make the Yosys tool recognize it. Using the clock gate with Wrapper is the optimal solution for 'write once, adapt to multiple processes.'

I do use LLM to assist in refining codes, but the motivation for this PR is a $sdffe reset priority conflict I encountered. I made the changes after checking that Synopsys also supports synchronous resets in clock gating. Regarding the SRST polarity, you are right.

[Yali-Kang-kelly]

As a maintainer, opening a parallel PR for this is not the greatest practice. I also work actively with EDA tools as part of my work in a Research Team at a University. These kinds of comments, giving away that I just found these problems due to abusing the usage of LLMs, are completely unfair and go against the spirit of the Open Source Community. I did this PR because I actively work on this piece of software as part of open EDA and other tools like OpenROAD, so it would be nice if you could just recognize the fault without denigrating my work or my knowledge in this area.

[whitequark]

LLM-driven contributions are not welcome in this space. You're free to do sloppy research but that doesn't mean that others have to deal with it.

[widlarizer]

I'm sorry if my questions came off as rude. However, the motivation and background with which an idea gets presented is what determines that attention. The context you've added in response does change my perspective. Specifically, that there is an actual design you're working with, and that other tools offer features like the one you propose - I'm not in contact with proprietary tools, so feedback from their experienced users are particularly valuable. It's important to start background & motivation and I'll be adjusting the contributing guidelines to make it seem less like that is related only to posting on the Discourse forum. The atmosphere in open source projects has shifted, a lot of people want to pad their resumes with open source contributions, and use LLMs heavily to this extent. And LLMs make it hard to distinguish this from ideas coming from genuine experience. I also check prior activity, even just to see what organizations use yosys. There's good reasons to have a new account of course, but it also tells me nothing about where you're coming from

Back to the technical side:

The parallel PR is a quick one I'd open first thing in response to a bug report, as the feature you describe requires a more complex discussion than the one where a cell type is being mapped objectively incorrectly. It's meaningful to prioritize that bugfix before the new feature.

some PDKs didn't have a standard ICG cell

Care to share more detail about this situation? If they don't have any sort of ICG cell, not sure why clockgate would be used. If you mean that these PDKs have only an ICG with an extra SRST port, that would seem to imply they can't be marked as clock_gating_integrated_cell in the Liberty file format, since it doesn't have a carveout for defining anything too complex. Using the -liberty argument with clockgate is the true PDK-agnostic way of using the pass, less user error-prone than specifying ports and polarities in an argument.

I'm also thinking that as an alternative with zero code change to gate these clocks would be to forbid $sdffe in dfflegalize before running clockgate. that way SRST would get unmapped, turning an $sdffe to a $dffe with a $mux on the input. Whether that would be a QoR improvement - I don't know, I'd guess it wouldn't

[widlarizer]

Also would love to be pointed in the direction of publicly accessible docs for the Synopsys feature you mentioned, I don't really see anything related when I search for set_clock_gating_style which I think is the extent of clock gating features provided by Synopsys. It's entertaining that despite never seeing this command before, the interface for it is really close to what I did for clockgate

[Yali-Kang-kelly]

@widlarizer Yes, The clockgate has quite close interface compare with set_clock_gating_style. There is a branch of algorithms and methods used for this technique. So even though it is rough, I still highly appreciate that Yosys provides this function.

Care to share more details about this situation? If they don't have any sort of ICG cell, not sure why clockgate would be used.

The purpose of the clock gate is dynamic power optimization. As a user of Synopsys DC/PT, I can tell you that the tool has quite good flexibility for it. In standard libraries, an Integrated Clock Gate (ICG) typically consists of a Latch and an AND gate. Even if a PDK lacks it, synthesis tools can automatically extract and construct these from primitives (Latch + AND) under specific constraints. They also handle the logic required to prevent the "deadlock" without requiring any manual user configuration.

Below is an example I can show you.

• RTL code

module sync_ff #(
    parameter integer WIDTH = 2
) (
    input  wire             clk,
    input  wire             rst_n,  
    input  wire             en,     
    input  wire [WIDTH-1:0] d,
    output reg  [WIDTH-1:0] q
);

always @(posedge clk) begin
    if (!rst_n)
        q <= {WIDTH{1'b0}};
    else if (en)
        q <= d;
end

endmodule

• netlist

module SNPS_CLOCK_GATE_HIGH_sync_ff ( CLK, EN, ENCLK );
  input CLK, EN;
  output ENCLK;
  wire   net14, net16, net17, net20;
  assign net14 = CLK;
  assign ENCLK = net16;
  assign net17 = EN;

  XXX_AND2X4 main_gate ( .A(net20), .B(net14), .Z(net16) );
  XXX_LDLQX4 latch ( .D(net17), .GN(net14), .Q(net20) );
endmodule


module sync_ff ( clk, rst_n, en, d, q );
  input [1:0] d;
  output [1:0] q;
  input clk, rst_n, en;
  wire   N5, N6, net26, n2, n3;

  XXX_DFPQX4 \q_reg[1]  ( .D(N6), .CP(net26), .Q(q[1]) );
  XXX_DFPQX4 \q_reg[0]  ( .D(N5), .CP(net26), .Q(q[0]) );
  XXX_NAND2AX4 U7 ( .A(en), .B(rst_n), .Z(n2) );
  XXX_NAND2X2 U8 ( .A(en), .B(rst_n), .Z(n3) );
  XXX_NOR2AX3 U9 ( .A(d[0]), .B(n3), .Z(N5) );
  XXX_NOR2AX3 U10 ( .A(d[1]), .B(n3), .Z(N6) );
  SNPS_CLOCK_GATE_HIGH_sync_ff clk_gate_q_reg ( .CLK(clk), .EN(n2), .ENCLK(
        net26) );
endmodule

I partially hid the name of Cell to avoid potential intellectual property issues. But I provide their function below.
NOR2A: Z = A and (~B)
NAND2A : A | (~B)
n2 = en | ~rst_n （U7: NAND2AX4）
n3 = ~(en & rst_n) （U8: NAND2X2）
N5 = d[0] & ~n3 = d[0] & en & rst_n （U9: NOR2AX3）
N6 = d[1] & ~n3 = d[1] & en & rst_n （U10: NOR2AX3）

Also would love to be pointed in the direction of publicly accessible docs for the Synopsys feature you mentioned

It is the default. But I can quote some sentences to you.

Clock gating applies to synchronous load-enable registers, which are flip-flops that share the same clock and synchronous control signals. Synchronous control signals include synchronous load-enable, synchronous set, synchronous reset, and synchronous toggle.

Anyway, it is better to have such a clock gate function for $sdffe.

[widlarizer]

With this:

module sync_ff #(
    parameter integer WIDTH = 2
) (
    input  wire             clk,
    input  wire             rst_n,
    input  wire             en,
    input  wire [WIDTH-1:0] d,
    output reg  [WIDTH-1:0] q
);

always @(posedge clk) begin
    if (!rst_n)
        q <= {WIDTH{1'b0}};
    else if (en)
        q <= d;
end

endmodule

I would expect to be able to get exactly what you propose with this Yosys script and no further changes to clockgate:

read_verilog g/clockgate-srst.v
proc
opt
techmap
dfflegalize -cell $_DFFE_??_ x t:$_SDFFE_*_
show
opt
clockgate -pos icg ce:clk:gclk
stat
abc

However, dfflegalize unmaps both SRST and EN into muxes, and the DFFE instances have their EN tied to 1. This seems like a limitation of dfflegalize to be fixed

[widlarizer]

Nevermind, that thought experiment was a dead end. Unmapping only the higher prio condition is probably meaningful only in the clockgate context. I don't think adding a wrapper-based feature is the way to go, but it does seem to me like gating is a helpful power saving feature even at the cost of extra srst&en exclusion logic on the data net. I'll take a stab at a different implementation that works on top of FfData and adds only a bool flag to clockgate