Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > VHDL > procedure problem

Reply
Thread Tools

procedure problem

 
 
Thomas Heller
Guest
Posts: n/a
 
      12-11-2012
I have a statemachine that needs to maintain each current state for a
certain time, then switch to another state.
To save tedious typing, I wrote a helper procedure that I call
with
- the state signal instance,
- the next state value,
- the counter signal instance which is used to count the remaining
clock cycles,
- and the number of clock cycles to wait.

Here is the procedure:

procedure advance_state (
signal state : inout state_type;
constant next_state : in state_type;
signal counter : inout integer range 0 to 1000;
constant dt : in integer)
is
begin
if counter = dt then
state <= next_state;
counter <= 0;
end if;
end advance_state;

and here a snippet of the state machine code:

process(clock)
begin
if rising_edge(clock) then
case state is
when IDLE =>
if we = '1' then
state <= START_A;
counter <= 0;
end if;

when START_A => advance_state(state, START_B, counter, 2000);
....
when START_B => advance_state(state, START_C, counter, 2000);
....
when START_C => advance_state(state, WRITE_A, counter, 2000);
....

Code like this works fine when synthesized in ISE14.1 for a Spartan6
device, but it does not work for Spartan3.

Does anyone see a problem with the above?
Are there better ways to implement a state machine with state-duration?

Thanks,
Thomas
 
Reply With Quote
 
 
 
 
Pontus
Guest
Posts: n/a
 
      12-11-2012
I noticed that

signal counter : inout integer range 0 to 1000;

and

advance_state(state, START_B, counter, 2000);

2000 is not in range 0 to 1000, does this compile?

Where do you increment the counter?

-- Pontus
 
Reply With Quote
 
 
 
 
Thomas Heller
Guest
Posts: n/a
 
      12-11-2012
Am 11.12.2012 16:33, schrieb Pontus:
> I noticed that
>
> signal counter : inout integer range 0 to 1000;
>
> and
>
> advance_state(state, START_B, counter, 2000);
>
> 2000 is not in range 0 to 1000, does this compile?


This is an error that I made when copying the code into
the message. I tried to simplify it a bit.
Yes, it does compile, and, as I said, it does even work
for Spartan 6.

> Where do you increment the counter?


The counter in incremented on every rising_edge(clock)
elsewhere in the process.

Thomas

 
Reply With Quote
 
Christopher Felton
Guest
Posts: n/a
 
      12-11-2012
On 12/11/2012 10:22 AM, Thomas Heller wrote:
> Am 11.12.2012 16:33, schrieb Pontus:
>> I noticed that
>>
>> signal counter : inout integer range 0 to 1000;
>>
>> and
>>
>> advance_state(state, START_B, counter, 2000);
>>
>> 2000 is not in range 0 to 1000, does this compile?

>
> This is an error that I made when copying the code into
> the message. I tried to simplify it a bit.
> Yes, it does compile, and, as I said, it does even work
> for Spartan 6.
>
>> Where do you increment the counter?

>
> The counter in incremented on every rising_edge(clock)
> elsewhere in the process.
>
> Thomas
>


When you say compile, it synthesizes without errors and passes PaR with
no timing errors (timing errors don't error out the software). Unless
it is a resource issue, I can't see why a S3 vs. S6 would make a difference.

Chris
 
Reply With Quote
 
Thomas Heller
Guest
Posts: n/a
 
      12-11-2012
Am 11.12.2012 17:53, schrieb Christopher Felton:
> On 12/11/2012 10:22 AM, Thomas Heller wrote:
>> Am 11.12.2012 16:33, schrieb Pontus:
>>> I noticed that
>>>
>>> signal counter : inout integer range 0 to 1000;
>>>
>>> and
>>>
>>> advance_state(state, START_B, counter, 2000);
>>>
>>> 2000 is not in range 0 to 1000, does this compile?

>>
>> This is an error that I made when copying the code into
>> the message. I tried to simplify it a bit.
>> Yes, it does compile, and, as I said, it does even work
>> for Spartan 6.
>>
>>> Where do you increment the counter?

>>
>> The counter in incremented on every rising_edge(clock)
>> elsewhere in the process.
>>
>> Thomas
>>

>
> When you say compile, it synthesizes without errors and passes PaR with
> no timing errors (timing errors don't error out the software). Unless
> it is a resource issue, I can't see why a S3 vs. S6 would make a
> difference.


Yes, no errors, and it fits timing.
I assume they use different parsers/map/par for s3 and s6.
Maybe that explains the issue.

How do others write statemachines with certain durations for the states?

Thomas
 
Reply With Quote
 
Christopher Felton
Guest
Posts: n/a
 
      12-11-2012
On 12/11/2012 11:52 AM, Thomas Heller wrote:
> Am 11.12.2012 17:53, schrieb Christopher Felton:
>> On 12/11/2012 10:22 AM, Thomas Heller wrote:
>>> Am 11.12.2012 16:33, schrieb Pontus:
>>>> I noticed that
>>>>
>>>> signal counter : inout integer range 0 to 1000;
>>>>
>>>> and
>>>>
>>>> advance_state(state, START_B, counter, 2000);
>>>>
>>>> 2000 is not in range 0 to 1000, does this compile?
>>>
>>> This is an error that I made when copying the code into
>>> the message. I tried to simplify it a bit.
>>> Yes, it does compile, and, as I said, it does even work
>>> for Spartan 6.
>>>
>>>> Where do you increment the counter?
>>>
>>> The counter in incremented on every rising_edge(clock)
>>> elsewhere in the process.
>>>
>>> Thomas
>>>

>>
>> When you say compile, it synthesizes without errors and passes PaR with
>> no timing errors (timing errors don't error out the software). Unless
>> it is a resource issue, I can't see why a S3 vs. S6 would make a
>> difference.

>
> Yes, no errors, and it fits timing.
> I assume they use different parsers/map/par for s3 and s6.
> Maybe that explains the issue.


It shouldn't have a different parser, I believe the Xilinx synthesis is
device aware but typically synthesis will synthesize to generic
primitives (gates) and then the mapper will translate to device
specific. I would expect only the mapper to be significantly different.
But there should be some kind of warning or error that there are not
enough resources etc., hmmmm.

>
> How do others write statemachines with certain durations for the states?


If you mean simple time delays, I usually have a local counter,
something like your example.. Other times I will make a generic state
delay, where the return state (next state out of the delay state) is a
register and the delay is a register. Then it is like calling a
function in CPU programming, you have to program the delay and return
state before transitioning to the delay state. It is a reusable delay
with the extra overhead of the programmable delay (expiration) and
return state. And normally it is three states, delay_start, delay_wait,
delay_end.


Regards,
Chris

Regards,
Chris
 
Reply With Quote
 
Gabor
Guest
Posts: n/a
 
      12-11-2012
Christopher Felton wrote:
> On 12/11/2012 11:52 AM, Thomas Heller wrote:
>> Am 11.12.2012 17:53, schrieb Christopher Felton:
>>> On 12/11/2012 10:22 AM, Thomas Heller wrote:
>>>> Am 11.12.2012 16:33, schrieb Pontus:
>>>>> I noticed that
>>>>>
>>>>> signal counter : inout integer range 0 to 1000;
>>>>>
>>>>> and
>>>>>
>>>>> advance_state(state, START_B, counter, 2000);
>>>>>
>>>>> 2000 is not in range 0 to 1000, does this compile?
>>>>
>>>> This is an error that I made when copying the code into
>>>> the message. I tried to simplify it a bit.
>>>> Yes, it does compile, and, as I said, it does even work
>>>> for Spartan 6.
>>>>
>>>>> Where do you increment the counter?
>>>>
>>>> The counter in incremented on every rising_edge(clock)
>>>> elsewhere in the process.
>>>>
>>>> Thomas
>>>>
>>>
>>> When you say compile, it synthesizes without errors and passes PaR with
>>> no timing errors (timing errors don't error out the software). Unless
>>> it is a resource issue, I can't see why a S3 vs. S6 would make a
>>> difference.

>>
>> Yes, no errors, and it fits timing.
>> I assume they use different parsers/map/par for s3 and s6.
>> Maybe that explains the issue.

>
> It shouldn't have a different parser, I believe the Xilinx synthesis is
> device aware but typically synthesis will synthesize to generic
> primitives (gates) and then the mapper will translate to device
> specific. I would expect only the mapper to be significantly different.
> But there should be some kind of warning or error that there are not
> enough resources etc., hmmmm.
>
>>
>> How do others write statemachines with certain durations for the states?

>
> If you mean simple time delays, I usually have a local counter,
> something like your example.. Other times I will make a generic state
> delay, where the return state (next state out of the delay state) is a
> register and the delay is a register. Then it is like calling a
> function in CPU programming, you have to program the delay and return
> state before transitioning to the delay state. It is a reusable delay
> with the extra overhead of the programmable delay (expiration) and
> return state. And normally it is three states, delay_start, delay_wait,
> delay_end.
>
>
> Regards,
> Chris
>
> Regards,
> Chris

S6 and newer devices use the "new parser" for XST. Older devices
do not, unless you add "-use_new_parser yes" to the command line
options for XST. This could very well explain the difference in
behavior between the two. You need ISE 12 or newer to use the
new parser.

-- Gabor
 
Reply With Quote
 
KJ
Guest
Posts: n/a
 
      12-12-2012
On Tuesday, December 11, 2012 9:52:41 AM UTC-5, Thomas Heller wrote:

- The state machine has no reset. There is nothing to get it into a valid state which then can be stepped through once reset completes. That alone would cause it to work in simulation but not always work in actual hardware.

- Signal state is an output only of the procedure. That shouldn't matter for the problem you're seeing (which is not clear what exactly is the problem...you didn't say).

- Since you asked, I would've written it like this...

if rising_edge(clock) then
if (reset = '1') then
state <= idle; -- I presume
counter = 0;
elsif (counter = dt) then
counter = 0;
case state is
when START_A => state <= START_B;
...
end case;
else
counter <= counter + 1;
end if;
end if;

Not that what you have is wrong, but you asked how others would write it that are 'better'. Whether you think yours is better or not is up to you based on your own criteria.

Kevin Jennings
 
Reply With Quote
 
Christopher Felton
Guest
Posts: n/a
 
      12-12-2012
On 12/11/2012 3:37 PM, Gabor wrote:
> Christopher Felton wrote:
>> On 12/11/2012 11:52 AM, Thomas Heller wrote:
>>> Am 11.12.2012 17:53, schrieb Christopher Felton:
>>>> On 12/11/2012 10:22 AM, Thomas Heller wrote:
>>>>> Am 11.12.2012 16:33, schrieb Pontus:
>>>>>> I noticed that
>>>>>>
>>>>>> signal counter : inout integer range 0 to 1000;
>>>>>>
>>>>>> and
>>>>>>
>>>>>> advance_state(state, START_B, counter, 2000);
>>>>>>
>>>>>> 2000 is not in range 0 to 1000, does this compile?
>>>>>
>>>>> This is an error that I made when copying the code into
>>>>> the message. I tried to simplify it a bit.
>>>>> Yes, it does compile, and, as I said, it does even work
>>>>> for Spartan 6.
>>>>>
>>>>>> Where do you increment the counter?
>>>>>
>>>>> The counter in incremented on every rising_edge(clock)
>>>>> elsewhere in the process.
>>>>>
>>>>> Thomas
>>>>>
>>>>
>>>> When you say compile, it synthesizes without errors and passes PaR with
>>>> no timing errors (timing errors don't error out the software). Unless
>>>> it is a resource issue, I can't see why a S3 vs. S6 would make a
>>>> difference.
>>>
>>> Yes, no errors, and it fits timing.
>>> I assume they use different parsers/map/par for s3 and s6.
>>> Maybe that explains the issue.

>>
>> It shouldn't have a different parser, I believe the Xilinx synthesis
>> is device aware but typically synthesis will synthesize to generic
>> primitives (gates) and then the mapper will translate to device
>> specific. I would expect only the mapper to be significantly
>> different. But there should be some kind of warning or error that
>> there are not enough resources etc., hmmmm.
>>
>>>
>>> How do others write statemachines with certain durations for the states?

>>
>> If you mean simple time delays, I usually have a local counter,
>> something like your example.. Other times I will make a generic state
>> delay, where the return state (next state out of the delay state) is a
>> register and the delay is a register. Then it is like calling a
>> function in CPU programming, you have to program the delay and return
>> state before transitioning to the delay state. It is a reusable delay
>> with the extra overhead of the programmable delay (expiration) and
>> return state. And normally it is three states, delay_start,
>> delay_wait, delay_end.
>>
>>
>> Regards,
>> Chris
>>
>> Regards,
>> Chris

> S6 and newer devices use the "new parser" for XST. Older devices
> do not, unless you add "-use_new_parser yes" to the command line
> options for XST. This could very well explain the difference in
> behavior between the two. You need ISE 12 or newer to use the
> new parser.
>
> -- Gabor


Seems odd to me, wonder what the rational was. That is, a new release
of software to use a different parser (frontend of synthesis) for
different devices.

Regards,
Chris
 
Reply With Quote
 
Gabor
Guest
Posts: n/a
 
      12-12-2012
Christopher Felton wrote:
> On 12/11/2012 3:37 PM, Gabor wrote:
>> Christopher Felton wrote:
>>> On 12/11/2012 11:52 AM, Thomas Heller wrote:
>>>> Am 11.12.2012 17:53, schrieb Christopher Felton:
>>>>> On 12/11/2012 10:22 AM, Thomas Heller wrote:
>>>>>> Am 11.12.2012 16:33, schrieb Pontus:
>>>>>>> I noticed that
>>>>>>>
>>>>>>> signal counter : inout integer range 0 to 1000;
>>>>>>>
>>>>>>> and
>>>>>>>
>>>>>>> advance_state(state, START_B, counter, 2000);
>>>>>>>
>>>>>>> 2000 is not in range 0 to 1000, does this compile?
>>>>>>
>>>>>> This is an error that I made when copying the code into
>>>>>> the message. I tried to simplify it a bit.
>>>>>> Yes, it does compile, and, as I said, it does even work
>>>>>> for Spartan 6.
>>>>>>
>>>>>>> Where do you increment the counter?
>>>>>>
>>>>>> The counter in incremented on every rising_edge(clock)
>>>>>> elsewhere in the process.
>>>>>>
>>>>>> Thomas
>>>>>>
>>>>>
>>>>> When you say compile, it synthesizes without errors and passes PaR
>>>>> with
>>>>> no timing errors (timing errors don't error out the software). Unless
>>>>> it is a resource issue, I can't see why a S3 vs. S6 would make a
>>>>> difference.
>>>>
>>>> Yes, no errors, and it fits timing.
>>>> I assume they use different parsers/map/par for s3 and s6.
>>>> Maybe that explains the issue.
>>>
>>> It shouldn't have a different parser, I believe the Xilinx synthesis
>>> is device aware but typically synthesis will synthesize to generic
>>> primitives (gates) and then the mapper will translate to device
>>> specific. I would expect only the mapper to be significantly
>>> different. But there should be some kind of warning or error that
>>> there are not enough resources etc., hmmmm.
>>>
>>>>
>>>> How do others write statemachines with certain durations for the
>>>> states?
>>>
>>> If you mean simple time delays, I usually have a local counter,
>>> something like your example.. Other times I will make a generic state
>>> delay, where the return state (next state out of the delay state) is a
>>> register and the delay is a register. Then it is like calling a
>>> function in CPU programming, you have to program the delay and return
>>> state before transitioning to the delay state. It is a reusable delay
>>> with the extra overhead of the programmable delay (expiration) and
>>> return state. And normally it is three states, delay_start,
>>> delay_wait, delay_end.
>>>
>>>
>>> Regards,
>>> Chris
>>>
>>> Regards,
>>> Chris

>> S6 and newer devices use the "new parser" for XST. Older devices
>> do not, unless you add "-use_new_parser yes" to the command line
>> options for XST. This could very well explain the difference in
>> behavior between the two. You need ISE 12 or newer to use the
>> new parser.
>>
>> -- Gabor

>
> Seems odd to me, wonder what the rational was. That is, a new release
> of software to use a different parser (frontend of synthesis) for
> different devices.
>
> Regards,
> Chris


The rationale is very simple actually. They didn't want to run
regression testing on all of the older device families. They
give you a switch to turn on (or off) the new parser regardless of
the FPGA family, but if you decide to use the new parser on an
older device you should not expect support.

-- Gabor
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
procedure as argument in procedure AlexWare VHDL 2 10-23-2009 09:14 AM
'Procedure or function <stored procedure name> has too many arguments specified',,,ARGH! Mike P ASP .Net 0 06-19-2006 01:19 PM
Interesting Stored Procedure Problem.. Bilbo ASP .Net 3 11-20-2003 09:31 PM
Stored Procedure/Parameter problem ElmoWatson ASP .Net 1 08-06-2003 01:19 AM
Stored Procedure Problem Leon Shaw ASP .Net 1 07-29-2003 09:19 AM



Advertisments