Gate level verilog modules are horribly broken and unusable

[RTimothyEdwards]

If the intent was to follow the standard used by the sky130 PDK, then this is an epic fail. It needs correcting on multiple fronts.

For starters, the standard cell verilog modules make references to modules with a _func suffix but do not define such modules anywhere.

Unlike the sky130 PDK, the modules are not broken up into base modules (functional and behavioral) plus a single module for each gate strength variant that calls the base modules based on #ifdef blocks for FUNCTIONAL, BEHAVIORAL, and USE_POWER_PINS. The functional module is identical (and therefore redundant) for every gate strength. The functional modules are only unique between strength variants because the wires have been unnecessarily renamed to add the entire verilog module name as a prefix.

Each module does not have a surrounding #ifdef to prevent it from being defined multiple times if included multiple times.

There is an #ifdef FUNCTIONAL inside the behavioral verilog module (and not in the functional verilog module!).

[QuantamHD]

What are some concrete steps we could take to make the GLS modules more usable in order of importance?

[atorkmabrains]

@RTimothyEdwards I don't remember that we worked on the gate level verilog as a deliverable. We could work on this. Will take a look and get back to you.

[RTimothyEdwards]

@atorkmabrains : I added you because I wasn't sure if you were involved with the standard cell deliverables or not.

[mithro]

@mkkassem - Can you please look at this with high priority and figure out the pathway forward?

[atorkmabrains]

@RTimothyEdwards @mithro I'll take a look at that and get back to you in the next few hours.

[RTimothyEdwards]

@QuantamHD , @atorkmabrains : Ideally these should be done in (more or less) the same way as sky130, with a single verilog file for each strength variant and a single verilog file for the base module that is used by all strength variants. e.g., in sky130_fd_sc_hd, file "sky130_fd_sc_hd__and2_1.v" defines module "sky130_fd_sc_hd__and2_1" but includes file "sky130_fd_sc_hd__and2.v" which defines the base module "sky130_fd_sc_hd__and2". Then "sky130_fd_sc_hd__and2.v" includes the functional or behavioral with or without power pins based on ifdefs (FUNCTIONAL and USE_POWER_PINS), and also defines SKY130_FD_SC_HD__AND2_V so that it won't get processed more than once if it is included multiple times.

One thing that was done wrong in the sky130 libraries was that the specify blocks were not handled correctly. There is clearly some confusion caused by the terms "functional" and "behavioral". Normally the distinction in verilog is made between "structural" and "behavioral". Generally speaking, if a verilog design has standard cells, then it's not behavioral; the idea of standard cell verilog being called "behavioral" is a bit weird. The general idea here, both for Sky130 and GF180MCU, is that "FUNCTIONAL" means simulation without parasitics, and "not(FUNCTIONAL)" means fully annotated simulation with parasitics (implying that the standard cell has a "specify" section).

[RTimothyEdwards]

Ideally, all open PDK standard cell libraries (both GF and SkyWater) should have verilog files that look like the following (example using the 2-input AND gate):

and2.tgz

There is a single file called gf180mcu_fd_sc_mcu7t5v0__and2.v for the cell that defines the cell function and nothing else. There is one file per gate strength, e.g., gf180mcu_fd_sc_mcu7t5v0__and2_1.v that calls the base functional cell and declares a specify block for annotation. Dealing with functional/not functional and power pins/no power pins is done within those two files. Each file has an ifndef block that prevents it from being defined multiple times. (This could be made more concise if the specify block was defined inside the base cell, but I'm not sure if annotation works that way or not.)

[mithro]

@RTimothyEdwards - that sounds good to me!

[mithro]

I've previously tried to describe some of this in the doc at https://bit.ly/open-source-pdks-naming (specifically the Recommended Verilog Include Structure section).

[atorkmabrains]

@RTimothyEdwards and @mkkassem I reviewed all requests with regards to this work. I don't believe that was something requested from us to do. I'm just confirming that now. We could update those files. But I believe it could take some time from our end. Please confirm if you would like us to do that.

[urish]

I'm also affected by this - trying to run gate level simulation, but getting a bunch of errors about missing _func modules

[proppy]

@RTimothyEdwards looking at your example in #24 (comment) couldn't the "not(FUNCTIONAL)" specify block also be moved to the common base functional cell's verilog file (and2/gf180mcu_fd_sc_mcu7t5v0__and2.v), it currently looks like it is duplicated across all gate strength (and2/gf180mcu_fd_sc_mcu7t5v0__and2_{1,2,4}.v):

`ifndef FUNCTIONAL
    // spec_gates_begin

    // spec_gates_end

    specify

	// specify_block_begin

	// comb arc A1 --> Z
	(A1 => Z) = (1.0,1.0);

	// comb arc A2 --> Z
	(A2 => Z) = (1.0,1.0);

	// specify_block_end

    endspecify

`endif	// FUNCTIONAL

Additionally, maybe it would be more readable to have the USE_POWER_PINS conditional around the module definition instead? example:

`ifndef GF180MCU_FD_SC_MCU7T5V0__AND2_1_V
`define GF180MCU_FD_SC_MCU7T5V0__AND2_1_V

`include gf180mcu_fd_sc_mcu7t5v0__and2.v

`ifdef USE_POWER_PINS
module gf180mcu_fd_sc_mcu7t5v0__and2_1( A1, A2, Z, VDD, VSS );
inout VDD, VSS;
input A1, A2;
output Z;

    gf180mcu_fd_sc_mcu7t5v0__and2_func gf180mcu_fd_sc_mcu7t5v0__and2_inst(.A1(A1),.A2(A2),.Z(Z),.VDD(VDD),.VSS(VSS));

endmodule

`else // If not USE_POWER_PINS

module gf180mcu_fd_sc_mcu7t5v0__and2_1( A1, A2, Z );
input A1, A2;
output Z;

    gf180mcu_fd_sc_mcu7t5v0__and2_func gf180mcu_fd_sc_mcu7t5v0__and2_inst(.A1(A1),.A2(A2),.Z(Z));

endmodule
`endif // USE_POWER_PINS

`endif // GF180MCU_FD_SC_MCU7T5V0__AND2_1_V

[proppy]

Looking at the current structure of the skywater-pdk https://github.com/google/skywater-pdk-libs-sky130_fd_sc_hd/tree/main/cells/and2, it looks a bit different from the example provided in #24 (comment):

• https://github.com/google/skywater-pdk-libs-sky130_fd_sc_hd/blob/main/cells/and2/sky130_fd_sc_hd__and2.v is just a proxy with conditional include statements that depends on USE_POWER_PINS and FUNCTIONAL to resolve the right verilog file.
• https://github.com/google/skywater-pdk-libs-sky130_fd_sc_hd/blob/main/cells/and2/sky130_fd_sc_hd__and2.functional.v and https://github.com/google/skywater-pdk-libs-sky130_fd_sc_hd/blob/main/cells/and2/sky130_fd_sc_hd__and2.behavioral.v are (almost) identical and implement the same logic.
• https://github.com/google/skywater-pdk-libs-sky130_fd_sc_hd/blob/main/cells/and2/sky130_fd_sc_hd__and2.functional.pp.v and https://github.com/google/skywater-pdk-libs-sky130_fd_sc_hd/blob/main/cells/and2/sky130_fd_sc_hd__and2.behavioral.pp.v are identical.
• specify blocks are broken out in a separate file and don't seem to be included anywhere https://github.com/google/skywater-pdk-libs-sky130_fd_sc_hd/blob/main/cells/and2/sky130_fd_sc_hd__and2.specify.v
• gate strength specific variants https://github.com/google/skywater-pdk-libs-sky130_fd_sc_hd/blob/main/cells/and2/sky130_fd_sc_hd__and2_1.v include the base verilog file sky130_fd_sc_hd__and2.v and instanciate the module depending on USE_POWER_PINS.

So any changes done do the verilog file organization should also be ultimatly reflected there.

Looks like the current set verilog files were (originally?) generated using this script: google/skywater-pdk#61, if we were to agree on the DRY organization suggested by @RTimothyEdwards in #24 (comment) shouldn't it be done by adapting the script in order to re-generate the files rather than patching them manually?

[proppy]

Would #26 be an acceptable workaround while @RTimothyEdwards suggestions get implemented consistently across all repositories ? It only address the missing _func but should hopefully unblock gate level simulation (assuming open_pdks distribution merges all the files, as it seems to be the case here: https://github.com/RTimothyEdwards/open_pdks/blob/master/gf180mcu/Makefile.in#L810)

[RTimothyEdwards]

@proppy : Possibly the specify block can be done non-redundantly but it depends on how the tools implement the preprocessor, and without testing it extensively on the tools, I was unwilling to write it that way in my example.

[atorkmabrains]

@RTimothyEdwards Is that still true or we can close this issue?