Vhdl beginner - Signal assignment doesn't work

[Beware]

Hi,

i'm make some simples VHDL design to keep alive my knowledges (i
try ).
But, i've got a little problem in my code, it's a shift register.

When i simulate this design, i haven't the execpted value on the
signal s1 (out) because i've not the good value on my internal signal
(it doesn't take the value of the input as it would be to).

I hope i'm clear in my explications.

Here my design :

library ieee;
use ieee.std_logic_1164.all;


--entity
entity accushift32 is

port (
e1 : in std_logic_vector(31 downto 0);
rst : in std_logic;
clk : in std_logic;
s1 : out std_logic);

end accushift32;

--design
architecture d_accushift32 of accushift32 is

signal internal : std_logic_vector(31 downto 0) := (others => '0');

begin -- d_accushift32

s1 <= internal(0);

process (clk, rst)
begin -- process
if rst = '0' then -- asynchronous reset (active
low)
internal <= e1;

elsif clk'event and clk = '1' then -- rising clock edge
internal <= '0' & internal(31 downto 1);

end if;
end process;

end d_accushift32;

Thanks to all of you for your answers (or your comments)

 

[Beware]

On 19 mai, 11:19, Jonathan Bromley <[email protected]>
wrote:

Hi Jonathan

There is one important problem with your design, which
I will explain later.  However, I do not understand why
you have trouble with simulation.  The design is good
VHDL and should simulate OK.  Can you show us the
testbench code you used to simulate it?  I guess your
problem is related to timing of signals in the testbench.

Thank you for your answer. You're right. I've changed my code and now
it's work pretty well.
I have to remember this for the next design.

Here's my new code :

library ieee;
use ieee.std_logic_1164.all;


--entity
entity accushift32 is

port (
e1 : in std_logic_vector(31 downto 0);
rst : in std_logic;
clk : in std_logic;
load : in std_logic;
s1 : out std_logic);

end accushift32;

--design
architecture d_accushift32 of accushift32 is

signal internal : std_logic_vector(31 downto 0) := (others => '0');

begin -- d_accushift32

s1 <= internal(0);

process (clk, rst, load)
begin -- process
if rst = '0' then -- asynchronous reset (active
low)
internal <= (others => '0');

elsif clk'event and clk = '1' then -- rising clock edge

if load = '1' then
internal <= e1;
else
internal <= '0' & internal(31 downto 1);
end if;

end if;

end process;

end d_accushift32;


Thanks.

 

[Beware]

On 19 mai, 14:53, Jonathan Bromley <[email protected]>
wrote:

hi,

Cool.  Two points to look at:

(1)
Get into the habit of using "rising_edge(clk)"
in place of "clk'event and clk='1'".  It's
easier to write and easier to read.

Ok, i'll remember it.
In fact, i use Emacs and its VHDL-mode to write my files. So i use the
shortcuts for write the vhdl and i don't rewrite it (i'm wrong i
know).

(2)
The process sensitivity list should be
  process(clk, rst)
You do NOT need "load" in the sensitivity list, because
it is sampled only on the active clock edge.  In VHDL
it will not break your simulation, but it will waste
simulation time and it may give synthesis warnings.
Never include synchronous inputs in the sensitivity list;
it should have ONLY clock and asynchronous reset.

Allright, i delete the 'load' entry in the sensitive list.

Thanks for all.

 

[Andy]

A few more recommendations for your code:

If you have a reset function, you should not initialize the register
signal in its declaration. The initialization makes it harder to test
whether your reset is working.

You can use the 'range attribute when declaring a signal that needs to
be the same range as a port:

signal internal : std_logic_vector(e1'range); -- same range as e1

Given your shift logic, this works because the port has the correct
range (direction & bounds) too.

Sometimes the port may not have the correct index bounds and/or
direction (it may be an unconstrained port, which take's its range
from the signal bound to the port for that instance):

signal internal : std_logic_vector(e1'length-1 downto 0); --
normalized range

Andy

 

[Martin Thompson]

On 19 mai, 14:53, Jonathan Bromley <[email protected]>
wrote:

hi,

Ok, i'll remember it.
In fact, i use Emacs and its VHDL-mode to write my files. So i use the
shortcuts for write the vhdl and i don't rewrite it (i'm wrong i
know).

You can change a setting in VHDL-mode to tell it to use rising_edge()
as well:

Click
VHDL..Options..Template..Sequential Process..Clock Edge Condition

Don't forget to save it to your .emacs file as well.
VHDL..Options..Save Options

While you're in there there's loads of other customisations as well

Cheers,
Martin

 

[Beware]

Thanks all for your answer.

@Andy : I didn't knew the 'range' option to describe a signal, it's
very usefull. Can i use them for describe all my internal signal? The
code will not become harder to read?

@Martin : Thanks for the option. You're are right, there's a lot of
options to customize emacs for the VHDL.

 

[Andy]

Thanks all for your answer.

@Andy : I didn't knew the 'range' option to describe a signal, it's
very usefull. Can i use them for describe all my internal signal? The
code will not become harder to read?

@Martin : Thanks for the option. You're are right, there's a lot of
options to customize emacs for the VHDL.

I use range/length attributes to make it clear that one signal/
variable's size needs to be related to another. If it just happens
that they are the same size, but do not need to be, I do not use those
attributes. It's just another hint about the code for when I'm reading
it weeks/months/years after I wrote it. Use of those attributes also
makes it easier to modify later on, since I can change the size of one
or a few signals/variables, and others will automatically adjust as
needed. Thinking about maintaining a piece of code, even while you are
writing it the first time, will pay for itself eventually.

Another example of this would be to change the shift assignment to
something like:

internal <= '0' & internal(internal'left downto 1);

There are other ways to accomplish the shift, but this is closest to
what you originally wrote.

This way, all you need to do to change the length of the shift
register is to change the length of the e1 port.

Andy

 

[Beware]

I use range/length attributes to make it clear that one signal/
variable's size needs to be related to another. If it just happens
that they are the same size, but do not need to be, I do not use those
attributes. It's just another hint about the code for when I'm reading
it weeks/months/years after I wrote it. Use of those attributes also
makes it easier to modify later on, since I can change the size of one
or a few signals/variables, and others will automatically adjust as
needed. Thinking about maintaining a piece of code, even while you are
writing it the first time, will pay for itself eventually.

Another example of this would be to change the shift assignment to
something like:

  internal <= '0' & internal(internal'left downto 1);

There are other ways to accomplish the shift, but this is closest to
what you originally wrote.

This way, all you need to do to change the length of the shift
register is to change the length of the e1 port.

Andy

Hi Andy,

Thank you for all your hints and your eplications.
It's now easy to modify the length of an input by using 'length,
'range or 'left options for internals signals.


I've one another question. I'm using GHDL for compilation and
simulation add to Gkwave. For compilation GHDL use the ieee libraries
and VHDL-1993(c) standard, i have no errors or warning. But if i user
VHDL-2000 standard, i have one error in my testbench file in the
declaration of my component.
The error :
testbench/tb_multi.vhd:36:3:warning: component instance
"multiplieur1" is not bound
testbench/tb_multi.vhd:11:14:warning: (in default configuration of
tb_multiplieur(d_tb_multiplieur))

The code :

library ieee;
use ieee.std_logic_1164.all;

--entity
entity tb_multiplieur is
end tb_multiplieur;

--design
architecture d_tb_multiplieur of tb_multiplieur is

component multiplieur
port (
op1 : in std_logic_vector(31 downto 0);
op2 : in std_logic_vector(31 downto 0);
res : out std_logic_vector(63 downto 0);
h : in std_logic;
raz : in std_logic;
load : in std_logic);
end component;

signal sortie : std_logic_vector(res'range) := (others => '0');
signal A : std_logic_vector(op1'range) := (others => '0');
signal B : std_logic_vector(op2'range) := (others => '0');
signal clk : std_logic := '0';
signal rst : std_logic := '0';
signal ld : std_logic := '0';

begin -- d_tb_multiplieur

multiplieur1:multiplieur
port map (
op1 => A,
op2 => B,
res => sortie,
h => clk,
load => ld,
raz => rst);

resetrocess
begin -- process reset
wait for 10 ns;
rst <= not rst;
wait for 1000 ms;
end process reset;

horloge: process
begin -- process horloge
wait for 10 ns;
clk <= not clk;
end process horloge;

inputs: process
begin -- process inputs
A <= x"A5A5A5A5";
B <= x"5A5A5A5A";
wait for 15 ns;
ld <= '1';
wait for 20 ns;
ld <= not ld;
wait for 900 ns;

end process inputs;

end d_tb_multiplieur;


But i don't know if the error is due to GHDL or to my code.

 

[Andy]

GHDL is just complaining that you have not compiled anything for the
multiplier component to bind to. Compile the entity/arch for
multiplier with the same signature (name and port list) and it should
get rid of the warning. If the entity signature is not the same as
that of the component, then you will have to use a configuration to
bind the entity/arch to the component instantiation.

You can also directly instantiate entity/architectures under vhdl'93
and later:

label: entity work.entity_name(architecture_name)...

The architecture name is optional.

Beware, this will result in errors (not warnings) if you have not
already compiled the entity (and architecture if the instantiation
specified it).

I use entity instantiation for stuff that I write, but components for
primitives from other libraries, especially when synthesis recognizes
the component automatically, but simulation needs to compile/link the
model for the primitive.

Andy

 

[Beware]

GHDL is just complaining that you have not compiled anything for the
multiplier component to bind to. Compile the entity/arch for
multiplier with the same signature (name and port list) and it should
get rid of the warning. If the entity signature is not the same as
that of the component, then you will have to use a configuration to
bind the entity/arch to the component instantiation.

You can also directly instantiate entity/architectures under vhdl'93
and later:

label: entity work.entity_name(architecture_name)...

The architecture name is optional.

Beware, this will result in errors (not warnings) if you have not
already compiled the entity (and architecture if the instantiation
specified it).

I use entity instantiation for stuff that I write, but components for
primitives from other libraries, especially when synthesis recognizes
the component automatically, but simulation needs to compile/link the
model for the primitive.

Andy

Hi,

Sorry but i still don't understand what i do wrong.
When i write the instancation of "multiplieur" in my testbench file, i
use the same name of the "multiplieur" entity and the same list port.

 

[Andy]

Hi,

Sorry but i still don't understand what i do wrong.
When i write the instancation of "multiplieur" in my testbench file, i
use the same name of the "multiplieur" entity and the same list port.- Hide quoted text -

- Show quoted text -

I think your instantiation is correct, you just have not yet compiled
an entity and/or architecture for the multiplier itself. There isn't a
model for multiplier yet that the simulator can bind to your component
instance.

Andy