Using process for two different tasks in VHDL

[arkoudinos]

I am trying to test a simple compare circuit. I want this comparator to initialize its ouptut on reset, because it is needed by another part of the circuit. I've came up writing something like this:

library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use ieee.std_logic_arith.all;
use ieee.std_logic_unsigned.all;

entity X16Comparator is

port (  
	  sad_cur, sad	: in std_logic_vector(15 downto 0);
	  pos_cur, pos	: in std_logic_vector(10 downto 0);
	  minSAD			: out std_logic_vector(15 downto 0);
	  minPOS			: out std_logic_vector(10 downto 0);
	  clk, rst : in std_logic
   );

end X16Comparator;

architecture Behavioral of X16Comparator is

signal aeqb, altb, agtb: std_logic;

begin

	aeqb <= '1' when (sad_cur = sad) else '0';
	altb <= '1' when (sad_cur < sad) else '0';
	agtb <= '1' when (sad_cur > sad) else '0';

	process(aeqb, altb, agtb, clk, rst)
	begin
	  if (rst = '1' AND rising_edge(clk)) then
	    minSAD <= (others => '1');
	    minPOS <= (others => '1');
	  end if;
	  
    	  if (aeqb = '1' or agtb = '1') then
	        minSAD <= sad;
		minPOS <= pos;
          else
		minSAD <= sad_cur;
		minPOS <= pos_cur;
	  end if;
		    
	end process;

end Behavioral;

It seems to work on simulation, however I'm concerned about the process I wrote. Could you please tell me if my approach is correct, and if not why?

 

[Dan Mills]

I don't see that the rst logic ever does anything (minSAD, MINpos is modified further down and only the last change to a signal matters), and don't think the clock serves any purpose with that sensitivity list either (Given the reset logic does nothing).

Why is this even a process, once the does nothing nature of the reset logic is recognised the rest is async logic?

if (sad_cur < sad) then  
                minSAD <= sad_cur;
                minPOS <= pos_cur;
else 
	        minSAD <= sad;
		minPOS <= pos;
end if;

Regards, Dan.

 

[dpaul]

Are you trying to build a clocked comparator?

This pseudo code might help you.

process(clock, rst, ip_signals, op_signals)
begin
if (rst = '1') then
-- Initialise the comp o/p signal (Set/Reset the comparator)
else
if (clock='1' and clock'event) then
-- Give your comparision condition here
-- and generate the desired o/p
end if;
end if;

end process;

 

[ads-ee]

Look at the template examples provided by both Altera and Xilinx in their respective tools for various types of flip-flops. Here is a summary of the various types of FFs.

-- async reset FF
process (clk)
begin
  if rst = '0' then
    -- reset code
  elsif rising_edge(clk) then
    -- FF d-input code
  end if;
end

-- sync reset FF
process (clk)
begin
  if rising_edge(clk) then
    if rst = '0' then
    -- reset code
    else
    -- FF d-input code
    end if;
  end if;
end

I've always used a self imposed rule that one should never assign the same signal in multiple if-end if; structures as it ends up only using the last set of assignments in many cases. If nothing else the code is ambiguous as to it's functionality. Making sure any signal is assigned only in a single if-end if; statement will ensure there is no ambiguity as to what will be assigned to the signal.

 

[vGoodtimes]

I agree, but have exceptions for default assignments at the top of the process (usually for non-clocked processes) and resets at the end.

-- sync reset FF
process (clk)
begin
  if rising_edge(clk) then
    -- FF d-input code
    if rst = '0' then
    -- reset code
    else
  end if;
end;

also works, and for FPGA design allows easier control over which signals get reset and which don't.

 

[ads-ee]

I agree, but have exceptions for default assignments at the top of the process (usually for non-clocked processes) and resets at the end.

-- sync reset FF
process (clk)
begin
  if rising_edge(clk) then
    -- FF d-input code
    if rst = '0' then
    -- reset code
    else
  end if;
end;

also works, and for FPGA design allows easier control over which signals get reset and which don't.

I hardly ever put different signals in the same if statement, and then only when they are all reset in the same way and have a close relationship with each other.

Nearly all of my "rules" that I've developed over the years all have to do with making it easier to maintain code for myself or others (when I'm no longer available). I've done a lot of product maintenance and code reuse (of poorly written code) over the years and have cursed out the authors of the such code more times than I can count. I've spoken with former co-workers and they pretty much all have stated that the code I write is easier to maintain than other people's code. Probably helps that I comment my code as to why the logic does what it does instead of just telling you what the process does (which you can determine by reading the process code). This is also why I usually only have a single signal in any given if statement so I can describe why the signal has the behavior it does.

 

[arkoudinos]

OK, first of all, thank you very much for your answers. They helped me a lot in clarifying some misunderstandings (newbie here ). Secondly, to be more precise, I'm trying to build a module that compares the value of a signal with a previous one. If the current value is smaller than the previous one, we keep it (among with its "pair" - POS signal). In order to implement this module, I've ended up using a register and the comparator.



However, some problems came up, because, there wasn't an initial value to compare with. Modelsim complained about making arithmetic calculations with 'U' values (or something like that). So I thought it would be a good idea to implement a "resettable" comparator in order to initialize the output signals on reset.

 

[vGoodtimes]

@arkoudinos: The issue is that you have the registers to store the values outside of this module. It looks like sad_cur and pos_cur are the current minimums stored in a register external to this module, with sad/pos being inputs. You need to reset the register in the higher level module. Later, you can evaluate if this needs to be a module, or if you can implement it elsewhere. For now, just reset the registers in the higher level modules. It is common to get warnings at the beginning of sim. Sometimes developers will just ignore anything that occurs before reset finishes.

@ads_ee: I'm always interested to hear objective views on coding style, good/bad practices, etc... I'm curious if you had seen the "reset at bottom" approach in the past, and if so why you would only place signals in the same if/else if they shared the same reset scheme (vs use the "reset at bottom" approach).

 

[ads-ee]

@ads_ee: I'm always interested to hear objective views on coding style, good/bad practices, etc... I'm curious if you had seen the "reset at bottom" approach in the past, and if so why you would only place signals in the same if/else if they shared the same reset scheme (vs use the "reset at bottom" approach).

Never seen anyone use this style of coding before, except perhaps some contracted code. They had all their software resetable registers in something similar. They also did all their clocking using wait statements, separated combinational and sequential code, and used camel case everywhere with a lot of 15-20 character names that might have 2-3 character differences between the names (very difficult to maintain code).

I actually thought the idea behind the register resets was clever, but the maintenance of this code was really poor, due to the distributed nature of the code, scrolling or using multiple windows to "see" the entire code for a specific signal is a pain (note: there were > 100 registers in the design ~500+ lines for the registers themselves and then the following reset code for those registers).

IMO if you can't see everything for how a signal is generated in a single 50 line page of text (that isn't some huge case statement) then you obviously didn't think about someone else having to look at your code and figure out what you were doing (especially as most people have incorrect, poor, obvious, or no comments in their code). I make a point of looking at some new unknown code and read a number of comments in the code and determine which of the above apply. If the comments are incorrect/poor/obvious then none is usually better so I typically just use a script to delete all the comments and fill in new useful comments into the code as I go.

This is also one of the reasons that I restrict any if-then-else-end if/if-begin-end-else-begin-end-end blocks to a single signal unless all the signals behave identically (e.g. a register broken up into individual names of the fields instead of a register name). This perhaps defeats my desire for less verbose code, but it makes maintenance very easy and everything is easily digested at a glance instead of checking if this or that signal is specified in that branch of the if or not to determine if the signal holds it's value or gets updated.

 

[vGoodtimes]

Interesting points. I'll admit that I often use a single if-else per signal/group of related signals. That is mainly because I often think about the elements I need to solve a problem and then describe them.

My main criticism about this method is that the result -- before adding comments -- can be the description of a bunch of "things". Without comments, it is left to the reader to figure out how they interact and what problem they actually solve. While having the logic for a single signal nearby is nice for determining how that signal is generated, I feel it can be more difficult to show how the signal relates to other signals -- the relation is no longer in that 50-line window.

I also don't like the possible repeated code or "almost repeated" code. The reset-at-end is nice because it reduces some code repetition.

Another fringe downside is that procedures/tasks become less useful. This isn't an issue if you only use them for simulation or the fsm skip-idle trick.



I'm also always interested in how coding styles are impacted by editors. I use Vim and many of my code style choices are influenced by this. What editor do you use, and do you think you would write code differently if your editor had features to remove these pain-points you describe?