Common Coding Mistakes for Lab 2A.
Contributed by the (brilliant) Nikolaos Strikos, Spring 2012 141L TA.
Updates by the (pizza-loving) Michael Taylor, Spring 2012 141L Professor.
mux.v items
[A] Your inputs are inversed, i.e. if sel is 0, your mux returns input 1 and vice versa.
signext.v items
[A] The correct syntax to replicate a bit N times is {N{bit}}.
[B] Your sign extender was not correctly/fully parameterized.
[C] You used for. Bad. Generally non-synthesizable. Please check coding hints below.
[D] The output is off by one.
fetch.v items
[A] You have been given a parameterized fetch unit module; your submodules and nets should be parameterized too.
[B] sel_mux should be declared as a vector of 4 nets, each of which is a single bit control signal that drives the sel input of mux0-mux3
respectively.
[C] pc_r is pc_next registered and this should be a (non-blocking) assignment in some always_ff block.
[D] Respect the names of signals and submodules given in the reference schematic.
[E] ip_in and ip_out should be declared with type instr_packet_s.
[F] All internal signals you introduce must be declared.
[G] According to the schematic, restart_addr_i, load_store_addr_i and store_addr_i must be registered.
[H] instruction_addr_o, instruction_data_o and load_data_o are ip_out.addr, ip_out.instr and load_data_o; they must be assigned and there is
no register or logic in between.
[I] Don't use <= in always_comb. Don't use = in always_ff.
[J] Use the adder module to add to pc_r, don't do pc_r + 1.
[K] This part does not ask to implement any of the control logic. Don't assign (incorrect) values to sel_mux and other control signals.
[L] The MSB of logic [N-1:0] my_wire is my_wire[N-1]. It's not my_wire[0]. The 5 lowest bits of my_wire are my_wire[4:0].
[M] There is some error in the datapath connections of the muxes. Make sure the output of every mux is connected to the correct input
of the next one.
[N] The data inputs and outputs of muxes are instruction addresses, i.e. logic of width A_WIDTH.
[O] You're feeding the +1 adder with an incorrect version of the literal 1. If you use 1'b1, this will leave
the remaining A_WIDTH - 1 wires to z. If you use just plain 1, and do not specify a width, then you will default
to 32 bits, which will cause a warning in many cases, and will bite you if 32 is not enough. Here's how to do it:
- In Verilog, you should always specify a width, but, you need to make sure it is the right width. For the value 1,
a parameterized version would be:
{ { A_WIDTH-1 { 1'b0 } }, 1'b1} // A bit ugly, but does the right thing.
- In System Verilog, it gets better -- you can make use of the '1 ( == 0xFFFFFF.....FFF), '0 (== 0x0000 ... 000) primitives,
as described in the slides. In this case, we can do:
{ '0, 1'b1 } // The '0 will expand out to however many 0's you need. Progress!
[P] You have done incorrect modifications to the provided code.
[Q] All registers should latch their input at the positive edge of the clock, i.e. you muse use @(posedge clk) for every always_ff block.
[R] Don't use positional arguments.
Coding hints
a. Don't create needless variables. If the .d0_i of an adder is 1, just use .d0_i({'0,1'b1}).
b. You don't have to use a whole always_comb block when a simple assign will do; it takes more time and space to write!
c. Specify your clock for always_ff by appending @(posedge clk).
d. Guys, you don't need 15 LOC for a sign extender that only takes 1 to write:
assign d_o = { { OUT-IN { d_i[IN-1] } }, d_i };
No always_comb blocks, no strange bit manipulations, no shifting, no adding, no intermediate variables, no for's are needed. Learn to think
simply (but not simplistically!) when making something.
e. Simplicity is the ultimate sophistication." -- Leonardo Da Vinci
f. "Rule of Simplicity: Design for simplicity; add complexity only where you must.
Rule of Robustness: Robustness is the child of transparency and simplicity."
-- Eric Raymond