Forum Discussion
Need advice about coding style
Hello I'm Alex and I am novice for VHDL coding. if my english awful pls forgive me.
I trying to understand VHDL PROCESS statement. I try with 1bit full adder with carry save function in Quartus2 v15. I write down code:LIBRARY IEEE;
USE IEEE.std_logic_1164.ALL;
-- truth-table of full-adder
-- with three inputs: ci{carry_in}, a{first in} and b{second in}
-- and two ouputs: s{sum} and co{carry_out}
-- ci a b | s co
-- -----------------
-- 0 0 0 | 0 0
-- 0 0 1 | 1 0
-- 0 1 0 | 1 0
-- 0 1 1 | 0 1
-- 1 0 0 | 1 0
-- 1 0 1 | 0 1
-- 1 1 0 | 0 1
-- 1 1 1 | 1 1
ENTITY full_add_1bit IS
PORT (
ci : IN STD_LOGIC;
a : IN STD_LOGIC;
b : IN STD_LOGIC;
s : OUT STD_LOGIC;
co : OUT STD_LOGIC
);
END full_add_1bit;
ARCHITECTURE b OF full_add_1bit IS
BEGIN
p0 : PROCESS(ci, a, b)
VARIABLE
aab,
aob,
axb,
sxc : STD_LOGIC;
BEGIN
aab := a AND b;
axb := a XOR b;
sxc := axb XOR ci;
s <= sxc;
IF ci = '0' THEN
co <= aab;
ELSIF ci = '1' THEN
aob := axb XOR aab; -- the same as (a OR b)
co <= aob;
END IF;
END PROCESS p0;
END b; but during compilation i recieves message --- Quote Start --- Warning (10631): VHDL Process Statement warning at full_add_1bit.vhd(30): inferring latch(es) for signal or variable "co", which holds its previous value in one or more paths through the process --- Quote End --- I follow the simple logic - let's compute new values then do all signal assignments and the question is question1 : does pressure on variables so need?! I rewrite architecture to new one, let's compiler do all work: ARCHITECTURE b1 OF full_add_1bit IS
BEGIN
p0 : PROCESS(ci, a, b)
VARIABLE
result_i : INTEGER;
VARIABLE
result_v : STD_LOGIC_VECTOR (0 TO 1);
BEGIN
result_i := CONV_INTEGER(a)+CONV_INTEGER(b)+CONV_INTEGER(ci);
result_v := CONV_STD_LOGIC_VECTOR (result_i, 2);
co <= result_v(0);
s <= result_v(1);
END PROCESS p0;
END b1; Finally i make schematic for this with 2mux1 blocks as for fun... in the attachments. never use schematic for establishing any equation - you waste your time. But result is the same with that scheme. question 2: how to find the way to good coding style?31 Replies
- Altera_Forum
Honored Contributor
I was going for a better answer but it took me 5 times to login successfully. This website is broken and ignored.
Your statement can't be generalised. but one thing sure can; do not generate clock from logic, never so as a beginner. state machine is just a methodology of design, old but could be useful to clear a disturbed mind. It is not essential and is just based on renaming of state on some registers outputs (state coding). I very rarely used state machine and when I do I use single process approach and don't want to know what is mealy or Moore who incidentally I believe were two PhD students having a drink at a pub then came with the idea. The difference between them is one clock delay between I think outputs of state machine and state transition. In short, what you want to do at algorithm level decides what you should do at implementation level. - Altera_Forum
Honored Contributor
Ok. I conclude - just describe your intetion as algorithm then compiler will decide about implemantion. By the way, looking at code for full_adder_nbit is there upper limit for N when the logic sysnthesis with plus operator will be worst than using build-in PARALLEL_ADD structure , the last i found in IP-catalog. It is also about coding style - using built-in functions and blocks rather than reinventing one's own.
- Altera_Forum
Honored Contributor
Let's go further.
I see that built-in function has pipelining, I want to use it in my code, Im' confused a little bit. Any idea are welcome. I wrote comments, is it good style for VHDL comments? What should be commented? The comments should be present in code for person who will be read it. I remove initial values. I believe it is useful in simulation only, correct me if it is falsehood.LIBRARY IEEE; USE IEEE.std_logic_1164.ALL; USE IEEE.numeric_std.ALL; -- addition of two operands with carry-save ENTITY full_adder_Nbit IS GENERIC ( N : POSITIVE := 4 -- default operands size ); PORT ( clk : IN STD_LOGIC; -- clock signal ci : IN STD_LOGIC; -- carry-in a : IN UNSIGNED (N-1 DOWNTO 0); -- augend b : IN UNSIGNED (N-1 DOWNTO 0); -- addend s : OUT UNSIGNED (N-1 DOWNTO 0); -- sum co : OUT STD_LOGIC -- carry-out ); END full_adder_Nbit; ARCHITECTURE b OF full_adder_Nbit IS SIGNAL out_s : UNSIGNED (N-1 DOWNTO 0); -- new value for sum SIGNAL out_co : STD_LOGIC; -- new value for carry-out BEGIN -- provide outputs with recently calculated values (from p1) p0 : PROCESS (clk) BEGIN IF rising_edge(clk) THEN co <= out_co; s <= out_s; END IF; END PROCESS p0; -- calculate new values that will be outputed (for p0) p1 : PROCESS (ci, a, b) VARIABLE result_v : UNSIGNED (N DOWNTO 0); -- intermediate result BEGIN result_v := ('0' & a) + ('0' & b) + ('0' & ci); out_co <= result_v(N); out_s <= result_v(N-1 DOWNTO 0); END PROCESS p1; END b; - Altera_Forum
Honored Contributor
Hi. Could anyone clarify me why i get multiplexor instead of register?
function regCreate( signal clk : std_logic; aclr : std_logic; data : unsigned) return unsigned is variable temp : unsigned(data'range); begin if aclr = '1' then temp := to_unsigned(0, data'length); elsif rising_edge(clk) then temp := data; end if; return temp; end regCreate; - Altera_Forum
Honored Contributor
--- Quote Start --- Hi. Could anyone clarify me why i get multiplexor instead of register? --- Quote End --- Because you tried to use the clock inside a function. Basically, dont do that. - Altera_Forum
Honored Contributor
Will it be the same if I will use "procedure". Or should i shift down to component instance?
Before I believe that semantics of sequential operators the same as in "process". Perhaps i misunderstand "using function to generate schematic". I belive that fucntion do computation and generate some combinatorial schematic, why it can not manage with synchronous?
with this function we get simple and-gate in schematic rather than big comparatorfunction isRegZero(reg : unsigned) return std_logic is variable temp : std_logic; begin temp := '1'; for i in reg'range loop temp := temp and (not reg(i)); end loop; return temp; end isRegZero; - Altera_Forum
Honored Contributor
Why not stick to the coding guidelines and use processes?
- Altera_Forum
Honored Contributor
Functions are not used for synchronous description because they have to be called from a sequential context (like a process). Functions also complete in 0 time and cannot have any time associated with them.
Procedures can have time within them, but I highly recommend you stick with processes. - Altera_Forum
Honored Contributor
It seems that rising_edge evaluated at compile time in function.
so i chage to procedure
It works. test codeprocedure regCreate( signal clk : IN std_logic; aclr : IN std_logic; data : IN unsigned; signal reg: OUT unsigned) is begin if aclr = '1' then reg <= to_unsigned(0, data'length); elsif rising_edge(clk) then reg <= data; end if; end regCreate; procedure regCreate( signal clk : IN std_logic; aclr, ce : IN std_logic; data : IN unsigned; signal reg: OUT unsigned) is begin if aclr = '1' then reg <= to_unsigned(0, data'length); elsif rising_edge(clk) then if ce = '1' then reg <= data; end if; end if; end regCreate;library IEEE; use IEEE.std_logic_1164.all; use IEEE.numeric_std.all; use work.utility_pkg.all; entity PulseGenerator is generic( REG_WIDTH : natural := 4 ); port( reset clk, wrn, wrk, start : IN std_logic; data : IN unsigned(REG_WIDTH-1 downto 0); z : OUT std_logic ); end PulseGenerator; architecture rtl1 of PulseGenerator is signal reg_N, reg_K : unsigned(data'range); begin regCreate(wrn, reset, data, reg_N); regCreate(clk, reset, wrk, data, reg_K); z <= isRegZero(reg_N) and isRegZero(reg_K); end rtl1; - Altera_Forum
Honored Contributor
thank you a lot of, Tricky. Your advices help me.
I understand why you insist on using "process". using procedure and function in such manner like i do, can be compared with old-macrofunction. We should stay in the middle.