UPGRADE YOUR BROWSER

We have detected your current browser version is not the latest one. Xilinx.com uses the latest web technologies to bring you the best online experience possible. Please upgrade to a Xilinx.com supported browser:Chrome, Firefox, Internet Explorer 11, Safari. Thank you!

cancel
Showing results for 
Search instead for 
Did you mean: 
Scholar ronnywebers
Scholar
2,327 Views
Registered: ‎10-10-2014

VHDL coding style question - using default values with if-elsif-elsif ... chains

Jump to solution

I'm reviewing some HDL code of a collegue - he uses this code style a lot, and his code seems to work,, but it looks ambiguous to me.  Here's a code extract, in wich data from a soft-core cpu gets written to either SPI port 0 or SPI port 1

 

    process(resetn, clk)
    begin
    
    if(resetn = '0') then
                
        s_spi_run <= '0';
        s_din_buf_0 <= (others => '0');
        s_din_buf_1 <= (others => '0');
        
    elsif(rising_edge(clk)) then
    
        if(io_wr_0 = '1') then    -- Write pulse to Interface 0
    
            s_din_buf_0 <= io_din_0;    -- Buffer Data
            s_spi_select <= '0';         -- Select SPI Interface 0
            s_spi_run <= '1';            -- Initiate SPI_0 transaction
        
        elsif(io_wr_1 = '1') then -- Write pulse to Interface 1
        
            s_din_buf_1 <= io_din_1;     -- Buffer Data
            s_spi_select <= '1';          -- Select SPI Interface 1
            s_spi_run <= '1';             -- Initiate SPI_1 transaction
            
        elsif(s_spi_busy = '1') then   -- ***
        
            s_spi_run <= '0';   -- Reset SPI run trigger when the busy feedback flag is high
            
        else
        
            s_spi_run <= s_spi_run;
            s_din_buf_0 <= s_din_buf_0;
            s_din_buf_1 <= s_din_buf_1;
        
        end if;
        
    end if;
        
    end process;

I have a few questions about this code style :

 

1) in the elsif marked with '***' 

-> there is no assignment to s_din_buf_0 or s_din_buf_1

 

Q1 : could this cause issues? how does synthesis handle this?

 

in a 'quick edit' I would have written it like this, to avoid any ambiguity :

        elsif(s_spi_busy = '1') then
        
            s_spi_run <= '0';   -- Reset SPI run trigger when the busy feedback flag 
	    s_din_buf_0 <= s_din_buf_0;
	    s_din_buf_1 <= s_din_buf_1;

        else

 

also, s_spi_select only gets a value assigned in 2 of the if case, in the other 2 nothing is assigned to s_spi_select (he also forgot to put it in the rest part) 

 

Q2 : how does synthesis handle this?  what does synthesis do in the 2 if's where there is no assignment? Is it 'free to choose' what to do?

 

Q3 : the 'else' case at the end -> since it's a clocked process (so no unintentional latches can be created), is this really needed? What is the difference if I would just remove it?

 

Q4 : would this be a better code style towards unambiguous synthesis? (I"m using 'defaults' to avoid unnecessary assignments in each 'if' :

 

    process(resetn, clk)
    begin
    
    if(resetn = '0') then     
        s_spi_run <= '0';
        s_din_buf_0 <= (others => '0');
        s_din_buf_1 <= (others => '0');
        s_spi_select <= '0';
        
    elsif(rising_edge(clk)) then
        -- set defaults
        s_spi_run <= '0';
        s_din_buf_0 <= s_din_buf_0;
        s_din_buf_1 <= s_din_buf_1;       
        s_spi_select <= '0';

        if(io_wr_0 = '1') then    -- Write pulse to SNaP Port Interface 0
            s_din_buf_0 <= io_din_0;    -- Buffer Data
            s_spi_select <= '0';         -- Select SPI Interface 0
            s_spi_run <= '1';            -- Initiate SPI_0 transaction
        
        elsif(io_wr_1 = '1') then -- Write pulse to SNaP Port Interface 1    
            s_din_buf_1 <= io_din_1;     -- Buffer Data
            s_spi_select <= '1';          -- Select SPI Interface 1
            s_spi_run <= '1';             -- Initiate SPI_1 transaction
            
        elsif(s_spi_busy = '1') then
            -- this is handled by the defaults, but it's clearer to add it here
            s_spi_run <= '0';   -- Reset SPI run trigger when the busy feedback flag is high
        
        end if;
    end if;
    
    end process;

I think this code avoids ambiguity. But maybe there's room for further improvement?

 

also, I would probably use a synchronous reset here instead of async

 

 

 

** kudo if the answer was helpful. Accept as solution if your question is answered **
Tags (1)
1 Solution

Accepted Solutions
Scholar richardhead
Scholar
3,336 Views
Registered: ‎08-01-2012

Re: VHDL coding style question - using default values with if-elsif-elsif ... chains

Jump to solution

There are no issues, as it is a clocked process. Any path where a signal is assigned conditionally just puts logic on the enable pin of the register (which is a normal thing).  Assigning a signal to itself is unnecessary as this would be the default action anyway. I would say having assignments like this is MORE ambiguous as it may make a reader believe something is happening, when actually it isnt.

 

So to answer directly:

 

1. No Issues - see above

2. See above

3. else case not needed - see above.

4. Remove the default self assignments, and it is better code.

 

I think you might be getting confused between synchronous logic and asynchronous logic. Async logic MUST be assigned a value in ALL cases to avoid latches, but self assignments will create a latch.So never do self assignments in asynchronous logic, unless that logic is then registered (when that would be fine). But, using the two process style like this may mean the clock enable pin of the register is unused.

14 Replies
Mentor hgleamon1
Mentor
2,317 Views
Registered: ‎11-14-2011

Re: VHDL coding style question - using default values with if-elsif-elsif ... chains

Jump to solution

Apart from the asynchronous active low reset (some good reason, I hope), remember that it is a clocked process so you are inferring registers.

 

Any lack of explicit assignment in a register means that it will hold its previous value. Guaranteed. I would say that this is accepted coding standard and helps keep the line count down without making the code confusing.

 

I then, however, fail to see the point of the final else condition of self-assignment as it does absolutely nothing. Synthesis will handle the implicit signal retention just fine and completely ignore the final else as being useless (which it is).

----------
"That which we must learn to do, we learn by doing." - Aristotle
Scholar richardhead
Scholar
3,337 Views
Registered: ‎08-01-2012

Re: VHDL coding style question - using default values with if-elsif-elsif ... chains

Jump to solution

There are no issues, as it is a clocked process. Any path where a signal is assigned conditionally just puts logic on the enable pin of the register (which is a normal thing).  Assigning a signal to itself is unnecessary as this would be the default action anyway. I would say having assignments like this is MORE ambiguous as it may make a reader believe something is happening, when actually it isnt.

 

So to answer directly:

 

1. No Issues - see above

2. See above

3. else case not needed - see above.

4. Remove the default self assignments, and it is better code.

 

I think you might be getting confused between synchronous logic and asynchronous logic. Async logic MUST be assigned a value in ALL cases to avoid latches, but self assignments will create a latch.So never do self assignments in asynchronous logic, unless that logic is then registered (when that would be fine). But, using the two process style like this may mean the clock enable pin of the register is unused.

Mentor hgleamon1
Mentor
2,312 Views
Registered: ‎11-14-2011

Re: VHDL coding style question - using default values with if-elsif-elsif ... chains

Jump to solution

With regards to Q4 I'm not convinced by your default assignments.

 

Without knowing how the io_wr_x signals behave (the comments say pulse but for how many clock cycles?). If it is a single clock pulse then your s_spi_select signal will toggle back to zero after 1 clock instead of retaining its value, which may not be the intended behaviour (my experience of SPI is the the select signal should be active for the entire duration of the transaction).

----------
"That which we must learn to do, we learn by doing." - Aristotle
Scholar ronnywebers
Scholar
2,289 Views
Registered: ‎10-10-2014

Re: VHDL coding style question - using default values with if-elsif-elsif ... chains

Jump to solution

@hgleamon1, io_wr_x  is indeed a single pulse, you're right about that - interesting remark ...  thanks!

** kudo if the answer was helpful. Accept as solution if your question is answered **
0 Kudos
Scholar ronnywebers
Scholar
2,285 Views
Registered: ‎10-10-2014

Re: VHDL coding style question - using default values with if-elsif-elsif ... chains

Jump to solution

just wondering if the same goes for FSM's - I always use defaults there, to avoid unnecessary assignments. But looks I need to review this habbit  :-)

 

architecture behavioral of fsm_1 is
	type state_type is (s1, s2, s3);
	signal state : state_type;
begin
	process(clk)
	begin
		if rising_edge(clk) then
			if (reset = '1') then
				state <= s1;
				out1 <= '0';
				out2 <= '0';
			else
				-- put defaults here?
				-- out1 <= '0';
				-- out2 <= '0';
				
				case state is
				when s1 => 
					if flag = '1' then
					    out2 <= '1';					
					    state <= s2;
					else
					    state <= s2;
					end if;
				when s2 => 
					out1 <= '1';				
					state <= s3;
				when s3 => 
					out2 <= '0';
					state <= s1;					
				end case;
			end if;
		end if;
	end process;
end behavioral;

The fsm simply goes from s1 -> s2 -> s3 -> s1 ...

 

regarding out1

as soon as s2 happens, out1 gets 'stuck at 1'

 

regarding out2

in s1, flag decides wether out2 gets toggled to '1' or left unchanged (and thus '0' as set in s3)

 

if I uncomment the defaults , I know in each state & if branch what the value of out1 and out2 is.

 

but if I ommit the defaults, out2 can have either '0' or '1' in state 's2'

 

so I see 2 options here to keep code clear : either assing out1 and out2 in each state, in each if branch, ... or use defaults.

 

what is your opinion on that @hgleamon1 @richardhead

 

** kudo if the answer was helpful. Accept as solution if your question is answered **
0 Kudos
Scholar ronnywebers
Scholar
2,284 Views
Registered: ‎10-10-2014

Re: VHDL coding style question - using default values with if-elsif-elsif ... chains

Jump to solution

actually, I started using defaults after this post. If you look at the fsm code at the bottom of page 1, you see that I use 'defaults' there.

** kudo if the answer was helpful. Accept as solution if your question is answered **
0 Kudos
Scholar richardhead
Scholar
2,259 Views
Registered: ‎08-01-2012

Re: VHDL coding style question - using default values with if-elsif-elsif ... chains

Jump to solution

I use defaults for signals I know I usually want to be a specific value, and only set something different occasionally. If it is a signal that changes in several states I would probably not assign a default.

 

Defaults can be a little dangerous in larger state machines, as other users may not realise a default assignment has been made, and you are relying on the VHDL mechanics to override the default assignment, but it is a fairly standard practice thing to do (some people do it and like it, some maybe dont like it). Story here: make your code obvious whats going on.

 

WRT your code - I cant really comment on whether default or no default is better. It depends on the spec. If the outs was some form of enable or valid signal, you probably want it defaulted rather than latched, so it is easier to track what value it could take. But if they were a data bus (with sideband enable/valid) you probably dont care about tracking it in ever state - you can just latch it (no default) as you dont care what value it has when the valid/enable is low.

 

Scholar ronnywebers
Scholar
2,246 Views
Registered: ‎10-10-2014

Re: VHDL coding style question - using default values with if-elsif-elsif ... chains

Jump to solution

thanks @richardhead, I got the point, learned something today :-)

 

last question : in your reply above you wrote 'But, using the two process style like this may mean the clock enable pin of the register is unused.'

 

do you mean by 2 process style, the async reset followed by the clocked part? Does writing code like this prevent usage of the enable pin? 

** kudo if the answer was helpful. Accept as solution if your question is answered **
0 Kudos
Mentor hgleamon1
Mentor
2,236 Views
Registered: ‎11-14-2011

Re: VHDL coding style question - using default values with if-elsif-elsif ... chains

Jump to solution

@ronnywebers

 

It seems to me you are slightly confusing design and coding style. If your company has a coding standard you should be following that to the letter.

 

Using defaults is absolutely OK behaviourally if you understand what you are doing. And for that, you need to understand the architecture or requirements - perhaps you are the both the architect/designer and the implementor.

 

To remain general for a moment, I use defaults on signals that I know I want to have that one-shot behaviour (a bit like a monostable). It defaults to its deasserted state until the VHDL syntax assigns an asserted state. The next clock cycle it becomes deasserted again (unless the asserting condition remains true). I find this simplifies many lines of code and looks aesthetic.

 

In your specific case of your example FSM, I cannot comment precisely because I don't know what the behaviour of out1 and out2 is designed to be. If you want out1 to be "stuck at 1" then your code is OK. If you want out1 to be deasserted after one clock cycle, then use defaults or explicitly deassert the signal in the next state.

 

In terms of built logic it (possibly) won't matter and it will, in any case, function the same in hardware (if you investigate the FPGA Editor for that signal you might find that the LUT has different inputs to generate the same output depending on defaults or explicit coding).

 

On a slightly unrelated note, in state s1

when s1 => 
 if flag = '1' then
     out2 <= '1'; 
     state <= s2;
 else
     state <= s2;
 end if;

this code is more tidily realised as

 

when s1 => 
  if flag = '1' then
    out2 <= '1';
  end if;
  state <= s2;

the transition to state s2 will happen in either branch, so it doesn't need to be explicitly coded. Only the assignment to out2 is dependent on the input flag.

 

 

For a bit of TL;DR summary: use defaults if you fully understand the design and how to implement it. Use explicit coding if you are not sure if the particular part of the design benefits from defaults.

----------
"That which we must learn to do, we learn by doing." - Aristotle
Scholar richardhead
Scholar
1,463 Views
Registered: ‎08-01-2012

Re: VHDL coding style question - using default values with if-elsif-elsif ... chains

Jump to solution

The two process style is the one favoured by many older text books, that separate the asynchronous logic from the register descriptions. It used to be the case (before I started in the industry) that synthesisors needed this separation to actually work, but this has not been the case for many years. But since it is in textbooks and many engineers learned this method, it has stuck around (this again is a point about coding style).

 

2 process would be something like this:

 

 

next_state_proc : process(state, ip1, out1)
begin
  -- defaults avoid latches
  out_next <= out1;
next_state <= state; case state is when s1 => out_next <= '1';

if ip1 = '1' then next_state <= s2;
end if; when s2 => next_state <= s3 --etc end process; reg_proc : process(clk) begin if rising_edge(clk) then out1 <= out_next; state <= next_state; end if; end process;

 

Another thing Id like to point out in your code - the reset. It is common style to have the reset and non-reset in separate branches, like this:

 

 

sync_proc : process(clk)
begin
  if rising_edge(clk) then
    if reset = '1' then
      -- sync reset goes here
    else
      --logic goes here
    end if;
  end if;
end process

There is a slight gotcha with this. Using this style then ALL signals assigned in the process MUST be reset, otherwise the reset becomes part of the enable of the register, if it is missed from the reset part of the process. And usually you dont want to reset data path registers as there is usually no need. This leads to extra logic, routing and potentially making it harder to fit/time the design. If you must keep this style, the only workaround is to have two processes - one for registers with a reset and one with no reset.

 

 

You can use a style (that some hate) that allows you select which registers inside the process and which do not. All you have to do is put the reset at the end of the process

 

sync_proc : process(clk)
begin
  if rising_edge(clk) then
    --logic goes here

if reset = '1' then -- sync reset goes here - you can chose what to reset. end if; end if; end process

 This works because VHDL signals always take the last value assigned to them, and if reset is active then that will have priority. This can be applied to asynchronous resets as well, but again, because this doesnt fit with the "standard" style, some engineers dont like it.

Scholar ronnywebers
Scholar
1,453 Views
Registered: ‎10-10-2014

Re: VHDL coding style question - using default values with if-elsif-elsif ... chains

Jump to solution

thanks @hgleamon1 for the detailed explanation! 

 

we currently have no coding standard, but I'm actually working on it :-)

 

 I've seen that many books have different styles, don't go very much in detail, certainly don't explain the do's and don'ts, are very often outdated, ... so I usually turn to this forum to learn from people like you :-) I want to really understand the language & impact of syntax on the final result.

 

I think Xilinx should add a lot more (explanation) to the synthesis guide (UG901). But until then, I'm glad we have the forum.

 

 

** kudo if the answer was helpful. Accept as solution if your question is answered **
0 Kudos
Scholar ronnywebers
Scholar
1,452 Views
Registered: ‎10-10-2014

Re: VHDL coding style question - using default values with if-elsif-elsif ... chains

Jump to solution

thanks @richardhead, I was not aware of this :

 

Using this style then ALL signals assigned in the process MUST be reset, otherwise the reset becomes part of the enable of the register, if it is missed from the reset part of the process

 

I'm trying to get why/how this reset becomes part of the enable : I read something about a limited number of control sets in CLB's, does that have something to do with it?

 

I should double check that reset on my designs! Great tip!

 

** kudo if the answer was helpful. Accept as solution if your question is answered **
0 Kudos
Highlighted
Scholar richardhead
Scholar
1,440 Views
Registered: ‎08-01-2012

Re: VHDL coding style question - using default values with if-elsif-elsif ... chains

Jump to solution

Nothing to do with logic, just the code.

look at this code:

 

process(clk)
begin
  if rising_edge(clk) then 
    if srst = '1' then
      reg0 <= '0';
    elsif
      reg0 <= ip0;
      reg1 <= ip1
    end if;
  end if;
end process;

If srst = '1', then you're asking for an assignment to reg0, but there is no assignment to reg1. So reg1 will hold it's state when srst = '1', meaning reg1 can only take the value of ip1 when srst ='0'. 

Scholar ronnywebers
Scholar
1,433 Views
Registered: ‎10-10-2014

Re: VHDL coding style question - using default values with if-elsif-elsif ... chains

Jump to solution

thanks @hgleamon1 and @richardhead for your great advise. I wish I could accept 2 answers, but I'm limited to 1. 

 

 

** kudo if the answer was helpful. Accept as solution if your question is answered **
0 Kudos