2
\$\begingroup\$

I made 2 verilog codes for a BCD adder. On the first code:

  • I made a carry look ahead adder module
  • That adder module is then instantiated inside the BCD_Adder module to add two 4 bits
  • Then I use my own function(use of truth table and K-map) to know if the sum from the adder module is >9, if yes add 6 but if not then add 0.
module carrylookahead(input[3:0]a,b,input c0, output[3:0]s,output c4);
  wire[3:0] P,G;
  wire[4:0] c;
  assign c[0]=c0;
  
  assign P=(a^b),G=(a&b); //Propagate and Generate
  assign c[1]=G[0]||(P[0]&&c[0]),
         c[2]=G[1]||(P[1]&&c[1]),
         c[3]=G[2]||(P[2]&&c[2]),
         c[4]=G[3]||(P[3]&&c[3]);
  assign s=(P^c[3:0]);
  assign c4=c[4];
endmodule


module BCD_Adder(input[3:0]a,b,input c0,output[3:0]s,output c4);
  wire[3:0] fx,s_t;
  wire c0_t,c4_t;
  assign c0_t=1'b0;
  
  carrylookahead m1(a,b,c0,s_t,c4_t);
  assign c4=(c4_t)||(s_t[3]&&s_t[2])||(s_t[3]&&s_t[1]); //func to know if BCD sum needs plus 6 or not
  assign fx={1'b0,c4,c4,1'b0};
  carrylookahead m2(s_t,fx,c0_t,s,c4_t);
endmodule

On my second code:

  • I use the plus operator so that I won't need to make my own carrylookahead module
  • I use magnitude comparator operator (>) so that I won't need to make my own mathematical function to know if the sum is greater or less than 9.
module BCD_Adder_v2(input[3:0]a,b,input c0,output[3:0]s,output c4);
  wire[3:0] s_t,fx;
  wire c4_t;
  assign {c4_t,s_t}=a+b,
         c4=(s_t>(4'b1001)),
         fx={1'b0,c4,c4,1'b0};//if s_t>9 THEN fx=6 but if not then fx=0
  assign s=s_t+fx;
endmodule

Both works the same way in my test bench, my question is when should I bother to make a detailed module like what I have done on my first code? I thought that my first code will have fewer gates needed since compared to the second gate I did not use a plus operator and a magnitude comparator operator.

\$\endgroup\$
8
  • \$\begingroup\$ if you know that the operators you can use do exactly what you need, then by all means, use them; your modules are probably pretty much the same size and speed, too. \$\endgroup\$ Oct 6, 2020 at 12:33
  • \$\begingroup\$ guess the compacter variant is OK, but I would seperate magic numbers like 4'b1001 and use em with a good name in code and things like wire[3:0] s_t,fx; I would consider bad style... \$\endgroup\$
    – schnedan
    Oct 6, 2020 at 12:36
  • \$\begingroup\$ @Marcus Müller how to know the size(is this number of gates, perhaps?) of a builtin operator? \$\endgroup\$
    – hontou_
    Oct 6, 2020 at 12:44
  • 1
    \$\begingroup\$ @IwataniNaofumi of course we all smile about "it was hard to write, it should be hard to read"... but more useful is, code is like a book where you are writing to entertain for someone else. make it explicit, clear, well commented. \$\endgroup\$
    – schnedan
    Oct 6, 2020 at 12:56
  • 1
    \$\begingroup\$ @IwataniNaofumi the operator on its own doesn't have a size. Your design overall has a size after it's implemented and optimized for specific hardware. Can't give you a number that doesn't exist! \$\endgroup\$ Oct 6, 2020 at 15:05

1 Answer 1

8
\$\begingroup\$

In general, you should not be focusing on optimizing the implementation via the source code — the toolchain is much better at that than you are anyway. And specifying individual gates in the source code is pointless on an FPGA anyway — A function like (s_t > 9) becomes a single 4-input LUT in either case.

Instead, focus on clarity and readability for other programmers, even if that other programmer is you — weeks, months or years in the future. You'll thank yourself later!

When building larger systems, it is the module interfaces that are much more important than their internal implementations. To that end, here are a few rules that I follow, relative to your code:

  • Don't declare more than one thing per line (module ports, internal signals).
  • Don't use binary constants if it's the decimal value that is really relevant.
  • Don't create signals that are never used.
  • Don't create signals that are only used once unless they improve readability.
  • You can merge a wire declaration with its assignment in many cases.

With that in mind, I would use your second version, and I would write it like this:

module BCD_Adder_v2 (
  input   [3:0] a,
  input   [3:0] b,
  input         c0,
  output  [3:0] s,
  output        c4
);
  wire    [3:0] s_t = a + b + c0;

  assign c4 = (s_t > (4'd9));
  assign s = s_t + {1'b0, c4, c4, 1'b0};    // Adjust sum by adding 0 or 6
endmodule

Note that I not only declare one thing per line, but I also line up the names vertically. This makes it easy to glance up to remind yourself what the declaration of any given item actually is.

\$\endgroup\$
2
  • 4
    \$\begingroup\$ Nice answer! To stress what it says about readability: Design teams usually require code to adhere to a style and formatting standard, simply because badly formatted code can cost days down the line, and thus is very expensive. In many cases, the formatting is checked automatically before the code a digital designer writes is checked in to the central code repository, and if it's not up to formatting standards, it either "goes back" with "please fix this and that" or gets automatically corrected. \$\endgroup\$ Oct 6, 2020 at 15:08
  • 4
    \$\begingroup\$ Human error in development is hard enough on development progress – making your code as readable as possible to minimize confusion is something crucial, especially to longer-lasting code bases (code is "write once, read many times"!). You can often spot teams where there's one engineer that is socially difficult by the amount of badly formatted code that does great things. Absolute beginners are the other kind of people that check in badly readable code, but they get honest feedback, and also usually don't write great code (it simply takes experience). \$\endgroup\$ Oct 6, 2020 at 15:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.