Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Yosys not removing DFF with constant input and replacing with a constant driver. #4266

Closed
arunkpv opened this issue Mar 7, 2024 · 10 comments
Labels
pending-verification This issue is pending verification and/or reproduction

Comments

@arunkpv
Copy link

arunkpv commented Mar 7, 2024

Version

Yosys 0.34+43 (git sha1 d21c464, gcc 9.4.0-1ubuntu1~20.04.2 -fPIC -Os)

On which OS did this happen?

Linux

Reproduction Steps

  • I have an rv32i cpu core design in verilog which I am trying to synthesize using Yosys targeting the sky130 library.
  • For ease of doing gate level simulation, I am currently hard-coding the instructions for the test program.
  • The synthesis commands used in Yosys is given below:
  • (Required files are attached at the end of the below section)
read_liberty -lib ./lib/sky130_fd_sc_hd__tt_025C_1v80.lib

read_verilog ./riscv_pipelined_Final.v

hierarchy -check -top riscv_core
synth -top riscv_core
stat

dfflibmap -liberty ./lib/sky130_fd_sc_hd__tt_025C_1v80.lib
opt
stat

abc -liberty ./lib/sky130_fd_sc_hd__tt_025C_1v80.lib -script +strash;scorr;ifraig;retime,{D};strash;dch,-f;map,-M,1,{D}
flatten
setundef -undriven -init -zero
opt_clean -purge
rename -enumerate
stat
write_verilog -noattr ./synth/riscv_pipelined_Final_netlist.v

Expected Behavior

  • DFF with constant input are reducible to a sequential constant, but it seems like Yosys is not removing some of the DFF with constant inputs in the design and replacing them with constant drivers.

Actual Behavior

  • While I was doing basic checks of the synthesis output netlist, I found that there are three DFF instances that have a constant input of 1'h0.
    DFF_with_constant_inputs

  • In the ABC result, I can see that there are three const0 cells (which I believe must correspond to the above three DFF instances
    ABC_Results_const0

  • I even tried adding another opt command (which should by default run opt_dff) after the abc command, but these 3 DFF instances with constant input are kept as is.

  • Please find attached the following:

  • For taking a quick look, attaching some of the files directly here:

@arunkpv arunkpv added the pending-verification This issue is pending verification and/or reproduction label Mar 7, 2024
@arunkpv
Copy link
Author

arunkpv commented Mar 7, 2024

  • Attaching the verilog model files of the sky130 library cells.

  • Snapshots of the verilog model of the DFF cell of concern below (sky130_fd_sc_hd__dfxtp_1) showing that these are simple DFF without any set/ reset inputs. Hence, I think Yosys shouldn't see any concerns in removing them and replacing with constant drivers.

sky130_fd_sc_hd__dfxtp_1 image

@arunkpv
Copy link
Author

arunkpv commented Mar 7, 2024

Hello Claire @clairexen ,
Could you kindly take a look and help figure out why this is happening and how it can be avoided ?

@KrystalDelusion
Copy link
Member

KrystalDelusion commented Mar 19, 2024

I could be wrong, but I think this is because opt_dff doesn't know that sky130_fd_sc_hd__dfxtp_1 is still a DFF and so doesn't optimise it, and it isn't until after the call to abc that the CPU_is_slti_a1 signal is optimised to 0 (and those 3 cells become driven by 1'h0).

Testing locally I had success with the following script:

read_liberty -lib ./lib/sky130_fd_sc_hd__tt_025C_1v80.lib

read_verilog ./riscv_pipelined_Final.v

hierarchy -check -top riscv_core
synth -top riscv_core
opt
stat

abc -script +strash;scorr;ifraig;retime,{D};strash;dch,-f;map,-M,1,{D}
flatten
setundef -undriven -init -zero
opt_clean -purge
opt
rename -enumerate
stat

dfflibmap -liberty ./lib/sky130_fd_sc_hd__tt_025C_1v80.lib
stat
write_verilog -noattr ./synth/riscv_pipelined_Final_netlist.v

Where abc runs on the yosys internal cells, allowing the call to opt afterwards to optimise out the constant-driven cells and then running dfflibmap at the end before output. Does that work for you?

@arunkpv
Copy link
Author

arunkpv commented Mar 20, 2024

Thanks @KrystalDelusion !
I will try this out and get back to you tomorrow confirming whether or not this fits for me.

@arunkpv
Copy link
Author

arunkpv commented Mar 25, 2024

Hi @KrystalDelusion ,

  • Using the explanation that you had provided, I settled on the below sequence of steps:
read_liberty -lib ./lib/sky130_fd_sc_hd__tt_025C_1v80.lib

read_verilog ./riscv_pipelined_Final.v

hierarchy -check -top riscv_core
synth -top riscv_core -flatten
opt
stat

abc
opt
opt_clean -purge
stat

dfflibmap -liberty ./lib/sky130_fd_sc_hd__tt_025C_1v80.lib

abc -liberty ./lib/sky130_fd_sc_hd__tt_025C_1v80.lib -script +strash;scorr;ifraig;retime,{D};strash;dch,-f;map,-M,1,{D}
setundef -undriven -init -zero
opt
opt_clean -purge
rename -enumerate
stat

write_verilog -noattr ./synth/riscv_pipelined_Final_netlist.v

PS:
Sorry, it took me a while to get back.
I had another VirtualMachine running on my laptop that I could not open this VM for any work.

@arunkpv
Copy link
Author

arunkpv commented Mar 26, 2024

@KrystalDelusion,

Could you please help with another query I have, that is not directly related to the original topic of this issue ?

  • Like I mentioned in the first comment, I am currently hard-coding the instructions for the test program as shown below:
    Issue_4266_unrelated_IMEM

  • Is there a way for me to tell Yosys to not optimize this section of the code in any manner ?
    Even if some of these bits are unused, I need all of them to remain as is after synthesis without any getting removed.

@KrystalDelusion
Copy link
Member

Using the explanation that you had provided, I settled on the below sequence of steps

I'll go ahead and close this issue then.

Is there way for me to tell Yosys to not optimize this section of the code in any manner ?

If you mark something with the "keep" attribute then it will prevent Yosys from optimising it out. E.g.

// The program in an instruction memory.
(* keep *)
wire [31:0] instrs [0:12-1];
assign instrs[0] = {7'b0000000, 5'd0, 5'd0, 3'b000, 5'd10, 7'b0110011};
assign instrs[1] = {7'b0000000, 5'd0, 5'd0, 3'b000, 5'd15, 7'b0110011};
assign instrs[2] = {7'b0000000, 5'd0, 5'd0, 3'b000, 5'd14, 7'b0110011};
assign instrs[3] = {12'b1010, 5'd0, 3'b000, 5'd12, 7'b0010011};
assign instrs[4] = {7'b0000000, 5'd0, 5'd0, 3'b000, 5'd13, 7'b0110011};
assign instrs[5] = {7'b0000000, 5'd14, 5'd13, 3'b000, 5'd14, 7'b0110011};
assign instrs[6] = {12'b1, 5'd13, 3'b000, 5'd13, 7'b0010011};
assign instrs[7] = {1'b1, 6'b111111, 5'd12, 5'd13, 3'b100, 4'b1100, 1'b1, 7'b1100011};
assign instrs[8] = {7'b0000000, 5'd0, 5'd14, 3'b000, 5'd10, 7'b0110011};
assign instrs[9] = {7'b0000000, 5'd10, 5'd0, 3'b010, 5'b00100, 7'b0100011};
assign instrs[10] = {12'b00100, 5'd0, 3'b010, 5'd15, 7'b0000011};
assign instrs[11] = {1'b1, 10'b1111101010, 1'b1, 8'b11111111, 5'd7, 7'b1101111};

results in the following block in the output verilog:

assign \instrs[0]  = 32'd1331;
assign \instrs[10]  = 32'd4204419;
assign \instrs[11]  = 32'd4250924015;
assign \instrs[1]  = 32'd1971;
assign \instrs[2]  = 32'd1843;
assign \instrs[3]  = 32'd10487315;
assign \instrs[4]  = 32'd1715;
assign \instrs[5]  = 32'd15107891;
assign \instrs[6]  = 32'd1476243;
assign \instrs[7]  = 32'd4274441443;
assign \instrs[8]  = 32'd460083;
assign \instrs[9]  = 32'd10494499;

@arunkpv
Copy link
Author

arunkpv commented Mar 26, 2024

Thanks @KrystalDelusion !
For the (* keep *) attribute, is there some other keyword to mark the end of the code section that is to be prevented from getting optimized out ?
Or, does it need to be provided for each statement ?

@nakengelhardt
Copy link
Member

nakengelhardt commented Mar 26, 2024

Attributes don't apply to code sections, they apply to specific design elements (modules, wires and cells). In this case if it is set on a signal declaration, it ensures that a signal of this name remains in the design and is driven with the correct value. But it can optimize away anything in the input cone of the signal that is not necessary to produce the value, e.g. if you say (* keep *) wire [31:0] foo = 3+5; it will not keep the adder, it will only keep a wire in the design driven directly by the constant 32'd8. If it finds another wire in the design that happens to have the same value it is also allowed to reconnect it to that wire instead.

@arunkpv
Copy link
Author

arunkpv commented Mar 26, 2024

Thanks @nakengelhardt !
Understood it clearly.

I was experimenting it out in Yosys after I posted the query and was able to get what I wanted by adding (* keep *) to some of the required signal declarations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending-verification This issue is pending verification and/or reproduction
Projects
None yet
Development

No branches or pull requests

3 participants