first FPGA with merged TPARs

[jasonfan393]

PR linked with firmware_hls (338). contains changes to the interface of the first FPGA project in a split configuration.

Draft for now to work out merge conflicts in firmware_hls and cleanup.

[jasonfan393]

Working on adding split tag everywhere to preserve non dual-fpga chains

[aehart]

@jasonfan393 Would it be possible to remove this signal by setting the corresponding address ports to open?

Suggested change

	if (interface != -1 and not extraports) or (split == 1 and "TPAR" in mem):
	if "TPAR" in mem:
	wirelist += " signal "+mem+"_dummy : std_logic_vector(1 downto 0);\n"
	if (interface != -1 and not extraports):

[aehart]

@jasonfan393 Would it be possible to eliminate the hard-coded value of addrwidth, e.g., by concatenating the inputs to addr_arr?:

Suggested change

	addrwidth = 10
	ramwidth = memInfo.bitwidth
	numpages = 8
	seed = mem.split("_")[1][:-1]
	iTC = mem.split("_")[1][-1]
	for PCGroup in MPARdict[seed]:
	if iTC == PCGroup[0]:
	numInputs = len(PCGroup)
	merge_parameterlist += " RAM_WIDTH => "+str(ramwidth)+",\n"
	merge_parameterlist += " NUM_PAGES => "+str(numpages)+",\n"
	merge_parameterlist += " NUM_INPUTS => "+str(numInputs)+",\n"
	merge_parameterlist += " NUM_EXTRA_BITS => 2,\n"
	merge_portlist += " bx_in => TP_bx_out,\n"
	merge_portlist += " rst => '0',\n"
	merge_portlist += " clk => clk,\n"
	merge_portlist += " enb_arr => open,\n"
	merge_portlist += " bx_out => open,\n"
	merge_portlist += " merged_dout => MPAR_"+seed+PCGroup+"_stream_V_dout,\n"
	for i in range(4): merge_portlist += " din"+str(i)+"=>TPAR_"+seed+PCGroup[i%numInputs]+"_V_dout,\n"
	for i in range(4): merge_portlist += " nent"+str(i)+"=>TPAR_"+seed+PCGroup[i%numInputs]+"_AV_dout_nent,\n"
	for i in range(numInputs): merge_portlist += " addr_arr("+str(((i+1)*addrwidth)-1)+" downto "+ str(i*addrwidth)+ ")=>TPAR_"+seed+PCGroup[i%numInputs]+"_V_readaddr,\n"
	ramwidth = memInfo.bitwidth
	numpages = 8
	seed = mem.split("_")[1][:-1]
	iTC = mem.split("_")[1][-1]
	for PCGroup in MPARdict[seed]:
	if iTC == PCGroup[0]:
	numInputs = len(PCGroup)
	merge_parameterlist += " RAM_WIDTH => "+str(ramwidth)+",\n"
	merge_parameterlist += " NUM_PAGES => "+str(numpages)+",\n"
	merge_parameterlist += " NUM_INPUTS => "+str(numInputs)+",\n"
	merge_parameterlist += " NUM_EXTRA_BITS => 2,\n"
	merge_portlist += " bx_in => TP_bx_out,\n"
	merge_portlist += " rst => '0',\n"
	merge_portlist += " clk => clk,\n"
	merge_portlist += " enb_arr => open,\n"
	merge_portlist += " bx_out => open,\n"
	merge_portlist += " merged_dout => MPAR_"+seed+PCGroup+"_stream_V_dout,\n"
	for i in range(4): merge_portlist += " din"+str(i)+"=>TPAR_"+seed+PCGroup[i%numInputs]+"_V_dout,\n"
	for i in range(4): merge_portlist += " nent"+str(i)+"=>TPAR_"+seed+PCGroup[i%numInputs]+"_AV_dout_nent,\n"
	merge_portlist += " addr_arr=>(TPAR_"+seed+PCGroup[-1]+"_V_readaddr"
	for i in reversed(range(numInputs-1)): merge_portlist += ", TPAR_"+seed+PCGroup[i]+"_V_readaddr"
	merge_portlist += "),\n"

[aehart]

@jasonfan393 Do we even need to specify the range of the addr_arr port here, since there is only one input?:

Suggested change

	addrwidth = 10
	ramwidth = memInfo.bitwidth
	numpages = 8
	numInputs = 1
	merge_parameterlist += " RAM_WIDTH => "+str(ramwidth)+",\n"
	merge_parameterlist += " NUM_PAGES => "+str(numpages)+",\n"
	merge_parameterlist += " NUM_INPUTS => "+str(numInputs)+",\n"
	merge_parameterlist += " NUM_EXTRA_BITS => 0,\n"
	merge_portlist += " bx_in => TP_bx_out,\n"
	merge_portlist += " rst => '0',\n"
	merge_portlist += " clk => clk,\n"
	merge_portlist += " enb_arr => open,\n"
	merge_portlist += " bx_out => open,\n"
	merge_portlist += " merged_dout => "+mem+"_stream_V_dout,\n"
	for i in range(4): merge_portlist += " din"+str(i)+"=>" +mem+"_V_dout,\n"
	for i in range(4): merge_portlist += " nent"+str(i)+"=>" +mem+"_AV_dout_nent,\n"
	for i in range(numInputs): merge_portlist += " addr_arr("+str(((i+1)*addrwidth)-1)+" downto "+ str(i*addrwidth)+ ")=>"+mem+"_V_readaddr,\n"
	ramwidth = memInfo.bitwidth
	numpages = 8
	numInputs = 1
	merge_parameterlist += " RAM_WIDTH => "+str(ramwidth)+",\n"
	merge_parameterlist += " NUM_PAGES => "+str(numpages)+",\n"
	merge_parameterlist += " NUM_INPUTS => "+str(numInputs)+",\n"
	merge_parameterlist += " NUM_EXTRA_BITS => 0,\n"
	merge_portlist += " bx_in => TP_bx_out,\n"
	merge_portlist += " rst => '0',\n"
	merge_portlist += " clk => clk,\n"
	merge_portlist += " enb_arr => open,\n"
	merge_portlist += " bx_out => open,\n"
	merge_portlist += " merged_dout => "+mem+"_stream_V_dout,\n"
	for i in range(4): merge_portlist += " din"+str(i)+"=>" +mem+"_V_dout,\n"
	for i in range(4): merge_portlist += " nent"+str(i)+"=>" +mem+"_AV_dout_nent,\n"
	for i in range(numInputs): merge_portlist += " addr_arr=>"+mem+"_V_readaddr,\n"

[aehart]

@jasonfan393 Could we remove the hard-coded bit widths by, e.g., concatenating the inputs to the address port?:

Suggested change

	string_mem_ports += " "+argname+"_dataarray_data_V_address0(9 downto 0) => "
	string_mem_ports += mem.mtype_short() + "_" + mem.var()+"_writeaddr,\n"
	string_mem_ports += " "+argname+"_dataarray_data_V_address0(11 downto 10) => "
	string_mem_ports += mem.mtype_short() + "_" + mem.var()+"_dummy,\n"
	string_mem_ports += " "+argname+"_dataarray_data_V_address0 => ("
	string_mem_ports += mem.mtype_short() + "_" + mem.var()+"_dummy, "
	string_mem_ports += mem.mtype_short() + "_" + mem.var()+"_writeaddr),\n"

[jasonfan393]

@aehart remaining comments are related to a band-aid fix that I made because of a change for FPGA2 in the TPAR address size from 10 -> 12 to support FPGA2. I believe vivado sim does not support concatenation at the port map, which I agree would make these hacks a lot simpler. I think ultimately we need to make some change to the TPAR HLS memory so that it is not expecting a 12 bit address on FPGA1.

I think I have some idea on how to fix this, but should we just merge this and I can fix the underlying issue with a later patch?

[aehart]

I believe vivado sim does not support concatenation at the port map, which I agree would make these hacks a lot simpler.

I do see lines like this in the generated SectorProcessor:

      outerVMStubs_binmask8_0_V_0     => (VMSTE_L2PHIAn1_AV_dout_mask(0)(56), VMSTE_L2PHIAn1_AV_dout_mask(0)(48), VMSTE_L2PHIAn1_AV_dout_mask(0)(40), VMSTE_L2PHIAn1_AV_dout_mask(0)(32), VMSTE_L2PHIAn1_AV_dout_mask(0)(24), VMSTE_L2PHIAn1_AV_dout_mask(0)(16), VMSTE_L2PHIAn1_AV_dout_mask(0)(8), VMSTE_L2PHIAn1_AV_dout_mask(0)(0)),

This is taken from the port map for TP_L1L2A. So whether or not the simulation supports it, we do seem to be using concatenation in the port map.

[aryd]

Vivado allows this only if the port is input and not output (IIRC). Hence there are hacks around this limitation. -Anders Anders Ryd ***@***.******@***.***> On Sep 10, 2024, at 2:25 PM, Andrew Hart ***@***.******@***.***>> wrote: I believe vivado sim does not support concatenation at the port map, which I agree would make these hacks a lot simpler. I do see lines like this in the generated SectorProcessor: outerVMStubs_binmask8_0_V_0 => (VMSTE_L2PHIAn1_AV_dout_mask(0)(56), VMSTE_L2PHIAn1_AV_dout_mask(0)(48), VMSTE_L2PHIAn1_AV_dout_mask(0)(40), VMSTE_L2PHIAn1_AV_dout_mask(0)(32), VMSTE_L2PHIAn1_AV_dout_mask(0)(24), VMSTE_L2PHIAn1_AV_dout_mask(0)(16), VMSTE_L2PHIAn1_AV_dout_mask(0)(8), VMSTE_L2PHIAn1_AV_dout_mask(0)(0)), This is taken from the port map for TP_L1L2A. So whether or not the simulation supports it, we do seem to be using concatenation in the port map. — Reply to this email directly, view it on GitHub<#60 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABI4LTIV4ARG6GSCCY5TWODZV3QM7AVCNFSM6AAAAABGPR5UU2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNBQGU3DSOJRGM>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[aehart]

Looks good, modulo some hacks that will be addressed separately. Approved!