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?

multiple-attachments.zip34 KB

31 Replies

Altera_Forum
Honored Contributor
10 years ago
A slightly quirky one, I agree. However, the following code is confusing Quartus:
IF ci = '0' THEN co <= aab; ELSIF ci = '1' THEN aob := axb XOR aab; -- the same as (a OR b) co <= aob; END IF;

Using 'ELSE' covers all other cases. By using 'ELSIF' Quartus believes there may be some other paths it must handle - even though there aren't in this case.

Change these lines to:
IF ci = '0' THEN co <= aab; ELSE aob := axb XOR aab; -- the same as (a OR b) co <= aob; END IF;

I recommend you always use an ELSE at the end of conditional statements that infer combinational logic.

Cheers,
Alex
Altera_Forum
Honored Contributor
10 years ago
thanks for advice.
Altera_Forum
Honored Contributor
10 years ago
An another comment on coding style - the conv_std_logic_vector and conv_integer functions come from the non-standard std_logic_unsigned and std_logic_arith packages. You shouldnt use them, as they are non-standard. You should use the numeric_std package instead. (and probably use appropriate types to avoid type conversion as much as possible - you can use non-std_logic_vector types in ports - like unsigned or signed type.).
Altera_Forum
Honored Contributor
10 years ago
thanks for advice
Altera_Forum
Honored Contributor
10 years ago
Ok. I know how to use UNSIGNED ports. I make an example:
LIBRARY IEEE; USE IEEE.std_logic_1164.ALL; USE IEEE.numeric_std.ALL; ENTITY full_adder_Nbit IS GENERIC ( N : POSITIVE := 4 ); PORT ( ci : IN STD_LOGIC := '0'; a : IN UNSIGNED (N-1 DOWNTO 0) := X"0"; b : IN UNSIGNED (N-1 DOWNTO 0) := X"0"; s : OUT UNSIGNED (N-1 DOWNTO 0) := X"0"; co : OUT STD_LOGIC := '0' ); END full_adder_Nbit; ARCHITECTURE b OF full_adder_Nbit IS BEGIN p0 : PROCESS(ci, a, b) VARIABLE result_v : UNSIGNED (0 TO N+1); BEGIN result_v := ('0' & a & '1') + ('0' & b & ci); co <= result_v(0); s <= result_v(1 TO N); END PROCESS p0; END b;

I have question1 : When i write initial value for UNSIGNED like ... := "0" without prepand symbol 'X' ModelSim unable to start simulation (Error loading design and point to line number for port a). Which symbols allowed at the beggining?
I think the code above can synthesis any combinatoral adder. I want some that work with clocking. question 2: why i reinvent adders, is there any class of ready function, where i can read about?
question 3: If i instatiate my entity I am not sure about the length of result_v variable which n will be substituted.
Thx in advance.
Altera_Forum
Honored Contributor
10 years ago
For your questions.
1. What you have specified is a default value if nothing is connected to it. But the value have assigned is only 4 bits long, so if N is anything other than 4, there will be an error.
2. With unsigned type, you have all the arithmetic functions, as you've already done.

r <= a + b;

But in your code, you have extended a by 1 bit correct, but then you also append '1' to the LSB. You need to write:

result_v := ('0' & a) + ('0' & b) + ('0' & ci); -- if you get error about ambiguous types, write unsigned'('0' & ci);

You also have an error in your code. result should be created as

variable result_v : unsigned(N downto 0);

and s should be sliced

s <= result_v(N-1 downto 0);
co <= result_v(N);

3. when you fix the direction errors, result_v will have a length of N+1.
Altera_Forum
Honored Contributor
10 years ago
I've changed as you wish. But I am confused what's wrong with my previous. I agree that no extra bit requried in variable , but the reverse order - i'm confused. I can only suggest that LSB is always at index 0 but I am not sure about it. If your advice is coming from code can be read clearly, i accept it.
ARCHITECTURE b OF full_adder_Nbit IS BEGIN p0 : PROCESS(ci, a, b) VARIABLE result_v : UNSIGNED (N DOWNTO 0); BEGIN result_v := ('0' & a ) + ('0' & b) + ('0' & ci); co <= result_v(N); s <= result_v(N-1 DOWNTO 0); END PROCESS p0; END b;

Altera_Forum

Honored Contributor

10 years ago

Ok. Another one example. If I make my adder synchronus. Will be my code suitable for real application?

LIBRARY IEEE;
USE IEEE.std_logic_1164.ALL;
USE IEEE.numeric_std.ALL;
ENTITY full_adder_Nbit IS
GENERIC (
	N : POSITIVE := 4
);
PORT (
	clk : IN STD_LOGIC := '0';
	ci : IN  STD_LOGIC := '0';
	a  : IN  UNSIGNED (N-1 DOWNTO 0) := X"0";
	b  : IN  UNSIGNED (N-1 DOWNTO 0) := X"0";
	s  : OUT UNSIGNED (N-1 DOWNTO 0) := X"0";
	co : OUT STD_LOGIC := '0'
);
END full_adder_Nbit;
ARCHITECTURE b OF full_adder_Nbit IS
	SIGNAL
		reg_a,
		reg_b,
		out_s : UNSIGNED (N-1 DOWNTO 0);
	SIGNAL
		reg_ci,
		out_co : STD_LOGIC;
BEGIN
	p0 : PROCESS (clk)
	BEGIN
		IF rising_edge(clk) THEN
			reg_a <= a;
			reg_b <= b;
			reg_ci <= ci;
		END IF;
	END PROCESS p0;
	p1 : PROCESS(reg_ci, reg_a, reg_b)
	VARIABLE
		result_v : UNSIGNED (N DOWNTO 0);
	BEGIN
		result_v := ('0' & reg_a) + ('0' & reg_b) + ('0' & reg_ci);
		out_co <= result_v(N);
		out_s  <= result_v(N-1 DOWNTO 0);
	END PROCESS p1;
	p2 : PROCESS (clk)
	BEGIN
		IF falling_edge(clk) THEN
			co <= out_co;
			s  <= out_s;
		END IF;
	ENd PROCESS p2;
END b;

I am very unsure does it really need to store output value in register and at the falling edge. I can drive it directly but what i get?! I'm also unsure about storing input values in registry/ Perhaps I need only do addtion when clock arrive. So as result I can write down FOUR different solutions:

1) no register at input and output, do result when clock transition from 0 to 1 arrive.

2) register at inputs and no register at output.

3) no register at input but at the output.

4) inputs and outputs have register. (as shown in code above).

question 1. What will be the best solution, how it depends on application?

question 2. Do I make transition to synchronus right?

Altera_Forum
Honored Contributor
10 years ago
in principle the addition (combinatorial only) will do the job.
registering inputs/outputs is a measure to achieve synchronous design and best fmax.
The tradition is to register outputs only (not inputs) per module since then inputs received are already registered.
Excluding very first paths at io which need to be registered
Altera_Forum
Honored Contributor
10 years ago
thank you, Kaz.
I think external inputs should lead to some state machine which do control for entire device or just for local module. if so i should develope a pulse generator with programmed delay and number of output cycle to provide internal clocks. But what type of state-machine to choose Melay or Moore?

Forum Discussion

Need advice about coding style

31 Replies

Recent Discussions

Board-Aware Flow autoring

Quartus Eda_Writer keeps crashing

Quartus/Signaltap complains about wrong version

Repeat of a Questa License Issue

Perpetual license Quartus II doesn't allow to build Cyclone V devices anymore.