Forum Discussion

Altera_Forum's avatar
Altera_Forum
Icon for Honored Contributor rankHonored Contributor
16 years ago

Shift register problem

I'm having trouble using the lpm shift register. I want to create a shift register using the lpm shift register in quartus. The problem I think is that I'm trying to use it on a clocked environment while the megafunction is not clocked. It compiles fine but my signals are synthesized away for some reason and I need them for the code to work. Here is my code: 


library ieee;
use ieee.std_logic_1164.all;
entity shift_register_2 is
	port
	(
		clk,reset: in std_logic;
		w: in std_logic;
		z: out std_logic
	);
end shift_register_2;
architecture structure of shift_register_2 is
	signal r_in: std_logic_vector(3 downto 0);
	signal r_out: std_logic_vector(3 downto 0);
	
	component shift
	port
	(
		data		: IN STD_LOGIC_VECTOR (3 DOWNTO 0);
		distance		: IN STD_LOGIC ;
		result		: OUT STD_LOGIC_VECTOR (3 DOWNTO 0)
	);
	end component;
	
begin
	process(clk,reset,w)
	begin
		if(reset='1') then
			r_in <= "0000";
		elsif(clk'event and clk='1') then
			r_in <= w & r_out(2 downto 0);
		end if;
	end process;
	
	process(r_out)
	begin
		if(r_out="1001" OR r_out="1111") THEN
			z<='1';
		else z<='0';
		end if;
	end process;
	
	stage0: shift port map(r_in,'1',r_out);
end structure;

11 Replies

  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    16 years ago

    Unfortunately, we can't see the LPM_CLSHIFT parameters, particularly if you are shifting in the right direction.

    But basically you are trying a longwinded solution for a simple problem. You can implement the shiftreg in a single VHDL line without instantiating a MegaFunction. 

    signal sr: std_logic_vector(3 downto 0);
begin
process(clk,reset)
  begin
    if(reset='1') then
      sr <= "0000";
    elsif(clk'event and clk='1') then
      sr <= w & sr(3 downto 1); -- implements a right shift
    end if;
  end process;
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    16 years ago

    I think our professor wants us to use a megafunction so that we learn how to use them correctly. The thing that I don't understand is why my signals get synthesized away. If I have a signal and I'm using it to hold values why would quartus synthesize it away? How can I keep my signals with their values?

  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    16 years ago

    As I already mentioned, the only explanation I can think of, is that you are shifting the data in the wrong direction. Then, the w input is ignored, the sr output is constantly zero and thus the respective logic is removed in synthesis. Assuming a meaningful shift direction, your design should work.

    In my opinion, learning how to use MegaFunctions correctly also involves to distinguish, when their usage is effective and when other means are more appropriate. But of course, the problem can be solved this way. I wonder however, if LPM_SHIFTREG rather than LPM_CLSHIFT has been meant.
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    16 years ago

    I couldn't find the shift register before but I found it this time and it was much easier to set up and use and it worked correctly! My signals were still synthesized away though.

    I'm glad that I was able to get that to work but I'd still like to know if theres a way to keep things from being synthesized away or something.

    In this example I have a modified register and a constant and I basically want to check if the register contents = the constant when the counter gets to a certain value but with the registers being synthesized away, how can I ever see what the values are without just making unneeded output pins? 

    
stage0: crc_register port map(main_clk AND compute_enable,main_reset,main_u,r_out);
stage1: lpmconstant port map(r_fixed);
check <= '1' WHEN (counter = 48) ELSE '0';
valid <= '1' WHEN (counter = 48 AND r_fixed = r_out) ELSE '0';

    Here's the entire code for that example, but the above part is the important part: 

    
library ieee;
use ieee.std_logic_1164.all;
entity crc_control is
	port
	(
		main_clk,main_reset: in std_logic;
		main_u: in std_logic;
		compute_enable: in std_logic;
		check,valid: out std_logic
	);
end crc_control;
architecture structure of crc_control is
	signal r_out: std_logic_vector(15 downto 0);
	signal r_fixed: std_logic_vector(15 downto 0);
	signal counter: integer range 0 to 48;
		
	component crc_register
	port
	(
		clk,reset: in std_logic;
		u: in std_logic;
		x: out std_logic_vector(15 downto 0)
	);
	end component;
	
	component lpmconstant
	PORT
	(
		result		: OUT STD_LOGIC_VECTOR (15 DOWNTO 0)
	);
	end component;
	
begin
	process(main_clk,main_reset,main_u)
	begin
		if(main_reset='1') then
			counter <= 0;
		elsif(main_clk'event and main_clk='1' and counter < 48) then
			counter <= counter + 1;
		end if;
	end process;
	
stage0: crc_register port map(main_clk AND compute_enable,main_reset,main_u,r_out);
stage1: lpmconstant port map(r_fixed);
check <= '1' WHEN (counter = 48) ELSE '0';
valid <= '1' WHEN (counter = 48 AND r_fixed = r_out) ELSE '0';
	
end structure;
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    16 years ago

    It's not clear, which signals are "synthesized away" and what you're actually missing. Is it in Quartus Simulator tool? There are different ways to keep logic cells and registers, synthesis attributes are one, virtual pins another. It may be also the case, that you are trying to access a signal in the wrong place, e.g. a register, that exists in a component has to be accessed there rather than in the top entity. Quartus Simulator and also Signal Tap II aren't very smart in this respect.

  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    16 years ago

    Thanks for the great tips guys. The signals I was losing were: 

    
	signal r_out: std_logic_vector(15 downto 0);
	signal r_fixed: std_logic_vector(15 downto 0);

    so I just made output pins so that I would have to be able to read them.

    The reason I fed the clock into the port map is because the component that I made needs a clock signal. I AND it with something so that it's only active when I want it to be, instead of all of the time. I shouldn't have had main_u in the sensitivity list but I'm not sure why I would want the counter in the sensitivity list. Since the counter is changing in the process, depending on the reset and clock, why would I want it in the sensitivity list?

    THanks for the help again guys. I should tell you that the code does work right now.
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    16 years ago

    --- Quote Start ---

    The reason I fed the clock into the port map is because the component that I made needs a clock signal. I AND it with something so that it's only active when I want it to be, instead of all of the time.

    --- Quote End ---

    This is the bad design I was talking about. It can cause all sorts of problems. The best thing to do is ask for a separate clock enable port, instead of passing the clock through an and gate.

    --- Quote Start ---

    I shouldn't have had main_u in the sensitivity list but I'm not sure why I would want the counter in the sensitivity list. Since the counter is changing in the process, depending on the reset and clock, why would I want it in the sensitivity list?

    --- Quote End ---

    Because of the way you have the code, counter should be in the sensitivity list (I bet if you check through the warnings, this will come up as an incomplete sensitivity list warning).

    what you have could be interpreted by the synthesisor by anding the clock and (counter < 48) together, which is not what you want, what you are best writing to avoid this is: 

    
...
if rising_edge(clk) then
  if counter < 48 then
    counter <= counter + 1;
  end if;
end if;
....
    It then garantees that the counter < 48 is the input to the clock enable port on a register, rather than creating logic as the input to a register. This can be clocked alot faster.
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    16 years ago

    So you're saying instead of this: 

    
stage0: crc_register port map(main_clk AND compute_enable,main_reset,main_u,r_out);

    I should just have this: 

    
stage0: crc_register port map(counter,main_reset,main_u,r_out);

    where I update counter like you had said? What effect will passing an integer instead of a bit have on my register? Also, I'm still not sure I understand why counter needs to be in the sensitivity list. Isn't a sensitivity list just a list of the things you want to trigger the process when they change? If counter is changing inside the process won't it trigger the process again unnecessarily?
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    16 years ago

    --- Quote Start ---

    So you're saying instead of this: 

    
stage0: crc_register port map(main_clk AND compute_enable,main_reset,main_u,r_out);
    I should just have this: 

    
stage0: crc_register port map(counter,main_reset,main_u,r_out);
    where I update counter like you had said? What effect will passing an integer instead of a bit have on my register?

    --- Quote End ---

    No. what you should do is modify crc_counter so that it has a clock port and a clock enable port. Inside the crc, you do someting like this: 

    
crc_reg_proc : process(clk, reset)
begin
  if rising_edge(clk) then
    if clock_enable = '1' then
      ....register something
    end if;
  end if;
end process;
    You then connect the clock_enable input to the compute_enable like you were doing before. what you had before would work, but not safely or reliably, and would be prone to timing problems.

    --- Quote Start ---

    Also, I'm still not sure I understand why counter needs to be in the sensitivity list. Isn't a sensitivity list just a list of the things you want to trigger the process when they change? If counter is changing inside the process won't it trigger the process again unnecessarily?

    --- Quote End ---

    Yes it is a list of things you want it to be sensitive to, but this list is only useful in a simulator. In a synthesisor it ignores the list, and works out what the logic is from the code. This way, it may AND together the clock and the counter check because of what you have written, which leads you to timing issues, when you had no problems in simulation. If you put counter in the list, it wont make any difference to simulation in this case because it needs a rising edge of the clock with the (counter < 48) at the same time, but it is a point to bare in mind for the future. For your code, the synthesisor will probably work out that what you actually mean is a clock enable, but the way you have written it is not the recommeneded coding style (as I put in the previous post). Its quite common to see people just put a couple of things in the sensitivity list then wonder why their hardware doesnt behave like the simulation.