Compare and update in same clock cycle synthesis problem

Something simular to the following code fragments works in pre-synthesis
simulation, but not in real hardware and post P&R timing simulation.

   entity controller is 
    port (
      ...
      datain   in  : std_logic_vector(7 downto 0);
      clock    in  : std_logic;
      ...
      lastval  out : std_logic_vector(7 downto 0);
    );
   end entity controller;

   architecture rtl of controller is
   begin
     process(clock)
       ...
       variable lastval_v : std_logic_vector(7 downto 0);
     begin
       if rising_edge(clock) then
         case control_state_v is

           ...

           when STATE_CHECK =>
             if datain /= lastval_v then
               lastval_v := datain;
               control_state_v := STATE_CHANGED
             else
               control_state_v := STATE_OTHER;
             end if;

           when STATE_CHANGED =>
             ...

           when STATE_OTHER =>
             ...

         end case;
       end if; 

       lastval <= lastval_v;

     end process;
   end architecture;
 
This works in pre-synthesis simulation (modelsim) and "STATE_CHANGED" is
only reached when datain is actually different from lastval_v. After
synthesis however, STATE_CHANGED is always reached except when datain is
equal to X"00". If I check the synthesis report, I find something about
lastval_v_mux0000 has a constant value during circuit operation. There
is also no 8-bit comparator found for the check state. Both I find 
amazing as the pre-synth simulation works and I know the data on datain
is changing.

To localize the problem, I have split the state in a separate check and
update state like this:

           when STATE_CHECK =>
             if datain /= lastval_v then
               control_state_v := STATE_UPDATE
             else
               control_state_v := STATE_OTHER;
             end if;

           when STATE_UPDATE =>
             lastval_v := datain;
             control_state_v := STATE_CHANGED;

This works in both pre- and post-synthesis simulation and also in real
hardware. There is now an 8-bit comparator found for the compare line
and no more warning about constant values.

So my question is: Is there a problem with comparing and updating a
value in the same state (clock)?

And does the problem als exist for single bit values when used like
this:

   if my_bit_v = '1' then
     my_bit_v := '0';
     -- do stuff
   end if;


All inputs to the entity come from other parts in the fpga and are all
updated on the same clock. The synthesis report claims the design can
run at 58 MHz, actual clock is 50 MHz, no other constraints defined.

The hardware is Xilinx spartan-3e and synthesis is done with ISE9.2.

-- 
Stef    (remove caps, dashes and .invalid from e-mail address to reply by mail)

On 4 Jun., 23:16, Stef <stef...@yahooI-N-V-A-L-I-D.com.invalid> wrote:

> This works in both pre- and post-synthesis simulation and also in real
> hardware. There is now an 8-bit comparator found for the compare line
> and no more warning about constant values.
>
> So my question is: Is there a problem with comparing and updating a
> value in the same state (clock)?

No as a general problem. The code shown is IMHO synthesisable.
Only strange thing I see is lastval beeing updatet again at the
falling edge of clock, but the code shown here should work right.

However I would change this code in case of trouble, as this code
might easily mask problems resulting from other parts of the design
(eg. if you have a module where clk and data change their order in
timing).

It might be also possible, that Xst struggles with this code even if
it the overall design is correct.
Try if other tools behave well with this code.

bye Thomas

In comp.arch.fpga,
Thomas Stanka <usenet_nospam_valid@stanka-web.de> wrote:
> On 4 Jun., 23:16, Stef <stef...@yahooI-N-V-A-L-I-D.com.invalid> wrote:
>
>> This works in both pre- and post-synthesis simulation and also in real
>> hardware. There is now an 8-bit comparator found for the compare line
>> and no more warning about constant values.
>>
>> So my question is: Is there a problem with comparing and updating a
>> value in the same state (clock)?
>
> No as a general problem. The code shown is IMHO synthesisable.

OK, thanks. This is what I believed as well but started doubting
after this experience. I use the single bit compare and update in
a number of places and have not had problems with those. There
should not be a fundamental problem between single-bit and multi-
bit compare operations.

> Only strange thing I see is lastval beeing updatet again at the
> falling edge of clock, but the code shown here should work right.

Sorry about that, that was a mistake in the example, the update
should have been inside the "if rising_edge(clock) then".

> However I would change this code in case of trouble, as this code
> might easily mask problems resulting from other parts of the design
> (eg. if you have a module where clk and data change their order in
> timing).
>
> It might be also possible, that Xst struggles with this code even if
> it the overall design is correct.
> Try if other tools behave well with this code.

At the moment I only have access to Xst for synthesis, so I can't try
that. I have now split up the compare and update in several states and
this has solved the problem. As the performance of this statemachine is
not critical, I will leave it like that.


-- 
Stef    (remove caps, dashes and .invalid from e-mail address to reply by mail)

control_state_v is being assigned to as a variable. It should be a signal.

In comp.arch.fpga,
RCIngham <robert.ingham@gmail.com> wrote:
> control_state_v is being assigned to as a variable. It should be a signal.
>

Why should that be a signal? control_state_v is my state variable and is
declared (not visible in the fragments) and used only in the clocked
process.


-- 
Stef    (remove caps, dashes and .invalid from e-mail address to reply by mail)

Stef wrote:

> Why should that be a signal? control_state_v is my state variable and is
> declared (not visible in the fragments) and used only in the clocked
> process.

There is nothing wrong
with a state variable instead of signal.

When sim and synth don't match the
problem is often missing synchronization
or some sneaky asynch path.

I would bet that datain
is not synchronized to clock.

       -- Mike Treseler

In comp.arch.fpga,
Mike Treseler <mike_treseler@comcast.net> wrote:
> Stef wrote:
>
>> Why should that be a signal? control_state_v is my state variable and is
>> declared (not visible in the fragments) and used only in the clocked
>> process.
>
> There is nothing wrong
> with a state variable instead of signal.

Phew, that was what I thought as well. ;-)

> When sim and synth don't match the
> problem is often missing synchronization
> or some sneaky asynch path.

As far as I know, there are no async paths in the design.
Except for one async reset for a ripple counter that only
controls an LED and that one is reported by synthesis.

> I would bet that datain
> is not synchronized to clock.

No, datain is synchronized (it's internally generated, receive
data from uart like device). The whole design runs on a single
clock (except for the upper stages of said ripple counter) and
all inputs are synchronized with that clock with 2 or more DFF's


-- 
Stef    (remove caps, dashes and .invalid from e-mail address to reply by mail)

On Jun 4, 5:16 pm, Stef <stef...@yahooI-N-V-A-L-I-D.com.invalid>
wrote:
> Something simular to the following code fragments works in pre-synthesis
> simulation, but not in real hardware and post P&R timing simulation.
>
...snip...
>
> This works in pre-synthesis simulation (modelsim) and "STATE_CHANGED" is
> only reached when datain is actually different from lastval_v. After
> synthesis however, STATE_CHANGED is always reached except when datain is
> equal to X"00". If I check the synthesis report, I find something about
> lastval_v_mux0000 has a constant value during circuit operation. There
> is also no 8-bit comparator found for the check state. Both I find
> amazing as the pre-synth simulation works and I know the data on datain
> is changing.
>
> To localize the problem, I have split the state in a separate check and
> update state like this:
>
>            when STATE_CHECK =>
>              if datain /= lastval_v then
>                control_state_v := STATE_UPDATE
>              else
>                control_state_v := STATE_OTHER;
>              end if;
>
>            when STATE_UPDATE =>
>              lastval_v := datain;
>              control_state_v := STATE_CHANGED;
>
> This works in both pre- and post-synthesis simulation and also in real
> hardware. There is now an 8-bit comparator found for the compare line
> and no more warning about constant values.

This can be significant.  Do you have the tools to "see" the logic
produced by this code?  It will be a *lot* easier to understand the
logic produced if you can reduce the problem code to a minimum set.
Look at exactly what is being produced in the way of logic.  You talk
about an 8 bit comparator, but I only see an equivalence check.  If
the variables are 8 bits, this only takes 5 LUTs in two levels.  I
supposed it might use 4 LUTs with the carry chain, but I don't think
that is essential.

> So my question is: Is there a problem with comparing and updating a
> value in the same state (clock)?

No, certainly the language does not know what you are doing in terms
of updating or comparing.  It only knows what you tell it.  If you
said to compare values and update the register that is used in the
compare, it is happy doing that... as long as that is what you are
telling it.

In comp.arch.fpga,
rickman <gnuarm@gmail.com> wrote:
> On Jun 4, 5:16 pm, Stef <stef...@yahooI-N-V-A-L-I-D.com.invalid>
> wrote:
>>
>>            when STATE_CHECK =>
>>              if datain /= lastval_v then
>>                control_state_v := STATE_UPDATE
>>              else
>>                control_state_v := STATE_OTHER;
>>              end if;
>>
>>            when STATE_UPDATE =>
>>              lastval_v := datain;
>>              control_state_v := STATE_CHANGED;
>>
>> This works in both pre- and post-synthesis simulation and also in real
>> hardware. There is now an 8-bit comparator found for the compare line
>> and no more warning about constant values.
>
> This can be significant.  Do you have the tools to "see" the logic
> produced by this code?  It will be a *lot* easier to understand the
> logic produced if you can reduce the problem code to a minimum set.
> Look at exactly what is being produced in the way of logic.  You talk
> about an 8 bit comparator, but I only see an equivalence check.  If
> the variables are 8 bits, this only takes 5 LUTs in two levels.  I
> supposed it might use 4 LUTs with the carry chain, but I don't think
> that is essential.

Xst actually reports "Found 8-bit comparator not equal for signal..."
So I presume it will implement something like you describe.

>> So my question is: Is there a problem with comparing and updating a
>> value in the same state (clock)?
>
> No, certainly the language does not know what you are doing in terms
> of updating or comparing.  It only knows what you tell it.  If you
> said to compare values and update the register that is used in the
> compare, it is happy doing that... as long as that is what you are
> telling it.

Somehow I must have failed telling that in the one-state approach. ;-)
Why, I still don't know. If there's a real error in my logic, I would
expect the pre-synth simulation to fail as well. 

To my relief, there is no problem with that type of coding, as I have
used this in more places in the design (but mostly with 1-bit flags).

The 2-state approach solved the curent problem, but it would indeed
be interesting to see if I can reproduce the problem in a minimal
design, but that will have to wait.


-- 
Stef    (remove caps, dashes and .invalid from e-mail address to reply by mail)

On Jun 6, 3:46 am, Stef <stef...@yahooI-N-V-A-L-I-D.com.invalid>
wrote:
> In comp.arch.fpga,
>
> Thomas Stanka <usenet_nospam_va...@stanka-web.de> wrote:
> > Only strange thing I see is lastval beeing updatet again at the
> > falling edge of clock, but the code shown here should work right.
>
> Sorry about that, that was a mistake in the example, the update
> should have been inside the "if rising_edge(clock) then".


Take this example:

sigA <= varB + varC;

If it appears immediately before the clock's "end if" (after any
possible assignments to the variables), then sigA is a register, and
the addition is performed on the combinatorial references to the
variables.

If it appears after the clock's "end if", then the references to the
variables are registered and the addition is performed and assigned
combinatorially to sigA (without a register).

The cycle-accurate behavior of both of these descriptions is
identical, and so would that of the resulting implementations. The
effects of combinatorial logic before or after the register may have
other effects (glitches, etc.).

However, since you have no operators in your variable expression, the
two would be identical in implementation too.

Andy