kactus2 / kactus2dev

Kactus2 is a graphical EDA tool based on the IP-XACT standard.
https://research.tuni.fi/system-on-chip/tools/
GNU General Public License v2.0
191 stars 34 forks source link

[PERFECTIVE] Optimize the default value of the verilog input port #67

Closed vowstar closed 1 year ago

vowstar commented 1 year ago

The current Port default value forces decimal after writing out verilog and loses width and sign information. This is lax in some cases and may introduce some coding style lint warnings.

To solve this problem I made some improvements, which will carry the corresponding width information according to the port width in the default value, making the synthesizer happy.

E.g.:

Set default value 40'h4000000000 to port pad_cpu_apb_base.

image

Before:

// Generated verilog results
...
        .pad_cpu_apb_base    (274877906944),
        .pad_cpu_rst_b       (),
        .pad_cpu_rvba        (cpu0_pad_cpu_rvba),
        .pad_cpu_sys_cnt     (),
        .pad_plic_int_cfg    (),
        .pad_plic_int_vld    (),
        .pad_tdt_dm_core_unavail(0),
        .pad_yy_dft_clk_rst_b(1),
        .pad_yy_icg_scan_en  (0),
        .pad_yy_mbist_mode   (0),
        .pad_yy_scan_enable  (0),
        .pad_yy_scan_mode    (0),
        .pad_yy_scan_rst_b   (1),
...

After:

// Generated verilog results
...
        .axim_clk_en         (),
        .pad_cpu_apb_base    (40'h4000000000),
        .pad_cpu_rst_b       (),
        .pad_cpu_rvba        (cpu0_pad_cpu_rvba),
        .pad_cpu_sys_cnt     (),
        .pad_plic_int_cfg    (),
        .pad_plic_int_vld    (),
        .pad_tdt_dm_core_unavail(1'b0),
        .pad_yy_dft_clk_rst_b(1'b1),
        .pad_yy_icg_scan_en  (1'b0),
        .pad_yy_mbist_mode   (1'b0),
        .pad_yy_scan_enable  (1'b0),
        .pad_yy_scan_mode    (1'b0),
        .pad_yy_scan_rst_b   (1'b1),
...

This is easier to read and solves a lint problem.

If you want it to be in decimal, then write the verilog canonical number literals in decimal directly, such as 64'd314159265, and then you can keep the decimal.

If you want this to be a signed number, write signed verilog canonical number literals, such as 4'sd15, so that you can keep the signed number default value when outputting verilog.

teuhom commented 1 year ago

Merged to master with minor changes. Please check if this still functions in the desired way.

vowstar commented 1 year ago

Thanks.

I tested the default value with the latest code, but found a small change in behavior:

port default setting generated default expected default
pad_cpu_apb_base 40'h4000000000 40'h4000000000 40'h4000000000
pad_tdt_dm_core_unavail 1'b0 0 1'b0
        .axim_clk_en         (),
        .pad_cpu_apb_base    (40'h4000000000),
        .pad_cpu_rst_b       (),
        .pad_cpu_rvba        (cpu0_pad_cpu_rvba),
        .pad_cpu_sys_cnt     (),
        .pad_plic_int_cfg    (),
        .pad_plic_int_vld    (),
        .pad_tdt_dm_core_unavail(0),
        .pad_yy_dft_clk_rst_b(1),
        .pad_yy_icg_scan_en  (0),
        .pad_yy_mbist_mode   (0),
        .pad_yy_scan_enable  (0),
        .pad_yy_scan_mode    (0),
        .pad_yy_scan_rst_b   (1),

When the width is 0, it cannot be handled normally, and the width will be missing to introduce lint warning.

I did a more detailed test

port default setting generated default expected default
const_pi 64'd314159265 64'h12b9b0a1 64'd314159265
signed_test 4'sd15 4'h0 4'sd15

After rolling back to the previous commit, the results is:

port default setting generated default expected default
pad_cpu_apb_base 40'h4000000000 40'h4000000000 40'h4000000000
pad_tdt_dm_core_unavail 1'b0 1'b0 1'b0
port default setting generated default expected default
const_pi 64'd314159265 64'd314159265 64'd314159265
signed_test 4'sd15 4'sd15 4'sd15

And the case where the width is 0 can be handled correctly.