FPGA Central

Hey Guys,

I am pasting my code for an ALU implementation based on my old
undergrad textbook. I took some suggestions from the folks here from
my older posts, so I am hoping this code should be a bit better than
the first one in style and taste :-)

Anyways, two questions:

1) Do you suggeston using assign statements to registers in always @*
blocks? If not, is there an alternative? I am trying to work this
out just like it is in the textbook and the module diagram doesn't
show a clock input.
2) Are there any suggestions to improvise on this piece of code?

`timescale 1ns / 1ps
module alu(
opcode,
op1,
op2,
alu_result,
zero);

parameter OW = 4; // opcode width
parameter BW = 32; // bus width

parameter op_AND = 4'b0000; //bitwise and
parameter op_OR = 4'b0001; //bitwise or
parameter op_ADD = 4'b0010;
parameter op_SUB = 4'b0110;
parameter op_SLT = 4'b0111;
parameter op_NOR = 4'b1100; //bitwise nor

input [OW-1:0] opcode;
input [BW-1:0] op1;
input [BW-1:0] op2;
output [BW-1:0] alu_result;
output zero;

reg [BW-1:0] alu_result;
reg zero;

always @*
begin
if (opcode == op_AND)
assign alu_result = op1 & op2;
else if (opcode == op_OR)
assign alu_result = op1 | op2;
else if (opcode == op_ADD)
assign alu_result = op1 + op2;
else if (opcode == op_SUB)
begin
assign alu_result = op1 - op2;
assign zero = (op1 == op2) ? 1 : 0;
end
else if (opcode == op_SLT)
assign alu_result = (op1 < op2) ? 1 : 0;
else if (opcode == op_NOR)
assign alu_result = ~(op1 | op2);
end
endmodule


Mahurshi Akilla

First of all, the general idea is right. But the implementation is
wrong

always @*
begin
alu_result = 0; // to eliminate latch
zero = 0; // to eliminate latch
if (opcode == op_AND)
alu_result = op1 & op2;
else if (opcode == op_OR)
alu_result = op1 | op2;
else if (opcode == op_ADD)
alu_result = op1 + op2;
else if (opcode == op_SUB)
begin
alu_result = op1 - op2;
zero = (op1 == op2) ? 1 : 0;
end
else if (opcode == op_SLT)
alu_result = (op1 < op2) ? 1 : 0;
else if (opcode == op_NOR)
alu_result = ~(op1 | op2);
end

Or even better:
always @*
begin
alu_result = 0; // to eliminate latch
zero = 0; // to eliminate latch
case(opcode)
op_AND: alu_result = op1 & op2;
op_OR: alu_result = op1 | op2;
op_ADD: alu_result = op1 + op2;
op_SUB: begin
alu_result = op1 - op2;
zero = (op1 == op2) ? 1 : 0;
end
op_SLT: alu_result = (op1 < op2) ? 1 : 0;
op_NOR: alu_result = ~(op1 | op2);
default: begin
$display("Opcode %4h is not valid", opcode);
end
endcase
end

On Apr 15, 1:36 am, "Michael" <michae...@gmail.com> wrote:
> First of all, the general idea is right. But the implementation is
> wrong
The implementation is wrong? Were you kidding or were you
serious? :-) Sorry, I couldn't tell.

>
> always @*
> begin
> alu_result = 0; // to eliminate latch
> zero = 0; // to eliminate latch
Could you pl. explain what you mean by this? Was this done just to
eliminate the "red lines" from appearing at the beginning of the
simulation? If so, would it be okay to put these in an always @*
block? I thought you'd want it to initialize to 0 for once at the
beginning and not worry about it in the always @ blocks. I remember
that there's some syntax to have this be done only once at the
beginning of the simulation, but don't remember exactly what it is :-)

<...>

> Or even better:
> always @*
> begin
> alu_result = 0; // to eliminate latch
> zero = 0; // to eliminate latch
> case(opcode)
> op_AND: alu_result = op1 & op2;
> op_OR: alu_result = op1 | op2;
> op_ADD: alu_result = op1 + op2;
> op_SUB: begin
> alu_result = op1 - op2;
> zero = (op1 == op2) ? 1 : 0;
> end
> op_SLT: alu_result = (op1 < op2) ? 1 : 0;
> op_NOR: alu_result = ~(op1 | op2);
> default: begin
> $display("Opcode %4h is not valid", opcode);
> end
> endcase
> end

I really like these case statements. I totally forgot that they exist
in verilog too. I will switch to this style.

Another question: Is there a difference between saying "assign
alu_result = op1 - op2" and saying "alu_result = op1 - op2" ?

Mahurshi Akilla

On 15 Apr 2007 08:29:46 -0700, "Mahurshi Akilla"
<[email protected]> wrote:

>Another question: Is there a difference between saying "assign
>alu_result = op1 - op2" and saying "alu_result = op1 - op2" ?

YES!!!!!

If you write "assign something = expression;" inside a
procedural block (always block) in a design, you are
almost certainly doing the wrong thing. Who allowed
you to get away with this in your original coursework?
If it's from a textbook, tell me the name of the book
so I can tell my students that they should burn it :-)

The assign keyword does two wildly different things in
Verilog. You can use it *inside* an always or initial
block, as in your ALU. This is bad - I'll explain why
later. The common, sensible, OK-for-design use is
to put it at the top level of a module, outside any
always blocks:

module adder ( input [7:0] a,b, output [8:0] sum );
assign sum = a + b;
endmodule

In this case your "assign"...
* is driving a net (wire) - "sum" in this example;
* represents a driver (gate output) permanently
connected to the driven net;
* automatically updates whenever the expresson (a+b)
changes;
* is a completely sensible representation of a piece
of combinational logic.

There is, of course, an alternative. You could
write the same thing in an always block:

module adder ( input [7:0] a,b, output reg [8:0] sum );
always @(a or b) begin
sum = a + b;
end
endmodule

Note, now, that "sum" must be a variable - hence the "reg"
declaration in the port list. Once again, this is a
good and useful synthesisable description of an adder.

But your code contained stuff like this...

always @...
assign sum = a + b;

OUCH! This is a COMPLETELY different form of "assign".
It's a "procedural continuous assignment". It says that
when you execute that line of code, Verilog sets up a
process that monitors a and b, and whenever they change
it will calculate a+b and write that value to "sum".
This procedural continuous assignment will remain active
until you execute a different "assign sum = ...".

Your code, then, was effectively asking Verilog to switch-in
a completely new piece of hardware each time the opcode
changed! Most synthesis tools will not process that code.

Procedural continuous assigns can be useful for modelling
certain kinds of asynchronous load operation, but in my
opinion you should NEVER use them in synthesisable design.
--
Jonathan Bromley, Consultant

DOULOS - Developing Design Know-how
VHDL * Verilog * SystemC * e * Perl * Tcl/Tk * Project Services

Doulos Ltd., 22 Market Place, Ringwood, BH24 1AW, UK
[email protected]
http://www.MYCOMPANY.com

The contents of this message may contain personal views which
are not the views of Doulos Ltd., unless specifically stated.

Ahh.. thanks for the detailed explanation. Now, I understand it much
better. And no, it wasn't my textbook. It was just me fooling around
with behavioral verilog by myself for giggles. When I was in school,
my prof. made us write "structural" verilog (basically the "netlist"),
which I thought was hopeless. I guess his intention was to make us
aware of the underlying complexity in logic, but the result was that
none of us learned behavioral verilog. Anyways.. but that is school
and the profs. have their own ways. Now that I have graduated ... I
am trying to home school myself. :-)

Also.. could you explain me why we have the below two lines in the
always @ block? If the intention was to initialize to 0, why was it
placed inside the always @ block?

alu_result = 0; // to eliminate latch
zero = 0; // to eliminate latch

Thanks,

Mahurshi Akilla

On Apr 15, 8:59 am, Jonathan Bromley <jonathan.brom...@MYCOMPANY.com>
wrote:
> On 15 Apr 2007 08:29:46 -0700, "Mahurshi Akilla"
>
> <mahur...@gmail.com> wrote:
> >Another question: Is there a difference between saying "assign
> >alu_result = op1 - op2" and saying "alu_result = op1 - op2" ?
>
> YES!!!!!
>
> If you write "assign something = expression;" inside a
> procedural block (always block) in a design, you are
> almost certainly doing the wrong thing. Who allowed
> you to get away with this in your original coursework?
> If it's from a textbook, tell me the name of the book
> so I can tell my students that they should burn it :-)
>
> The assign keyword does two wildly different things in
> Verilog. You can use it *inside* an always or initial
> block, as in your ALU. This is bad - I'll explain why
> later. The common, sensible, OK-for-design use is
> to put it at the top level of a module, outside any
> always blocks:
>
> module adder ( input [7:0] a,b, output [8:0] sum );
> assign sum = a + b;
> endmodule
>
> In this case your "assign"...
> * is driving a net (wire) - "sum" in this example;
> * represents a driver (gate output) permanently
> connected to the driven net;
> * automatically updates whenever the expresson (a+b)
> changes;
> * is a completely sensible representation of a piece
> of combinational logic.
>
> There is, of course, an alternative. You could
> write the same thing in an always block:
>
> module adder ( input [7:0] a,b, output reg [8:0] sum );
> always @(a or b) begin
> sum = a + b;
> end
> endmodule
>
> Note, now, that "sum" must be a variable - hence the "reg"
> declaration in the port list. Once again, this is a
> good and useful synthesisable description of an adder.
>
> But your code contained stuff like this...
>
> always @...
> assign sum = a + b;
>
> OUCH! This is a COMPLETELY different form of "assign".
> It's a "procedural continuous assignment". It says that
> when you execute that line of code, Verilog sets up a
> process that monitors a and b, and whenever they change
> it will calculate a+b and write that value to "sum".
> This procedural continuous assignment will remain active
> until you execute a different "assign sum = ...".
>
> Your code, then, was effectively asking Verilog to switch-in
> a completely new piece of hardware each time the opcode
> changed! Most synthesis tools will not process that code.
>
> Procedural continuous assigns can be useful for modelling
> certain kinds of asynchronous load operation, but in my
> opinion you should NEVER use them in synthesisable design.
> --
> Jonathan Bromley, Consultant
>
> DOULOS - Developing Design Know-how
> VHDL * Verilog * SystemC * e * Perl * Tcl/Tk * Project Services
>
> Doulos Ltd., 22 Market Place, Ringwood, BH24 1AW, UK
> jonathan.brom...@MYCOMPANY.comhttp://www.MYCOMPANY.com
>
> The contents of this message may contain personal views which
> are not the views of Doulos Ltd., unless specifically stated.

On 15 Apr 2007 18:17:30 -0700, "Mahurshi Akilla"
<[email protected]> wrote:

>my prof. made us write "structural" verilog

It happens.

[...]
>Also.. could you explain me why we have the below two lines in the
>always @ block? If the intention was to initialize to 0, why was it
>placed inside the always @ block?
>
> alu_result = 0; // to eliminate latch
> zero = 0; // to eliminate latch

This sort of idiom is not an initialization; it's a default value.

always @* begin
special_case = 0;
if (incredibly_complicated_condition)
special_case = 1;
if (different_incredibly_complicated_condition)
special_case = 1;
end

Every time an input changes, the always block executes again.
If there is no other assignment to "special_case", it takes
its default value of 0; but if there is a second assignment
to it, the later assignment overwrites the earlier default.
Because it would be so very tiresome to determine the "else"
condition, the default assignment is very useful.

Here's another, more realistic example:

always @* begin
overflow = 0;
result = 0;
case (opcode)
op_NOP:
; // do nothing at all
op_ADD:
begin
result = opA + opB;
if ((result[7] != opA[7]) && (opA[7] == opB[7]))
overflow = 1;
end
op_XOR:
result = opA ^ opB;
endcase
end

In this situation we get several benefits from the
default assignments to "overflow" and "result":
- in the case statement, a branch that doesn't need to
act on "overflow" need not write to it
- the case statement is incomplete, but that doesn't matter
because we always have an assignment to every output

If we DON'T assign to every output, then the always block
will create a transparent latch on that output so that it
holds its value when not updated. This is, mostly, bad.

If you don't care about the value of "overflow" in the
cases where it's not specified, you may get better
(more compact) logic by making the default assignment
put "X" in it:

always @* begin
overflow = 1'bx;
result = 8'bx;
...

Synthesis tools will treat the X values as "don't care",
allowing them to create simpler logic. Simulation will
show you red ink on the waveforms if the X condition
occurs, helping you to find possible errors.

Hope this helps
--
Jonathan Bromley, Consultant

DOULOS - Developing Design Know-how
VHDL * Verilog * SystemC * e * Perl * Tcl/Tk * Project Services

Doulos Ltd., 22 Market Place, Ringwood, BH24 1AW, UK
[email protected]
http://www.MYCOMPANY.com

The contents of this message may contain personal views which
are not the views of Doulos Ltd., unless specifically stated.

On Apr 16, 9:17 am, "Mahurshi Akilla" <mahur...@gmail.com> wrote:
> Ahh.. thanks for the detailed explanation. Now, I understand it much
> better. And no, it wasn't my textbook. It was just me fooling around
> with behavioral verilog by myself for giggles. When I was in school,
> my prof. made us write "structural" verilog (basically the "netlist"),
IMO this is best done in a graphical schematic rather than in text -
what I do is write a little conceptual layout using text graphics in a
comment in my top blocks, then just wire the blocks together.

> which I thought was hopeless. I guess his intention was to make us
> aware of the underlying complexity in logic, but the result was that
> none of us learned behavioral verilog.
Should have taught you barrel shifter implementations instead, which
shows how, yes, logic is complicated, and no, once you *get* it, you
just think "kewl, that was so simple, why didn't I think of that!"

>
> Also.. could you explain me why we have the below two lines in the
> always @ block? If the intention was to initialize to 0, why was it
> placed inside the always @ block?
>
> alu_result = 0; // to eliminate latch
> zero = 0; // to eliminate latch
>
Here's a different attempt at explaining:

Latches are EVIL (unless you know what you're doing, in which case you
should be able to judge when to use latches).
Latches must be eliminated!
To eliminate latches, the previous value of the register should never
be remembered.
....so, to eliminate latches, you have to *erase* the previous value.

An alternative (which I prefer) is to use the default branch:
default: begin
alu_result = 0; zero = 0; end endcase end

Also, as suggested, using x (usually 'bx, if you have a variable-width
design and are too lazy to do something like {width{1'bx}}).