Implicit Memory Ports Connection

[sdamghan]

Description

Odin II considered the same memory access in LHS and RHS as different ports with different addresses for hard block memories. In this PR, I resolved this issue by checking the source of new address signals to the connected one if there were any. Moreover, the memory type would have considered 'DUAL_PORT_MEM' even if there was only one port with the same address signals due to the previous design. This issue also has been resolved by considering the 'SINGLE_PORT_MEM' as the default memory type and changing it to the DUAL_PORT_MEM if there is another address signal that is not connected previously.

Related Issue

#1642

Motivation and Context

• Wrong signal connection
• Misinfering DPRAM instead of SPRAM

How Has This Been Tested?

1. make test (ODIN-II)
2. Basic Regression Tests
3. Strong Regression Test
4. Basic Valgrind Memory Tests (also, Valgrind memory tests for all other regression suits)
5. Sanitized Basic Regression Tests
6. Odin-II Micro Tests

Types of changes

• Bug fix (change which fixes an issue)
• New feature (change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My change requires a change to the documentation
• I have updated the documentation accordingly
• I have added tests to cover my changes
• All new and existing tests passed

[sdamghan]

Hi @aman26kbm ,
Would you please let me know if these changes work well with your benchmarks? This should be good now!

[aman26kbm]

Sure. Lemme try it out.

Thanks for working on this :)

[aman26kbm]

Hi @sdamghan ,

Single port ram inference worked well. But I am seeing an error for a dual port ram.

Here's the design code:

module dpram (
	clk,
	address_a,
	address_b,
	wren_a,
	wren_b,
	data_a,
	data_b,
	out_a,
	out_b
);

parameter AWIDTH=10;
parameter NUM_WORDS=1024;
parameter DWIDTH=32;

input clk;
input [(AWIDTH-1):0] address_a;
input [(AWIDTH-1):0] address_b;
input  wren_a;
input  wren_b;
input [(DWIDTH-1):0] data_a;
input [(DWIDTH-1):0] data_b;
output reg [(DWIDTH-1):0] out_a;
output reg [(DWIDTH-1):0] out_b;

reg [DWIDTH-1:0] ram[NUM_WORDS-1:0];

always @ (posedge clk) begin 
  if (wren_a) begin
      ram[address_a] <= data_a;
  end
  else begin
      out_a <= ram[address_a];
  end
end
  
always @ (posedge clk) begin 
  if (wren_b) begin
      ram[address_b] <= data_b;
  end 
  else begin
      out_b <= ram[address_b];
  end
end

endmodule

In the resulting netlist, there are multiple drivers for a net. Pasting a snippet below. out1 and out2 are connected to the same net.

  15 .subckt dual_port_ram addr1[0]=dpram^nMUX~1^MUX_2~112\
  16  addr1[1]=dpram^nMUX~1^MUX_2~113 addr1[2]=dpram^nMUX~1^MUX_2~124\
  17  addr1[3]=dpram^nMUX~1^MUX_2~135 addr1[4]=dpram^nMUX~1^MUX_2~146\
  18  addr1[5]=dpram^nMUX~1^MUX_2~150 addr1[6]=dpram^nMUX~1^MUX_2~151\
  19  addr1[7]=dpram^nMUX~1^MUX_2~152 addr1[8]=dpram^nMUX~1^MUX_2~153\
  20  addr1[9]=dpram^nMUX~1^MUX_2~154 addr1[10]=unconn addr1[11]=unconn\
  21  addr1[12]=unconn addr1[13]=unconn addr1[14]=unconn\
  22  data1=dpram^nMUX~1^MUX_2~114 we1=dpram^nMUX~1^MUX_2~149 clk=dpram^clk\
  23  addr2[0]=dpram^nMUX~3^MUX_2~56 addr2[1]=dpram^nMUX~3^MUX_2~57\
  24  addr2[2]=dpram^nMUX~3^MUX_2~58 addr2[3]=dpram^nMUX~3^MUX_2~59\
  25  addr2[4]=dpram^nMUX~3^MUX_2~60 addr2[5]=dpram^nMUX~3^MUX_2~61\
  26  addr2[6]=dpram^nMUX~3^MUX_2~62 addr2[7]=dpram^nMUX~3^MUX_2~63\
  27  addr2[8]=dpram^nMUX~3^MUX_2~64 addr2[9]=dpram^nMUX~3^MUX_2~65\
  28  addr2[10]=unconn addr2[11]=unconn addr2[12]=unconn addr2[13]=unconn\
  29  addr2[14]=unconn data2=dpram^nMUX~3^MUX_2~66 we2=dpram^nMUX~3^MUX_2~55\
  30  out1=^dpram.ram~0 out2=^dpram.ram~0

I am using the architecture: k6_frac_N10_frac_chain_depop50_mem32K_40nm.xml

[sdamghan]

Pasting a snippet below. out1 and out2 are connected to the same net.

Thats weird, let me check!

[sdamghan]

@aman26kbm That was a tricky point, now this should be okay! I have not indexed the out1 and ou2 properly so that both output signals would connect to the same net.
If there is no issue anymore with your benchmarks, please let me know to merge it into the main branch :)

[aman26kbm]

Cool. Trying it now. Will let you know in a bit.

[aman26kbm]

@sdamghan
I tested some cases of both single port and dual port rams. All of them worked as expected.

[sdamghan]

Thanks @aman26kbm

[sdamghan]

Hi @vaughnbetz ,
I have labelled Kokoro twice, but in both runs, the VtR Nightly Tests resulted in "Tool Failed." Would you know is there still any problem with Kokoro? Also, it seems I do not have access to the log file; it shows the following error when I click on the log link:

[email protected] does not have storage.objects.get access to the Google Cloud Storage object.

[vaughnbetz]

Not sure ... worth flagging this to @litghost

[vaughnbetz]

Sarah had a problem with Kokoro today; so seems like there may still be an issue. I think kokoro restarted with your latest push; please comment here on whether it finishes in a reasonable time (one day is good, two days may be OK, more than that definitely looks like a problem).

[mithro]

Sarah had a problem with Kokoro today; so seems like there may still be an issue. I think kokoro restarted with your latest push; please comment here on whether it finishes in a reasonable time (one day is good, two days may be OK, more than that definitely looks like a problem).

FYI - @litghost

[sdamghan]

@vaughnbetz It works the same as before; it resulted in "Tool Failed" for VtR Nightly Tests.

[vaughnbetz]

It would be good to run vtr_nightly on your own machine and confirm QoR is OK (or do a QoR run on the VTR benchmarks and paste the results here), since vtr_reg_nightly is not running automatically right now.

[sfkhalid]

Before you run vtr_reg_nightly locally, run the following command:
make get_symbiflow_benchmarks

[sdamghan]

Before you run vtr_reg_nightly locally, run the following command:
make get_symbiflow_benchmarks

Thanks, I ran the above-mentioned command, however, I still get the following error by running the following command:

> ./run_reg_test.py vtr_reg_nightly

==========================================================================
                  Verilog-to-Routing Regression Testing
==========================================================================
           Running vtr_reg_nightly
----------------------------------------------
scripts/run_vtr_task.py -l /home/casauser/Desktop/Most-Updated-VTR/sdamghan/vtr_flow/tasks/regression_tests/vtr_reg_nightly/task_list.txt -j 1 -script run_vtr_flow.py -short_task_names 

Error: Failed to resolve VTR source file directrf_stratixiv_arch_timing.blif
Error: Failed to resolve VTR source file directrf_stratixiv_arch_timing.blif

Test 'vtr_reg_nightly' had -1 qor test failures

Test 'vtr_reg_nightly' had -1 run failures

Error: -2 tests failed

Is the used command wrong or something else?

[sfkhalid]

Before you run vtr_reg_nightly locally, run the following command:
make get_symbiflow_benchmarks

Thanks, I ran the above-mentioned command, however, I still get the following error by running the following command:

> ./run_reg_test.py vtr_reg_nightly

==========================================================================
                  Verilog-to-Routing Regression Testing
==========================================================================
           Running vtr_reg_nightly
----------------------------------------------
scripts/run_vtr_task.py -l /home/casauser/Desktop/Most-Updated-VTR/sdamghan/vtr_flow/tasks/regression_tests/vtr_reg_nightly/task_list.txt -j 1 -script run_vtr_flow.py -short_task_names 

Error: Failed to resolve VTR source file directrf_stratixiv_arch_timing.blif
Error: Failed to resolve VTR source file directrf_stratixiv_arch_timing.blif

Test 'vtr_reg_nightly' had -1 qor test failures

Test 'vtr_reg_nightly' had -1 run failures

Error: -2 tests failed

Is the used command wrong or something else?

Did you also run "make get_titan_benchmarks" and "make get_ispd_benchmarks"? You also need to run these before running nightly

[sdamghan]

Thanks @sfkhalid
@vaughnbetz I have tried to run vtr_nightly_tests on my machine; however, after an hour, it crashed. By the way, all achieved results were OK until the crash time. I have also done the QoR test, and the results are as follows:

baseline_results.txt
implicit_ram_connections_results.txt
comparison.xlsx

It worth noting that RAM inference has considerably changed compared to the past since not only actual DPRAMs were not inferred, but also SPRAMs were considered as DPRAMs if there were both read and write accesses to that block of memory in the Verilog file. This could be a reasonable cause for considerable change in abc_synth_time.

[vaughnbetz]

Thanks, the QoR results look OK. I just updated the branch; will merge when the fast tests pass so we can get this in.

[sdamghan]

Thanks @vaughnbetz .
It seems another update is required. I will update the branch and squash commits all into one and then it would be mergeable.