Keep attribute not working as expected

[hpretl]

Version

Yosys 0.38 (git sha1 543faed, clang 15.0.0 -fPIC -Os)

On which OS did this happen?

macOS

Reproduction Steps

I don't understand why the following code does not work. yosys keeps optimizing the delay line away. The code of tdc.v is attached.

tdc.v.zip

Expected Behavior

Keep all the instances and wires with the (* keep *) attribute set.

Actual Behavior

The delay is removed, and this is the results:

2.25. Printing statistics.

=== tdc ===

   Number of wires:                  5
   Number of wire bits:             28
   Number of public wires:           5
   Number of public wire bits:      28
   Number of memories:               0
   Number of memory bits:            0
   Number of processes:              0
   Number of cells:                  3
     $_DFF_P_                        2
     $_NOT_                          1

[KrystalDelusion]

What commands are you calling in Yosys? Do you have a .ys file?

[nakengelhardt]

I swear there was an issue or discussion about this already, but I can't find it now... This is due to how opt_clean works, it will reconnect all the even wires as aliases to a single driver. Currently the only workaround I'm aware of is to directly instantiate some blackbox cells that yosys doesn't know the function of instead of using not in the loop.
I'll put it on the list of use cases to address in the eventual opt_clean redesign that we will hopefully get around to: #2165

[hpretl]

@KrystalDelusion The only thing I do is a read_verilog tdc.v followed by synth, but I think it is already optimized away when I do a stat right after the read_verilog.

[nakengelhardt]

No, it's still what you intended after running hierarchy -top tdc and proc. I added in a splitnets w:w_delay_sig to make the picture easier to follow:

If you run opt_clean at this point that's what does the damage:

As opt_clean is a logic optimization pass this makes sense: this representation is logically equivalent for well-formed synchronous netlists with a deterministic output, and much more efficient (has a shorter critical path). It's just that in this case the base assumption for the optimization passes is not upheld, and we don't have a way to detect that, nor do we have a practical way to opt out manually.

[hpretl]

Got it, thanks for the clarification! I think the solution from a user point is to use the keep attribute, signaling to Yosys that I want a certain construct in this way, no matter what :-)

[hpretl]

It's me again. I followed now your steps, and the damage is already done earlier. After a read_verilog tdc.v; hierarchy -top tdc; proc; stat you see that the inverter chain is still there, but there is only one DFF, as per Verilog intention I want as many DFF as NOT to sample the state of the delay line:

4. Printing statistics.

=== tdc ===

   Number of wires:                 15
   Number of wire bits:             45
   Number of public wires:           5
   Number of public wire bits:      28
   Number of memories:               0
   Number of memory bits:            0
   Number of processes:              0
   Number of cells:                  6
     $dff                            1
     $not                            5

After opt_clean, the situation is pretty much unchanged (concerning the number of DFF and NOT):

6. Printing statistics.

=== tdc ===

   Number of wires:                  5
   Number of wire bits:             28
   Number of public wires:           5
   Number of public wire bits:      28
   Number of memories:               0
   Number of memory bits:            0
   Number of processes:              0
   Number of cells:                  6
     $dff                            1
     $not                            5

And finally, after opt also the inverter chain is gone:

=== tdc ===

   Number of wires:                  5
   Number of wire bits:             28
   Number of public wires:           5
   Number of public wire bits:      28
   Number of memories:               0
   Number of memory bits:            0
   Number of processes:              0
   Number of cells:                  2
     $dff                            1
     $not                            1

[nakengelhardt]

$dff is a coarse-grain cell, it has as many bits as you declared in reg [N_DELAY-1:0] r_delay_store; (i.e. 8 unless a non-default parameter value is passed). If you want to count gate-level $_DFF_*_ cells you'd first have to run techmap to map them to gate-level cells. But you're right that the opt_expr call in proc already removes some of the inverters. Here's what I think you are looking for:
yosys -p 'read_verilog tdc.v; proc -noopt; splitnets w:w_delay_sig; simplemap; stat; show'

=== tdc ===

   Number of wires:                 24
   Number of wire bits:             45
   Number of public wires:          14
   Number of public wire bits:      28
   Number of memories:               0
   Number of memory bits:            0
   Number of processes:              0
   Number of cells:                 17
     $_DFF_P_                        8
     $_NOT_                          9

Does that look right?

[hpretl]

That is exactly what I am looking for! So, to achieve that result via keep you need to fix some things in Yosys, right?

[nakengelhardt]

We discussed this at the last dev JF, this goes beyond what (* keep *) is meant for. We discussed a (* dont_touch *) or similar, intensified version, for when you have inputs that you want to be structurally preserved exactly as given. But the likely implementation to achieve that would be to insert blackbox buffer cells on each wire in the design, so that yosys is unable to understand anything about it. So for your use case this is almost the same as you can achieve today without any yosys modifications by directly instantiating the target architecture NOT gate, or a placeholder gate:

(* blackbox *)
module bb_not(input A, output Y);
endmodule

module tdc #(parameter N_DELAY = 8) (
[...]
    genvar i;
    generate
        for (i=0; i<=N_DELAY; i=i+1) begin : delay_chain
            bb_not n(w_delay_sig[i], w_delay_sig[i+1]); 
        end
    endgenerate
[...]

With this code change, no (* keep *) is required anywhere and the output conserves the netlist structure of the input even through the full optimization of synth: yosys -p 'read_verilog tdc.v; synth -top tdc; splitnets w:w_delay_sig; chtype -set $_NOT_ t:\bb_not; stat; show'