memory_libmap support for K6N10f

[rakeshm75]

1. SPRAM inferencing
2. DPRAM inferencing
3. Split support for SPRAM (placing 2 18K RAMs in a 36K block)
4. Asymmetric RAM support for SPRAM
5. Testcases to verify RAM inferencing, Split support & Asymmetric RAM inferencing

[tpagarani]

@nakengelhardt let's use this issue for further discussion on memory_libmap support

[nakengelhardt]

I had a look at the examples in https://github.com/QuickLogic-Corp/yosys-f4pga-plugins/tree/main/ql-qlf-plugin/tests/qlf_k6n10f, especially bram_tdp and bram_tdp_split, and I am confused by your definition of TDP mode. It seems that the input in bram_tdp.v describes a memory with two independent read ports and two independent write ports, so four ports total. But the output of the current synth_quicklogic implementation is mapped to a memory with two read-write ports, and it looks like that will write to the read address if rce and wce are enabled simultaneously. This seems incompatible with the input code to me, am I misunderstanding something?

[rakeshm75]

The basic TDP36K BRAM IP has only one addr port (no separate read address and write address):
.CLK_A1_i
.ADDR_A1_i
.WEN_A1_i
.BE_A1_i
.REN_A1_i
.WDATA_A1_i
.RDATA_A1_o

In the mapping the read address & write address are multiplexed to port address based on read or write:
(in brams_map.v file)
assign PORT_A_ADDR = A1EN ? A1ADDR_TOTAL : (B1EN ? B1ADDR_TOTAL : 15'd0);
assign PORT_B_ADDR = C1EN ? C1ADDR_TOTAL : (D1EN ? D1ADDR_TOTAL : 15'd0);

Yes, you are correct it would write to the read address when rce and wce are enabled simultaneously of the same port (A or B).

[nakengelhardt]

In that case, we won't be able to reuse the existing ql_bram_split pass, I'll need to write a replacement. Can you confirm the exact limitations on the allowed combinations of setting the 8 mode parameters:

RMODE_A1_i
WMODE_A1_i
RMODE_A2_i
WMODE_A2_i
RMODE_B1_i
WMODE_B1_i
RMODE_B2_i
WMODE_B2_i

Which ones need to always be equal and which are allowed to be set to different values?

[rakeshm75]

With 36K,
RMODE_A1_i = WMODE_A1_i = RMODE_A2_i = WMODE_A2_i
RMODE_B1_i = WMODE_B1_i = RMODE_B2_i = WMODE_B2_i
With18K ,
RMODE_A1_i = WMODE_A1_i
RMODE_B1_i = WMODE_B1_i
RMODE_A2_i = WMODE_A2_i
RMODE_B2_i = WMODE_B2_i

[nakengelhardt]

Thanks! In that case, should I also allow asymmetric ports in split mode, or will that be a problem for the downstream flow?

How should I deal with the attributes "rd_data_width" and "wr_data_width"? The names don't really make sense since the widths are per-port and each port can do read and write simultaneously. Are the timing differences only influenced by the port modes? Should it be something like port_a_width/port_b_width for 36k mode and port_a1_width/port_a2_width/port_b1_width/port_b2_width for split mode?

[rakeshm75]

Yes, we should allow asymmetric ports in split mode as well (18K bram).

For SPRAM, one of the port was tied as write port and other port as read port, that the reason the attributes were named "rd_data_width" & "wr_data_width". But as you said each port can do a read and write, it would be appropriate have the attributes named based on ports, as you mentioned above port_a_width/port_b_width for 36k mode and port_a1_width/ port_a2_width/port_b1_width/port_b2_width for split mode.

Only for the FIFO's there is a restriction that the write port (Push) will always be tied to Port A and the read port (pop) will always be tied to Port B, as the FIFO flags are tied to the Port A read data port. Anyways, inferencing is not supported for FIFO's only FIFO instantiation is supported.

Timing difference is influenced by port widths, split/nonsplit and fifo modes.

[rakeshm75]

One question, earlier you mentioned that we won't be able to reuse the existing ql_bram_split pass and you would write a replacement, so if a designer writes a dual port ram like the one in bram_tdp and bram_tdp_split (with separate read and write address each port) would it be inferred ?

[nakengelhardt]

No, I think those descriptions would require a primitive with at least 3 ports (2 write + 1 read) to map the semantics. An additional read port could be created with duplication, but there's no way to create more write ports than the primitive has, and if both are used to write then there is no port left over to read...

[rakeshm75]

Ok, understood.

[nakengelhardt]

I've gone through the existing tests and rewritten the modules that infer memory with the patterns that memory_libmap expects. However the manually instantiated modules still use the old code path (bram_map.v + bram_final_map.v) which now has some issues as a) they are not considered as candidates for merging two 18k blocks in split mode, and b) they still use the rd_data_width/wr_data_width attributes vs the port_a_width/port_b_width. How would you like to handle this? Are all of the modules like BRAM2x18_TDP considered user-facing primitives that should continue to be supported?

(FYI I've also had some simulation issues with the included sim tests where sdffsre cells would always output X, so I've had to rewrite the bram_tdp examples in a way that changes the behavior of the module to not infer FF cells, that's why it has those intermediate wires for ce now.)

[rakeshm75]

With regards to the manually instantiated modules, I will make the changes to use the port_a_width/port_b_width instead of rd_data_width/wr_data_width attributes. Also, I would look into using the libmap_brams_map.v or integrate manually instantiated modules into this file.

The user facing primitives would be the following:

1. RAM_36K_BLK
2. RAM_18K_BLK
3. DPRAM_36K_BLK
4. DPRAM_18K_BLK
5. AFIFO_36K_BLK
6. AFIFO_18K_BLK
7. SFIFO_36K_BLK
8. SFIFO_18K_BLK

[rakeshm75]

I have been running tests with latest PR (#2), few observations:

I see that the Address and the Write data inputs to the RAMs are registered. Why is it registered?

I have attached the design here with the synthesis output files.

spram_32x1024.zip

[nakengelhardt]

If you run memory_libmap with debug it will tell you: - emulate transparency with write port 0
The pattern in this memory is for read-first behavior but the TDP36K block has undefined collision behavior, so it will insert some extra logic to handle this. The pattern for undefined behavior that can map directly would be:

always @(posedge clk) 
begin
	if (wce)
		memory[wa] <= wd;
	if (rce)
		rq <= memory[ra];
	if (wce && wa == ra)
		rq <= 'x;
end

Alternatively you can add the no_rw_check attribute to suppress collision handling:
(* no_rw_check = 1 *) reg [DWIDTH-1:0] memory[0:(2**AWIDTH)-1];