Forum Discussion
Coding style to minimize combinational path delay?
I'm tuning my logic to meet timing. I have a long continuous assignment statement that outputs a signal based on the current state in my FSM. The below implementation results in a long mux chain. Is there an alternative coding style that results in smaller path delay?
assign tx_tlp_dword_offset =
(curstate == HANDLE_BAR1_READ_1_STATE) ? 7'h4 :
(curstate == HANDLE_BAR1_READ_2_STATE) ? 7'h0 :
(curstate == HANDLE_BAR1_READ_3_STATE) ? 7'h1 :
(curstate == HANDLE_BAR1_READ_4_STATE) ? 7'h2 :
(curstate == HANDLE_BAR1_READ_5_STATE) ? 7'h5 :
(curstate == HANDLE_BAR1_READ_6_STATE) ? 7'h5 :
(curstate == H2D_DMA_INIT_MEMRD_DW0_STATE) ? 7'h0 :
(curstate == H2D_DMA_INIT_MEMRD_DW1_STATE) ? 7'h1 :
(curstate == H2D_DMA_INIT_MEMRD_DW2_STATE) ? 7'h2 :
(curstate == H2D_DMA_INIT_MEMRD_DW3_STATE) ? 7'h3 :
(curstate == H2D_DMA_SEND_MEMRD_TLP_STATE) ? 7'h3 :
(curstate == H2D_DMA_SEND_MEMRD_TLP2_STATE) ? 7'h3 :
(curstate == D2H_DMA_INIT_MEMWR_DW0_STATE) ? 7'h0 :
(curstate == D2H_DMA_INIT_MEMWR_DW1_STATE) ? 7'h1 :
(curstate == D2H_DMA_INIT_MEMWR_DW2_STATE) ? 7'h2 :
(curstate == D2H_DMA_INIT_MEMWR_PL_STATE) ? reg_tx_tlp_dword_offset :
(curstate == D2H_DMA_SEND_MEMWR_TLP_STATE) ? reg_tx_tlp_dword_offset :
(curstate == D2H_DMA_SEND_MEMWR_TLP2_STATE) ? reg_tx_tlp_dword_offset :
7'h0; I have attached the TimeQuest path information as well as the output from the RTL viewer.20 Replies
- Altera_Forum
Honored Contributor
In no vhdl expert, but maybe use single bit values for each of the 'curstate' values so that each line becomes:
which is simple logic. Dunno if that ?: gets optimised away (easy to do).(curstate & XXX ? 7'hx : 0) | - Altera_Forum
Honored Contributor
Use a case statement instead of "if-then-else". "if-then-else" generates a priority encoder and a case statement does not.
Hope that helps /Boris - Altera_Forum
Honored Contributor
Well cant comment exactly, but have a doubt.
DO you really need a priority encoder? Can you do it in a case statement or maybe a state machine? - Altera_Forum
Honored Contributor
I can't see the priority encoder point for the present code. As long as the IF conditions are mutually exclusive (of course they are!) there's no difference between the IF THEN ELSE chain and a case construct. Both can be expected to end up in the same gate level netlist. The actual effort depends however on the chosen state encoding.
- Altera_Forum
Honored Contributor
If you examine the TQ diagram you see that the code shown in post 1 is only half the data path and second that 66% of the data path is Interconnect delay.
The code shown only takes two hops, although it is a lot of typed text it is a rather simple 'mux' and the compiler can optimize heavily (given that the 'mux' inputs are mostly constants). The high ratio of interconnect makes me think your FPGA is getting full? Can I suggest to put a pipeline register between the resp. outputs of main_controller_inst and the input of tx_tlp_buffer_inst? - Altera_Forum
Honored Contributor
--- Quote Start --- I can't see the priority encoder point for the present code. As long as the IF conditions are mutually exclusive (of course they are!) there's no difference between the IF THEN ELSE chain and a case construct. Both can be expected to end up in the same gate level netlist. The actual effort depends however on the chosen state encoding. --- Quote End --- I think you are correct. I have in the past tried to convert this continuous assignment statement into a case construct and the resulting logic synthesised was very similar. No improvement in path delay could be achieved. - Altera_Forum
Honored Contributor
--- Quote Start --- In no vhdl expert, but maybe use single bit values for each of the 'curstate' values so that each line becomes:
which is simple logic. Dunno if that ?: gets optimised away (easy to do). --- Quote End --- QII normally optimizes the state machine encoding to use 1-hot (or rather one signal per state) but i'm not too fond of this state encoding because it is harder to interpret in my signaltap trace (easier to see a number when looking for a particular state). I may have to go that route to optimize things though.(curstate & XXX ? 7'hx : 0) | - Altera_Forum
Honored Contributor
--- Quote Start --- If you examine the TQ diagram you see that the code shown in post 1 is only half the data path and second that 66% of the data path is Interconnect delay. The code shown only takes two hops, although it is a lot of typed text it is a rather simple 'mux' and the compiler can optimize heavily (given that the 'mux' inputs are mostly constants). --- Quote End --- So, in other words, there's not much room for improvement by changing the statement? This seems to be in line with my earlier attempts to change into a case statement. --- Quote Start --- The high ratio of interconnect makes me think your FPGA is getting full? Can I suggest to put a pipeline register between the resp. outputs of main_controller_inst and the input of tx_tlp_buffer_inst? --- Quote End --- How can i find out whether the interconnects are getting crowded?. Only 18% of the total logic elements are used, accordingly to the compilation report. --- Quote Start --- Can I suggest to put a pipeline register between the resp. outputs of main_controller_inst and the input of tx_tlp_buffer_inst? --- Quote End --- This seems to be a good idea. I intended need to convert the receiving reg[] based 'memory' into a true M9K RAM-based memory anyways. The M9K-based RAM generated by the MegaWizard has all inputs registered so will pipeline the offending path. It seems now is a good time to do it. - Altera_Forum
Honored Contributor
I didn't bother opening your file but assuming those decodes are mutually exclusive then Quartus should hopefully remove the priority scheme for you. I would get into a habit of just using case statements when you don't need priority though since you are at the mercy of your synthesis engine if you just chain a bunch of ternary operators together.
That said even with the priority removed that's a wide mux so you should consider pipelining it if you need more speed. Luckily a lot of your inputs are hardcoded so those will result in more packing into the lookup table. Probably the best spot to pipeline it would be after all the contant inputs are muxed together so that you can provide a registered copy to the secondary mux that combines it with the non-constant inputs. - Altera_Forum
Honored Contributor
--- Quote Start --- I didn't bother opening your file but assuming those decodes are mutually exclusive then Quartus should hopefully remove the priority scheme for you. I would get into a habit of just using case statements when you don't need priority though since you are at the mercy of your synthesis engine if you just chain a bunch of ternary operators together.. --- Quote End --- Yes, I use the ternary operator out of habit. I find it somewhat more compact (code-wise) than a case statement. Also, i have done experiments with both ternary operators and case statements and there was little difference in path delay (which, of course, is what matters once the design is synthesised). I'll keep your suggestion i mind though. --- Quote Start --- That said even with the priority removed that's a wide mux so you should consider pipelining it if you need more speed. Luckily a lot of your inputs are hardcoded so those will result in more packing into the lookup table. Probably the best spot to pipeline it would be after all the contant inputs are muxed together so that you can provide a registered copy to the secondary mux that combines it with the non-constant inputs. --- Quote End --- Yes, I will change the design such that the 'tx_tlp_dword_offset' signal is registered by a downstream M9K block RAM. This will cut down on the path delay compared to the current reg[] design which adds a couple of ns due to the muxes to get the data into the reg array. Thanks.