Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > VHDL > combinational loops

Reply
Thread Tools

combinational loops

 
 
alb
Guest
Posts: n/a
 
      09-24-2013
Hi Mike,

On 23/09/2013 18:51, Mike Treseler wrote:
> On Friday, September 20, 2013 3:42:31 AM UTC-7, alb wrote:
>>> It sim'ed ok, both pre/post-synthesis and post-layout.

>
>
> In an init,update,output single process design using variables,
> you can ignore warnings about variable reuse if synthesis completes without error and sim is ok.


there are a couple of things that do not make me feel happy even if sim
is ok:

1. the amount of logic inferred is exceeding my predictions (this is
just an FSM with some registers and a couple of counters and it takes
12% of a 250k gates FPGA!).

2. in the lab it does not work.

Concerning 1. I'll need to investigate the RTL and see if there's
something wrong going on (the Synplify Pro RTL viewer should be
sufficient to do so).

About 2., this is an FSM to read out a 1-wire bus in an infinite loop
and I believe that at power up the 'Reset' on the 1-wire is sent too
early (the line is still not stable at 'H') and the presence pulse is
not sampled properly, causing the FSM to loop indefinitely in a state
waiting for the presence bit to show up. I think this can be easily
fixed waiting for the line to be stable first.


 
Reply With Quote
 
 
 
 
alb
Guest
Posts: n/a
 
      09-24-2013
On 23/09/2013 17:11, rickman wrote:
[]
>> A latch is inferred if you write a process where an output is not
>> assigned under all possible input conditions:
>>
>> <code>
>> process (clk, rst)
>> begin
>> if rst = '1' then
>> nRD_s<= '1'; -- nRD_s is a signal instead of a variable
>> foo_s<= nRD_s; -- foo_s is not assigned under all possible inputs
>> -- of the process therefore a latch is needed to
>> -- retain its value.
>> elsif rising_edge(clk) then
>> if<condition> then
>> nRD_s<= not nRD_s;
>> end if;
>> end if;
>> end process;
>> </code>
>>
>> So my code should not infer a latch (and indeed it does not).

>
> Your premise is not complete. A latch is inferred as well if you create
> a feedback loop. nRD_v := not nRD_v; creates feedback if it is not
> registered.


On a clocked process the assignment you quote is perfectly legal and
does not create any latch (it may still create a feedback mux if nRD_v
is not reset).

> It appears it should be registered however. So the
> questions are,
>
> 1) Is it registered?


yes

>
> 2) If it is registered, how is the message being generated?


Assuming the message you refer to is the OP's note issued by Designer,
that is an unfair question to ask since I asked it first

> BTW, you *don't* assign a value to nRD_v under all conditions.
>
> when IDLE =>
> cnt_v := 0;
> state_v := SET_CLKRATE;
> when READ_BGN =>
> state_v := READ_END; -- this is one clock cycle
> nRD_v := not nRD_v;


I know the code is a bit long to go through, but the 'autorun' procedure
is executed during 'update_reg', which is called in the 'if
rising_edge(clk)' branch of the -only - process.

The variable is also properly initialized in bus_init, called during
init_regs, in the 'other' branch of the clocked process. Therefore the
variable nRD_v *is* assigned under all possible conditions which will
trigger the process to run.

[]
>> Believe me, if I knew this stuff 'pretty well' I won't be asking .
>> I'm actually exploring the usage of variables to a greater extent,
>> together with a new 'procedural' approach and I'm afraid I'm misusing it
>> somehow.

>
> Yes, I have never used variables so much myself. That's why I missed
> that nRD_v was a variable. I missed both the := and the _v ... duh!


I keep the _v reminder in the name which may seem redundant since it is
clear that I cannot assign a variable with a non-blocking assignment.
But generally I tend to use no reminders for the entity ports to
increase readability and therefore I find it natural to use the _v
internally.

> I also don't use procedures so much. I should try to work with them
> more so I understand them better. There are times when they are useful
> for decomposition of code into reusable modules.


After having been contaminated by software development for three years I
started to see the power of using subprograms and IMO VHDL has all the
language constructs to allow a high level of abstraction, resulting in
fewer lines of code (-> less bugs [1]), greater readability and reuse.

>
>> Funny enough the code simulates just as I want it, maybe I've only been
>> lucky...

>
> *Something* is amiss. Has this error message been generated all along?
> If not, see what you changed. If it has I would say simplify the code
> to something that doesn't give the error and figure out what is different.


I can indeed start to remove bits and pieces to see what really fools
the tool, but I'm not happy about the flow. How can it be that only the
P&R has noticed the combinational loop?

[1] Halstead, Maurice H. (1977). Elements of Software Science.
Amsterdam: Elsevier North-Holland, Inc.

 
Reply With Quote
 
 
 
 
alb
Guest
Posts: n/a
 
      09-24-2013
Hi Rick,

On 23/09/2013 17:22, rickman wrote:
[]
> This is the first time you have compiled the code isn't it? So you have
> not done any testing on the code at all so far?


As I mentioned in the beginning the code has been sim'ed and it works
properly in post-synth and post-layout (even though STA shows timing
violations on unrelated parts of the code).

Unfortunately it does not work in the lab, but I think is more related
to the power-up rather than a functional problem (at power-up the 1-wire
line is pulled 'H' through a pull-up resistor and the sequence to talk
on the line starts too early).

> Where are your parameter lists for the procedures such as update_ports?


No parameter lists. All variables have global scope in the process.

This is indeed something I do not completely like about this approach
since I haven't found a way to 'protect' the access to variables and you
may end up with variable assignments all scattered, killing the benefit
of subprogram partitioning.
 
Reply With Quote
 
alb
Guest
Posts: n/a
 
      09-24-2013
Hi Andy,

On 23/09/2013 18:44, Andy wrote:
[]
> I have had problems in synthesis (synplify pro) when calling the same
> procedure under multiple conditions (e.g. states in a state machine)
> from a clocked process.


This is no good news! :-/

> Synthesizer did not complain, but the HW did not work, and the
> post-synth netlist simulation did not match the RTL simulation. The
> post-synth simulation was consistent with behavior observed in the
> lab (we could not observe the exact failure in the lab, but the
> overall results were consistent). P&R STA passed with no
> errors/warnings.


The HW does not work for me as well, but I'm afraid for a different
reason. Conversely to your case, my code sim ok, post-synth and
post-layout. P&R STA reports issues but on a different part of the design.

What about the RTL viewer? Did it show something reasonable or not?

> To work around it, I created a variable flag that was set anytime I
> needed to run the procedure, then below the fsm code (in the same
> clocked process or update_regs procedure in your case), I called the
> procedure within an if-statement when the flag was set. Note that
> since the flag was written every clock cycle before read to determine
> whether to run the procedure, the flag was combinatorial, and the
> procedure ran in the same clock cycle that set the flag.


something similar to this:

<code>
-- declarative part of global variables:
variable flag: std_logic;
-- ...
-------------------------------------------------------------------------------
procedure update_regs is
begin -- purpose: call the procedures above in the desired order
if flag then
autoread;
flag := '0'; -- reset flag.
end if;
end procedure update_regs;
-------------------------------------------------------------------------------
procedure template_v_rst is
begin -- a_rst is logically equivalent
if RST = '1' then
init_regs;
elsif rising_edge(CLK) then
flag := '1'; -- set flag.
update_regs;
end if;
update_ports;
end procedure template_v_rst;
-- ...

</code>

I'll try it and post results.

>
> I have not tried to create a boiled-down test case to submit to
> Synopsys yet.


I'll see what I can come up with, but I'm a bit under pressure to
complete and I'm not sure I'll have the time to find a test case.
 
Reply With Quote
 
alb
Guest
Posts: n/a
 
      09-24-2013
This is a correction to my previous post.

On 24/09/2013 10:19, alb wrote:
[]
>> BTW, you *don't* assign a value to nRD_v under all conditions.
>>
>> when IDLE =>
>> cnt_v := 0;
>> state_v := SET_CLKRATE;
>> when READ_BGN =>
>> state_v := READ_END; -- this is one clock cycle
>> nRD_v := not nRD_v;

>
> I know the code is a bit long to go through, but the 'autorun' procedure
> is executed during 'update_reg', which is called in the 'if
> rising_edge(clk)' branch of the -only - process.
>
> The variable is also properly initialized in bus_init, called during
> init_regs, in the 'other' branch of the clocked process. Therefore the
> variable nRD_v *is* assigned under all possible conditions which will
> trigger the process to run.


You are right, the variable nRD_v *is not* assigned under all possible
branches, but it is assigned in a clocked process therefore is
synthesized to a dff.

Sorry for the confusion.

 
Reply With Quote
 
Mike Treseler
Guest
Posts: n/a
 
      09-24-2013
On Tuesday, September 24, 2013 12:32:07 AM UTC-7, alb wrote:
>
> there are a couple of things that do not make me feel happy even if sim
> is ok:
> 1. the amount of logic inferred is exceeding my predictions (this is
> just an FSM with some registers and a couple of counters and it takes
> 12% of a 250k gates FPGA!).


Have a look on the rtl viewer. Some variable array is too large or some
block ram does not fit the device.

If you don't have a viewer, start out with the free quartus tools.

> 2. in the lab it does not work.


Start with with just the reset procedure
check the reset pulse and all static outputs.

Add a clock and counter and check that. etc.

> Concerning 1. I'll need to investigate the RTL and see if there's
> something wrong going on (the Synplify Pro RTL viewer should be
> sufficient to do so).


I never go near the bench until the RTL view is as expected.

> About 2., this is an FSM to read out a 1-wire bus in an infinite loop
> and I believe that at power up the 'Reset' on the 1-wire is sent too
> early (the line is still not stable at 'H') and the presence pulse is
> not sampled properly, causing the FSM to loop indefinitely in a state
> waiting for the presence bit to show up. I think this can be easily
> fixed waiting for the line to be stable first.


I would read a burst at at time and check sims that the bus port has Z,1,0
at the right times.

Good Luck,

-- Mike Treseler

 
Reply With Quote
 
Andy
Guest
Posts: n/a
 
      09-24-2013
Al,

I think you need flags to control calling the sub-procedures within autoread, not calling autoread itself. It looks like you would also need variablesto hold the parameters you are going to pass to some of your sub-procedures.

After reviewing your FSM though, I think you might do better with separate,smaller state machine(s) for the reusable states, and implement a hierarchical state machine. There is no point in combining all the reusable and unique states into the same FSM.

Does Synplify actually recognize your FSM as an FSM (what does it look likein the RTL viewer or FSM explorer)? Sometimes if you assign the next statefrom the contents of a register other than the current state, Synplify will not treat it as a state machine, which then excludes optimizations normally performed on FSMs.

As for my example that illuminated the synthesis problem, the procedure's logic was complex enough that any abnormalities would not have been easy to spot in RTL view. Unfortunately, Synplify unrolls functions and procedures for RTL view.

Andy
 
Reply With Quote
 
rickman
Guest
Posts: n/a
 
      09-25-2013
On 9/24/2013 4:29 AM, alb wrote:
> Hi Rick,
>
> On 23/09/2013 17:22, rickman wrote:
> []
>> This is the first time you have compiled the code isn't it? So you have
>> not done any testing on the code at all so far?

>
> As I mentioned in the beginning the code has been sim'ed and it works
> properly in post-synth and post-layout (even though STA shows timing
> violations on unrelated parts of the code).
>
> Unfortunately it does not work in the lab, but I think is more related
> to the power-up rather than a functional problem (at power-up the 1-wire
> line is pulled 'H' through a pull-up resistor and the sequence to talk
> on the line starts too early).
>
>> Where are your parameter lists for the procedures such as update_ports?

>
> No parameter lists. All variables have global scope in the process.
>
> This is indeed something I do not completely like about this approach
> since I haven't found a way to 'protect' the access to variables and you
> may end up with variable assignments all scattered, killing the benefit
> of subprogram partitioning.


Ok, at this point I am showing my ignorance of using procedures in VHDL.
I never realized that scope would work this way. In fact, this sounds
very unlike VHDL. I'm still a bit confused about a couple of things.

The procedures init_regs and update_ports may be in a clocked process,
but they are *not* in the clocked region of the clocked process and so
can generate combinatorial logic. Again, I have not looked at the code
in detail as that would require me to copy to a text file and open it in
a decent editor rather than this limited newsreader. Are you sure there
are no issues in one of those two procedures?

As to the scope issue, I don't think you *have* to use the global scope.
You can pass parameters into the procedures if you choose to. It
seems to me that using procedures provides limited advantages over
non-modular code by not using parameters.

--

Rick
 
Reply With Quote
 
alb
Guest
Posts: n/a
 
      09-25-2013
Hi Rick,

On 25/09/2013 06:52, rickman wrote:
[]
>>> Where are your parameter lists for the procedures such as update_ports?

>>
>> No parameter lists. All variables have global scope in the process.
>>
>> This is indeed something I do not completely like about this approach
>> since I haven't found a way to 'protect' the access to variables and you
>> may end up with variable assignments all scattered, killing the benefit
>> of subprogram partitioning.

>
> Ok, at this point I am showing my ignorance of using procedures in VHDL.
> I never realized that scope would work this way. In fact, this sounds
> very unlike VHDL. I'm still a bit confused about a couple of things.


There's only one process in the entire entity. Variables have local
scope in the process, but since there's nothing except one process they
can be considered 'global' in the entity. Moreover every procedure
defined within the process has access to all local variables as well.

> The procedures init_regs and update_ports may be in a clocked process,
> but they are *not* in the clocked region of the clocked process and so
> can generate combinatorial logic.


the init_regs procedure is called in the reset branch of the clocked
process, therefore will run to 'reset' all initial states of any locally
defined variable.

In update_ports all appropriate variables are 'wired' to ports (no
signals). Meaning that every time the process is triggered they will be
assigned to a port. Since update_ports is only called when the process
is triggered I'm not sure if you can have an output port which is mapped
to a pure combinatorial logic of a set of inputs.

> Again, I have not looked at the code
> in detail as that would require me to copy to a text file and open it in
> a decent editor rather than this limited newsreader. Are you sure there
> are no issues in one of those two procedures?


Yeah I apologize for the code format, I try to keep everything under 80
characters for this very purpose but I'm not always so strict :-/

I'm trying to remove most of the stuff and synthesize piece by piece.
Honestly I do not see how the init_regs and update_ports procedures can
be broken.

>
> As to the scope issue, I don't think you *have* to use the global scope.
> You can pass parameters into the procedures if you choose to. It seems
> to me that using procedures provides limited advantages over non-modular
> code by not using parameters.


AFAIK vhdl passes arguments by value, making a local copy of the
parameter which has local scope to the subprogram, in this case I do not
know how I can have my 'state' variable retaining the value through
clock cycles.

 
Reply With Quote
 
alb
Guest
Posts: n/a
 
      09-25-2013
On 24/09/2013 19:21, Andy wrote:
> Al,
>
> I think you need flags to control calling the sub-procedures within
> autoread, not calling autoread itself. It looks like you would also
> need variables to hold the parameters you are going to pass to some
> of your sub-procedures.


flags to control sub-procedures calling are just the state of the FSM,
while sub-procedures operate on global variables.

>
> After reviewing your FSM though, I think you might do better with
> separate, smaller state machine(s) for the reusable states, and
> implement a hierarchical state machine. There is no point in
> combining all the reusable and unique states into the same FSM.


I started off by 'encapsulating' the reusable states in separate
procedures, but I was not sure how to 'wait' the end of it, that is why
I ended up putting everything under a single FSM.

> Does Synplify actually recognize your FSM as an FSM (what does it
> look like in the RTL viewer or FSM explorer)? Sometimes if you assign
> the next state from the contents of a register other than the current
> state, Synplify will not treat it as a state machine, which then
> excludes optimizations normally performed on FSMs.


That is true indeed. Apparently the FSM explorer does not recognize an
FSM. Definitely not good, a hierarchical approach becomes definitely
mandatory.

>
> As for my example that illuminated the synthesis problem, the
> procedure's logic was complex enough that any abnormalities would not
> have been easy to spot in RTL view. Unfortunately, Synplify unrolls
> functions and procedures for RTL view.


The fact that my RTL looks messier than what I think should have been a
clear hint that something is wrong.


 
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
Combinational feedback loops john VHDL 13 05-03-2006 10:13 PM
Loops with loops using html-template Me Perl Misc 2 01-12-2006 05:07 PM
Combinational logic running over multiple clock cycles in Xilinx Taras_96 VHDL 2 08-22-2005 08:00 AM
Problem with Clock signals generated by combinational logic GuitarNerd VHDL 8 05-19-2005 05:16 PM
Combinational Loop? ALuPin VHDL 5 08-31-2004 09:22 PM



Advertisments