Verilog case statements

It may be cleaner to have simpler logic where the style mask_wr

process(rst_n,clk) begin if (rst_n='0') then mask_wr

Two FFs and four logic elements (x15 in the real circuit). You have to use a LUT to drive the D input and you have to use a LUT to drive the enable input. Of course you can feedback the output to the input and not use the clock enable, but that will also use more LUTs depending on if you have the extra inputs or not on the LUTs you are using. But this will be up to the tool and many times I see the tool generating logic to feed the clock enable.

It is not a big deal. I tend to think of things like this when I code in an HDL just so I know my logic generation is lean. Originally I had not seen the enclosing always statement and was thinking it was generating a latch. Once I started looking at it I realize the latch was not an issue but thought about the extra logic being generated. I tend to separate my registers from my logic just to allow me to optimize this sort of thing.

Rick -

This looks fine to me. It's easy to read and will function correctly, plus the case statement and use of parameters make it easy to modify. Given what I've heard of your relationship with the FPGA group, I would not flag this if I were you. It would be nitpicking.

Rob

I don't know your target, but I don't thinks a clock-enable will be implemented. There will be a mux that selects ('1', '0' or the previous value of the FFs) I guess.

If this is the desired functional behavior I don't see any option to make it smaller. If there are don't care conditions there may be options.

There will be no difference between the way of coding this (two processes for the FFs and the combinational logic). There will be only a difference if you find some functional simplification of the problem.

Ralf

You're taking the code structure as a literal guide for synthesis. There will be two FFs and two LUTs without a clock enable. Synthesis typically knows what simple logic breaks down to and how to best implement it. If you go into the technology view of your synthesizer to look at one of the flops, expect to see only one LUT driving it, no clock enable involved.

I never said I was going to "flag" it. I was just asking for some help understanding how Verilog worked. If it were VHDL I would know for certain how the synthesizer would handle it. But I am pretty confident that this will be less efficient than a properly constructed description.

That is how I code differently from a lot of people. Many designers "program" in the HDL. I design my hardware in block diagrams and use the language to describe my hardware. The above code certainly functions correctly, but it would be just as clear, if not more so to separate the logic and the register. A register description is very, very simple and small. These can be put after the logic or they can be lumped together in a register bunch at the end of the module.

The above program coded as logic would have a separate assignment for each signal. These assignments would not have any unspecified conditions and so would implement very efficiently and of course no latches. Expand the above code to 12 address lines and 25 signals and you will see how inefficient this could get.

With this simple example you are right, it will require a single

4-input LUTs even if you don't use the CE. The logic functions only has four inputs and so can be done in a single 4-input LUT. They are wr, the two address bits and the feedback from the given register. But if there is one more address line the feedback will push it to a pair of LUTs. Like I said, it is not a huge difference, but with the large number of signals in the real code, this could have been done very slightly different and would have been optimal.

I realized that a simple change would not be any more typing and would fully specify the assignments to eliminate the need for the feedback signal. At least if this works like VHDL it would work correctly. Like I said, I am not up to speed in Verilog anymore.

always @ (negedge rst_n or posedge clk) begin if (!rst_n)begin mask_wr

The only problem with that code is the lack of a default statement. Without a default statement the synthesizer will screw it up because the undefined cases won't be treated as do nothing. If you add a default statement then the synthesizer will do the right thing.

Gotta disagree, Josh. In a clocked process it is understood (and properly implemented by synthesis tools) that for undefined cases the last state of each register is held, which is what you want. You only need to explicitly code transitions. As long as the registers are initialized, which they are here by an async reset, then you're good to go. Even that is mainly a simulation issue. A synthesis tool can even handle no initialization, but you have to know what you're getting. In a non-clocked process you must spec defaults (either before the case statement or inside the case statement), or latches are inferred. In my opinion there is absolutely nothing wrong the code block Rickman posted other than asthetics (I don't care for how the begin/ends are indented).

Rob

But this is different to the initially posted code example! In the inital example both FFs hold their old values in some circumstances while now everytime a value is assigned to their inputs. Therefore this is a functional simplification which can only be done if the resulting behavior is acceptable.

If acceptable it seems to me to be a good way to code this.

Ralf

Ah, but the unspecified cases are where the code is not doing what was intended. In the code posted, the previous value will be held. Instead the intent was that the value should always be 0 except for the decodes specified in the case statement. It was just luck that the input conditions never create a failure. As long as the address remains constant while the wr strobe is asserted, it will not have to "remember" a value. That is why I said this would not work so well for the read decodes.

I can tell you from first hand experience that the current version (8.2sp1) of XST mishandles case statements that don't have defaults, I just ran into the problem. I've seen this happen before with other synthesizers. It's just good practice to include a default in a case statement. That default can be empty but it's presence tell the synthesizer that the states don't change for the un-specified cases.

That was the point. The initial code was not coded correctly for the intended behavior. It only worked in the application (so far) because the address did not change. This is just a set of address decodes feeding into a register. It is intended to be clocked on every clock cycle. In fact if I gave it a bit of thought, I would remove the enclosing IF and make the signal assignments from the WR signal...

always @ (negedge rst_n or posedge clk) begin if (!rst_n)begin mask_wr

Coding style like this?

wire mask_wr_in = (adr[2:1] == MASK) & wr; wire clear_wr_in = (adr[2:1] == CLEAR) & wr;

Yep, that is pretty much what I would have written. I think this would be the simplest and clearest description of the circuit that is desired. But after pushing it around a bit, I did come up with an always statement (shown later in this thread) that was about the same amount of code and could be said to be pretty much as clear as your code. At that point it just becomes a matter of preference.

Actually, I just looked a bit harder at your code and I don't understand the notation mask_wr Coding style like this?

Delay by 1 time step. (The step size is defined with the 'timescale directive.)

Although Verilog does not offer delta delays like VHDL it is possible to get almost the same behavior using blocked signal assignments (the "