Hello
I just got asked this programming question in an interview:
Programming Test:
Design a VHDL/Verilog module to divide two unsigned std_logic_vector
numbers and floor the result. For example, if I provided you a vectors
of 0x9 and 0x2 as the dividend and divisor, the result would be 0x4.
The input vectors can be considered valid as soon as the 'start' input
is pulsed high and the result must be presented on the 'quotient' output
along with asserting 'done' for one clock cycle.
Important: The design must synthesize on either Xilinx or Altera tools
targeting a Spartan/Cyclone class FPGA with the most efficient resource
utilization and as few clk cycles as you are able to achieve. (Results
are evaluated first on proper operation and ability to synthesize to
hardware, second on optimization.)
The interface entity is provided below:
entity divider is
port
(
clk : in std_logic;
start : in std_logic;
dividend : in std_logic_vector(7 downto 0);
divisor : in std_logic_vector(7 downto 0);
done : out std_logic;
quotient : out std_logic_vector(7 downto 0)
);
end entity divider;
This was my code:
However, upon simulating I get a constant quotient output as 0. I
probably screwed this one up for the interview but I still want to learn
how to do this. Can someone direct me as to why my code isn't
functioning?
Hi Omar,
I'm not a VHDL guru, and I did not study your code, but 3 things
attracted my attention. Two of them are minor:
1. Omar Rashad wrote:> with the most efficient resource> utilization and as few clk cycles as you are able to achieve.
This is a contradictory requirement. You can achieve either a sequential
solution with the most efficient resource utilization or fully static
design with zero clk cycles.
2. Why do you so often assign std_logic_vectors to std_logic_vectors
with an additional type conversion?
The last one is more irritating:
You have the concurrent statement
Qtemp <= std_logic_vector(unsigned(divisor));
and later a sequential statement where you re-assign Qtemp:
Qtemp <= std_logic_vector(unsigned(Qtemp) + unsigned(divisor));
That is contradictory and should lead to am error. But it seems you get
no error?
Uwe
Omar Rashad wrote:> signal Qfinal, Qtemp, counter: std_logic_vector (7 downto 0)
Why not simply using unsigned vectors for calculations? Thats what the
numeric_std package is desingd for. You would have much less hazzle with
casting types from here to there and backwards...
> with the most efficient resource utilization and as few clk cycles as> you are able to achieve.
You can have either the one or the other. But usually a (slow) single
clock cycle divider is NOT efficient in a way to use less real silicon.
> when s1 => if (start = '0' or dividend'event or divisor'event) then
Huh, Ouch, Whow!
You simply invoked an additional clock signal here (the 'event!!!). That
will NEVER ever run on real hardware, but thats required: "The design
must synthesize on Xilinx or Altera"...
Have a look there (maybe Google translator or Babelfish will help):
Beitrag "Division in VHDL"http://www.lothar-miller.de/s9y/archives/29-Division-in-VHDL.html
And a funny gag: Altera will implement a combinatorial divider just by
using the '/' operator. See in the
Beitrag "Re: Rechnen mit unsigned vs. signed und einer division"
Lothar Miller wrote:> Omar Rashad wrote:>> signal Qfinal, Qtemp, counter: std_logic_vector (7 downto 0)> Why not simply using unsigned vectors for calculations? Thats what the> numeric_std package is desingd for. You would have much less hazzle with> casting types from here to there and backwards...
1. I don't exaty understand what you mean. Can you give me an example of
how you would write these assignments then? Also, the reason I use these
type conversions is that numeric_std requires that ou declare any vector
as signed or unsigned before operating on it. Isn't that correcT?
2. Thanks for the note on Qtemp. I understand part of the confusion now.
But I need to initialize Qtemp to the value of the divisor. How can I do
that using signals and/or variables?
>> with the most efficient resource utilization and as few clk cycles as>> you are able to achieve.> You can have either the one or the other. But usually a (slow) single> clock cycle divider is NOT efficient in a way to use less real silicon.>>> when s1 => if (start = '0' or dividend'event or divisor'event) then> Huh, Ouch, Whow!> You simply invoked an additional clock signal here (the 'event!!!). That> will NEVER ever run on real hardware, but thats required: "The design> must synthesize on Xilinx or Altera"...
Since my divider is sequential the input has to be maintained at its
value until the output is calculated. So if the input changes before
completion that should take us back to state s0. How else can I
implement that?
> Have a look there (maybe Google translator or Babelfish will help):> Beitrag "Division in VHDL"> http://www.lothar-miller.de/s9y/archives/29-Divisi...>> And a funny gag: Altera will implement a combinatorial divider just by> using the '/' operator. See in the> Beitrag "Re: Rechnen mit unsigned vs. signed und einer division"
After going through the code several times, I discovered some possible
sources of the problem and corrected them (but the output is still 0).
Anyway, I decided to keep them as they seemed to make sense.
1. I initialized only state to s0 and not n_state. I thought that might
have been keeping the state = s0 infinitely.
2. I replaced Qfinal with a flag signal called complete since the only
use of that signal was to really indicate completion of division.
3. I eliminated the last else conditional statements in the last process
in the second block of the case statement as it was redundant.
This is the new code
Omar Rashad wrote:> (but the output is still 0).
You know, that you can dig inside a module with the simulator?
Usually you have to open the corresponding node on the structure tree on
the left side. Then you can select signals from inside a module and drag
them to the waveform window. There you can see whats going on after
restarting the simulation.
regarding your latest code:
in S1 you have a combination of if-clauses, which can never be
fullfilled.
if (Qtemp < dividend) then
......
if (Qtemp > dividend) then
If Qtemp is bigger than dividend, you do not enter the outer if-clause.
If it is smaller than dividend, you do not enter the inner if-clause. So
the inner if-clause will never be entered. (I am aware, that you assign
a new value to Qtemp inbetween, but this new value will not be
considered during this run of the process, as Qtemp is a signal, not a
variable)
I would expect that you get lots of warnings for generating latches.
Don't do such stuff: use flip-flops (registers) to store your results
(Qtemp, Counter, ....). In other words: make assignments to variables
in clocked processes (in the moment you only do this for the signal
"state).
Your whole module is build to run only once. You end in s2 and never
leave it. You do not re-initialize your signals (Qtemp, counter, ....)
to correct starting values. Your done-flag will be kept high eternally
(not just for one clk cycle, as I understood the mission.
Omar Rashad wrote:> So if the input changes before> completion that should take us back to state s0. How else can I> implement that?
You could store your input data to internal signals, when you detected a
rising edge on start with your edge-detector. Then a single cycle with
start high and dividend and divisor valid will be sufficient to execute
the whole calculation.
Most probably you describe the FSM in the multi-process style you have
been taught. I personally prefer a single process style. And as this
single process will be clock-triggered you will avoid many problems with
latches.
Achim S. wrote:> In other words: make assignments to variables> in clocked processes (in the moment you only do this for the signal> "state).
Aaah, I didn't want to write that.
Don't make assignments to variables at all! (unless you really know what
you're doing). I meant:
make assignments to signals in clocked processes
Thanks for all the input. I have used most of your insight to make
changes in the code. It is still not working though and I used the
waveform view to analyze what is happening: counter is being incremented
to 1 (but quotiend is never assigned counter so it remains 0 for some
reason whil counter is 1). System stays in s1 indefinitely and I do not
know why...
that's a simulation problem, which Lattice User already mentioned. The
"third process" (it might be a good idea to give a name to processes)
has only "state" in its sensitivity list. So this process only is
evaluated by the simulator, when state changes (i.e. only once for state
s1).
The hardware would not care about this incorrect sensitivity list. But
in hardware you would still have worse problems, since you still don't
store results in clock-triggered registers.
Achim S. wrote:> regarding your latest code:>> in S1 you have a combination of if-clauses, which can never be> fullfilled.>> if (Qtemp < dividend) then> ......> if (Qtemp > dividend) then>> If Qtemp is bigger than dividend, you do not enter the outer if-clause.> If it is smaller than dividend, you do not enter the inner if-clause. So> the inner if-clause will never be entered. (I am aware, that you assign> a new value to Qtemp inbetween, but this new value will not be> considered during this run of the process, as Qtemp is a signal, not a> variable)
I have never heard of this before. This is a process and everything is
executed sequentially. Isn't that the case? Are you sure of this? I do
not mean to undermine your experience but I never heard of this before.
Updating the value of Qtemp in between doesn't register until the next
run? That could be the reason...
> I would expect that you get lots of warnings for generating latches.> Don't do such stuff: use flip-flops (registers) to store your results> (Qtemp, Counter, ....). In other words: make assignments to variables> in clocked processes (in the moment you only do this for the signal> "state).>> Your whole module is build to run only once. You end in s2 and never> leave it. You do not re-initialize your signals (Qtemp, counter, ....)> to correct starting values. Your done-flag will be kept high eternally> (not just for one clk cycle, as I understood the mission.
I fixed that in my most updated code. Please take a look at it :D
Achim S. wrote:> The "third process" (it might be a good idea to give a name to> processes) has only "state" in its sensitivity list. So this process> only is evaluated by the simulator, when state changes (i.e. only once> for state s1).
Lets keep things short: the sensitivity list is INCOMPLETE. Therefore
the simulation is simply ans definitely WRONG. It will NOT match to real
hardware behaviour.
Try that and you will see some magic changes:
1
process(state,counter,divisor,dividend,qtemp)
2
begin
Keep in mind: each signal that requires a process to be recalculated
must be in the sensitivity list. Or easier: each signal on the right
side of an assignment (<=) or used in a case or used with an if-elsif
must be in the sensitivity list.
Omar Rashad wrote:> This is a process and everything is executed sequentially. Isn't that> the case?
In real hardware the whole process (as the whole description) will be
implemented in parallel. Each state, each case, each if-elsif will be
present in parallel beside each other on the FPGA.
Omar Rashad wrote:>> If Qtemp is bigger than dividend, you do not enter the outer if-clause.>> If it is smaller than dividend, you do not enter the inner if-clause. So>> the inner if-clause will never be entered. (I am aware, that you assign>> a new value to Qtemp inbetween, but this new value will not be>> considered during this run of the process, as Qtemp is a signal, not a>> variable)> I have never heard of this before. This is a process and everything is> executed sequentially. Isn't that the case?
It is.
But signals are updated at the END of a process. They KEEP there
INITIAL value THROUGHOUT the process and so they do NOT take an assigned
value IMMEDIATELY!
The behave entirely different from variables!
Lets take this code snippet and lets assume, that Qtemp is 55 at the
start, and also that the divisor is 5 at the start. Then I write the
value of Qtemp in the front of the lines:
So remember: signals keep there initial values throughout the process.
They are updated at the end of the process with the last value assinged
to them.
What value will a,b,c and d have here after "running" threough the
process:
1
signala:integer:=1;
2
signalb:integer:=2;
3
signalc:integer:=3;
4
:
5
process(a,b,c)
6
begin
7
a<=b+c;
8
c<=c+c;
9
b<=a+c;
10
a<=a-b;
11
b<=b-1;
12
c<=2*b;
13
a<=c;
14
b<=c-a;
15
c<=a;
16
endprocess;
Try to find it out. If you don't get it, then simulate it!
A little hint: due to the behaviour of signals you must only look
backwards at the last three assignments!
Have a &/very, very close look/ at this fact, otherwise you will
encouter some similar problems later on...
> I do not mean to undermine your experience but I never heard of this> before.
Yes, you are new to VHDL. You will learn further on...
Hello Omar,
Lothar already answered your questions in detail.
You are experiencing the same problem as many peoble, which are used to
software (running sequentially on a CPU) and now look the same way on
VHDL code.
But VHDL Code is no software in that sense. VHDL code may describe
something sequentially, but the described hardware will be present in
parallel and work in parallel. You may e.g. describe first, that you
need an AND-gate, and you describe second, that you need an XOR-gate.
Once this is mapped to real hardware, you will have all the gates in
parallel, though your description was one after the other.
Have a look at the RTL-viewer. It shows you, what parallel hardware is
generated from your sequential description. Understand what descriptions
lead to registers (good) and what descriptions lead to latches and
combinatorial loops (in most cases bad). Then you'll be able to use VHDL
successfully ;-)
Lothar Miller wrote:> A little hint: due to the behaviour of signals you must only look> backwards at the last three assignments!
A second hint: the order of the last three assignments is arbitrary. You
will get the very same results with this: