Continue to Site

Welcome to EDAboard.com

Welcome to our site! EDAboard.com is an international Electronics Discussion Forum focused on EDA software, circuits, schematics, books, theory, papers, asic, pld, 8051, DSP, Network, RF, Analog Design, PCB, Service Manuals... and a whole lot more! To participate you need to register. Registration is free. Click here to register now.

Register Log in

Is this a bad style of coding?

Status
Not open for further replies.
A

Alexium

Full Member level 2
Full Member level 2
Joined
Jan 6, 2011
Messages
148
Helped
39
Reputation
78
Reaction score
39
Trophy points
1,308
Location
Ukraine
Activity points
2,163
Greetings.
I have suddenly found myself writing such a code:

Code VHDL - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13

RXLoop: process (clk, rst)
begin
    if clk'event and clk = '1' then
       RXNewPacket <= '0';                                      
        if RXDone = '1' then
            if some_condition then
                --some code
            else
                RXNewPacket <= '1';
            end if;         
        end if;
    end if;
end process;


So, I have this signal, RXNewPacket, that should be implemented as a flip-flop. And it looks like I'm doing two asignments to it when some_condition is false. On the one hand, by the rules of VHDL, only the last asignment will take effect (and this is what I need). But somehow it doesn't look right to me, and I tried to avoid this coding technique in the past.
Will this code work correctly with any abstract synthesizer, and are there any reasons to avoid this?
Thanks.
 

this is generally accepted. It is generally bad practice if RXNewPacket is defined in multiple different if-else blocks. The above code is a compromise most designers will accept. it really has a strong role in combinatorial circuits where leaving the signal out of one path would result in a latch. It is common whenever there are a lot of conditions that result in the signal being set to '0', and only a few that set it to '1'.
 
I use it regularly in state machines, and other cases where assigning something is done rarely (so give it a default value like you have and override it later). Ive not had a problem.

From a readability point of view be careful not to embed too many nested if-else trees. It can become annoying and difficult to read, but the synthesisor should just flatten it into a single logic circuit.
 
That's fine for a flag/pulse type signal that has a 'default' value most of the time. Particularly if it is only sparsely set to a non-default in the if/then/else tree. Which is most likely the case for a signal with a name like that.

Of course if this was being used as a state variable/bit, you don't want to assign a value by default since the whole point of state variables is to hold on to what they had last cycle. And a default "A <= A;" is unnecessary since A already keeps its old value unless there's an "A <= B;" somewhere in the if/then/else.

also, rising_edge(clk) is a little more modern than clk'event and clk='1'.

Otherwise, fairly good style. and kudos for actually labeling the clocked process.
 
Status
Not open for further replies.

Similar threads

Part and Inventory Search

Welcome to EDABoard.com

Sponsor

Back
Top