Code Review Request..

Altera_Forum · ‎11-18-2011

Dear all,

I'm writing a VHDL code which is demuxing coefficients. When it switches from one to the other, other coefficients should be remained previous values.

My code is following but looking at the real output coefficients are changing when I switch from one to the others. Please review the code and giving me a comment is much appreciated.

Thanks,

----------------------------

library ieee;

use ieee.std_logic_1164.all;

entity demux_coef is

port(

clk: in std_logic;

--- Coefficient Input Set 1

R_y3mm1: in std_logic_vector(11 downto 0);

J_y3mm1: in std_logic_vector(11 downto 0);

R_y3pp1: in std_logic_vector(11 downto 0);

J_y3pp1: in std_logic_vector(11 downto 0);

R_y3mp1: in std_logic_vector(11 downto 0);

J_y3mp1: in std_logic_vector(11 downto 0);

R_y3pm1: in std_logic_vector(11 downto 0);

J_y3pm1: in std_logic_vector(11 downto 0);

R_Alpa1: in std_logic_vector(11 downto 0);

J_Alpa1: in std_logic_vector(11 downto 0);

--- Coefficient Input Set 2

NL_17_param:in std_logic_vector(11 downto 0);

NL_18_param:in std_logic_vector(11 downto 0);

NL_19_param:in std_logic_vector(11 downto 0);

NL_20_param:in std_logic_vector(11 downto 0);

NL_21_param:in std_logic_vector(11 downto 0);

NL_22_param:in std_logic_vector(11 downto 0);

NL_23_param:in std_logic_vector(11 downto 0);

NL_24_param:in std_logic_vector(11 downto 0);

---- Demux

sel_coef: in std_logic_vector (1 downto 0);

--- Coefficient Output 1

alpha_UU2: out std_logic_vector(11 downto 0);

beta_UU2: out std_logic_vector(11 downto 0);

alpha_UL2: out std_logic_vector(11 downto 0);

beta_UL2: out std_logic_vector(11 downto 0);

alpha_UU4: out std_logic_vector(11 downto 0);

beta_UU4: out std_logic_vector(11 downto 0);

alpha_UL4: out std_logic_vector(11 downto 0);

beta_UL4: out std_logic_vector(11 downto 0);

alpha_UU2L2: out std_logic_vector(11 downto 0);

beta_UU2L2: out std_logic_vector(11 downto 0);

--- Coefficient Output 2

alpha_LU2: out std_logic_vector(11 downto 0);

beta_LU2: out std_logic_vector(11 downto 0);

alpha_LL2: out std_logic_vector(11 downto 0);

beta_LL2: out std_logic_vector(11 downto 0);

alpha_LU4: out std_logic_vector(11 downto 0);

beta_LU4: out std_logic_vector(11 downto 0);

alpha_LL4: out std_logic_vector(11 downto 0);

beta_LL4: out std_logic_vector(11 downto 0);

alpha_LU2L2: out std_logic_vector(11 downto 0);

beta_LU2L2: out std_logic_vector(11 downto 0);

--- Coefficient Output 3

small_a_p5: out std_logic_vector(11 downto 0);

LARGE_A_p5: out std_logic_vector(11 downto 0);

small_a_p3: out std_logic_vector(11 downto 0);

LARGE_A_p3: out std_logic_vector(11 downto 0);

small_b_p3: out std_logic_vector(11 downto 0);

LARGE_B_p3: out std_logic_vector(11 downto 0);

small_c_p3: out std_logic_vector(11 downto 0);

LARGE_C_p3: out std_logic_vector(11 downto 0);

--- Coefficient Output 4

small_a_m5: out std_logic_vector(11 downto 0);

LARGE_A_m5: out std_logic_vector(11 downto 0);

small_a_m3: out std_logic_vector(11 downto 0);

LARGE_A_m3: out std_logic_vector(11 downto 0);

small_b_m3: out std_logic_vector(11 downto 0);

LARGE_B_m3: out std_logic_vector(11 downto 0);

small_c_m3: out std_logic_vector(11 downto 0);

LARGE_C_m3: out std_logic_vector(11 downto 0)

);

end demux_coef;

architecture rtl of demux_coef is

begin

process(clk)

begin

if (sel_coef = "00") then -- Coefficient Output 1

alpha_UU2 <= R_y3mm1;

beta_UU2 <= J_y3mm1;

alpha_UL2 <= R_y3pp1;

beta_UL2 <= J_y3pp1;

alpha_UU4 <= R_y3mp1;

beta_UU4 <= J_y3mp1;

alpha_UL4 <= R_y3pm1;

beta_UL4 <= R_y3pm1;

alpha_UU2L2 <= R_Alpa1;

beta_UU2L2 <= J_Alpa1;

elsif (sel_coef = "01") then -- Coefficient Output 2

alpha_LU2 <= R_y3mm1;

beta_LU2 <= J_y3mm1;

alpha_LL2 <= R_y3pp1;

beta_LL2 <= J_y3pp1;

alpha_LU4 <= R_y3mp1;

beta_LU4 <= J_y3mp1;

alpha_LL4 <= R_y3pm1;

beta_LL4 <= R_y3pm1;

alpha_LU2L2 <= R_Alpa1;

beta_LU2L2 <= J_Alpa1;

elsif (sel_coef = "10") then -- Coefficient Output 3

small_a_p5 <= NL_17_param;

LARGE_A_p5 <= NL_18_param;

small_a_p3 <= NL_19_param;

LARGE_A_p3 <= NL_20_param;

small_b_p3 <= NL_21_param;

LARGE_B_p3 <= NL_22_param;

small_c_p3 <= NL_23_param;

LARGE_C_p3 <= NL_24_param;

else -- Coefficient Output 4

small_a_m5 <= NL_17_param;

LARGE_A_m5 <= NL_18_param;

small_a_m3 <= NL_19_param;

LARGE_A_m3 <= NL_20_param;

small_b_m3 <= NL_21_param;

LARGE_B_m3 <= NL_22_param;

small_c_m3 <= NL_23_param;

LARGE_C_m3 <= NL_24_param;

end if;

end process;

end rtl;

-----------------------

Altera_Forum · ‎11-18-2011

you have created latches. Use clock edge assignment

Altera_Forum · ‎11-18-2011

My review

- use the clock properly

- learn how to use arrays (you'll have less crappy looking code)

- more comments.

Altera_Forum · ‎11-19-2011

My advice and keep in mind I ditched VHDL long ago for schematic design against my choice (but I'm never going back now that I know verilog....)

- Split register operations from combination logic

- Make sure all synchronous (register) operations are on a clock edge to avoid latches which are bad news in a FPGA

- In your case use 2D indexing since you are working with groups of signals at a time. There is no need to come up with a bunch of signal names for different conditions when the condition itself can select the functionality *.

All three of those points can be made of VHDL and verilog (and schematics in some ways) so as far as I'm concerned all three are good design practice. The first point I make I want to stress since I run into many so called "experienced designers" who don't follow that rule which makes their code a mysterious "who knows what comes out of the synthesis engine" mess......

* Note: This isn't always true when optimizing so take with a grain of salt.

Altera_Forum · ‎11-19-2011

Thank you all, I appreciate it!

Altera_Forum · ‎11-19-2011

To BadOmen:

--- Quote Start ---

My advice and keep in mind I ditched VHDL long ago for schematic design against my choice (but I'm never going back now that I know verilog....)

--- Quote End ---

I remember you warning agains starting a VHDL-Verilog war, but your comment could be the proverbial gauntlet ...

Altera_Forum · ‎11-19-2011

Just giving an indication that my VHDL is a bit rusty.... 9 years rusty to be exact.

Altera_Forum · ‎11-19-2011

--- Quote Start ---

Just giving an indication that my VHDL is a bit rusty.... 9 years rusty to be exact.

--- Quote End ---

Your VHDL may be rusty, but my copies of the standards indicate the language itself is only slightly 'rustier' :)

IEEE-STD-1076-2008 VHDL Language Reference Manual.pdf

26 January 2009

IEEE-STD-1800-2009 Verilog and SystemVerilog.pdf

11 December 2009