myhdl-list

[myhdl-list] myhdl.AlwaysCombError: signal (startmpy) used as inout in always_comb function argument

[Josy B. <jos...@gm...>]

In a module I have two state machines: one doing overall control and the 
second doing a serial multiply. I (almost always) write two-process 
state machine. As a result I have one @always_comb where the the first 
state machine issues a startpulse to the second and then waits for a 
donepulse from the second to continue. Both state machine have some 
(combinatorial or asynchronous) output signals in common so they have to 
reside in the same (VHDL) process.

MyHDL then complains that inouts in always_comb are not allowed.

If I comment out some code in _always_comb.py, as shown:
   def visit_Module(self, node):
#         inputs = self.inputs
#         outputs = self.outputs
        for n in node.body:
            self.visit(n)
#         for n in inputs:
#             if n in outputs:
#                 raise AlwaysCombError(_error.SignalAsInout % n)

everything goes fine, the simulation is OK and the VHDL code looks 
plausible as the 'offending' signals are well in the sensitivity list:
  KALMANFILTER_SMCOMB: process (D, smmpyp, yn, CoeffB, CoeffA, shiftc_Q, 
add_Q, mpycount, xnb, StrobeIn, shifta_Q, startmpy, mpydone, smkfp) is


The verilog conversion doesn't complain either and (as far as my Verilog 
knowledge goes) produces a plausible sensitivity list too:
  always @(D, smmpyp, yn, CoeffB, CoeffA, shiftc_Q, add_Q, mpycount, 
xnb, StrobeIn, shifta_Q, startmpy, mpydone, smkfp) begin: 
KALMANFILTER_SMCOMB

Obviously the VHDL will compile fine, I don't know about Verilog. Have I 
broken something?

Regards,

Josy

Re: [myhdl-list] myhdl.AlwaysCombError: signal (startmpy) used as inout in always_comb function argument

[Henry G. <he...@ca...>]

On 11/02/15 15:41, Josy Boelen wrote:
> In a module I have two state machines: one doing overall control and the
> second doing a serial multiply. I (almost always) write two-process
> state machine. As a result I have one @always_comb where the the first
> state machine issues a startpulse to the second and then waits for a
> donepulse from the second to continue. Both state machine have some
> (combinatorial or asynchronous) output signals in common so they have to
> reside in the same (VHDL) process.
<snip>

Do you have some example code?

Henry

Re: [myhdl-list] myhdl.AlwaysCombError: signal (startmpy) used as inout in always_comb function argument

[Josy B. <jos...@gm...>]

Henry Gomersall <heng <at> cantab.net> writes:

> 
> On 11/02/15 15:41, Josy Boelen wrote:
> > In a module I have two state machines: one doing overall control and 
the
> > second doing a serial multiply. I (almost always) write two-process
> > state machine. As a result I have one  <at> always_comb where the 
the first
> > state machine issues a startpulse to the second and then waits for a
> > donepulse from the second to continue. Both state machine have some
> > (combinatorial or asynchronous) output signals in common so they 
have to
> > reside in the same (VHDL) process.
> <snip>
> 
> Do you have some example code?
> 
><snip>
Henry,

the code is a handful (as usual) but I made a simplified example (as 
Chris also likes to see ...):

def contrivedtwostatemachineexample( Clk, Reset,  StartP, ... )
    states_first = enum( "IDLE" , "ONE" , "TWO")
    smfp, smfn = [Signal( states_first.IDLE ) for _ in range( 2 )]
    states_second = enum( "IDLE" , "BUSY")
    smfp, smfn = [Signal( states_second .IDLE ) for _ in range( 2 )]
    startsecond, donesecond = [Signal(bool(0)) _ for _ in range(2)}
 

    @always_comb
    def smcomb():
        startsecond.next = 0
        donesecond.next = 0

        if smfp == states_first.IDLE:
            if StartP:
                smfn.next = states_first.ONE
                startsecond.next = 1
            else:
                smfn.next = states_first.IDLE

        elif smfp == states_first.ONE:
            if donesecond:
                smfn.next = states_first.TWO
                startsecond.next = 1
            else:
               smfn.next = states_first.ONE

        elif smfp == states_first.TWO:
            if donesecond:
                smfn.next = states_first.IDLE
            else:
               smfn.next = states_first.TWO


       if smsp == states_second.IDLE:
           if startsecond:
               smsn.next = states_second.BUSY
           else:
               smsn.next = states_second.IDLE
  
       elif smsp == states_second.BUSY:
           donesecond.next = 1
           # must catch an immediate start command
           if startsecond:
               smsn.next = states_second.BUSY
           else:
               smsn.next = states_second.IDLE
 
    @always_seq(Clk.posedge, reset=Reset)
    def smprocess():
        smfp.next = smfn
        smsp.next = smsn
     
   return smcomb, smprocess


You see that both **startsecond** and **donesecond** are written and 
read in the combinatorial process. MyHDL chokes on that, spitting out 
the AlwaysCombError message. But as the two signals in question are not 
**Ports** there is no real reason to do so, unless Verilog has a problem 
with it. Which is my original question: is this a problem in Verilog?

Regards,

Josy

Re: [myhdl-list] myhdl.AlwaysCombError: signal (startmpy) used as inout in always_comb function argument

[Christopher F. <chr...@gm...>]

On 2/11/2015 10:54 AM, Josy Boelen wrote:
<snip>
>> Do you have some example code?
>>
>> <snip>
> Henry,
>
> the code is a handful (as usual) but I made a simplified example (as
> Chris also likes to see ...):

:)

>
> def contrivedtwostatemachineexample( Clk, Reset,  StartP, ... )
<snip>
>
>      @always_comb
>      def smcomb():


Why have a single comb vs. two comb?

     def smcombfp():
     ...

     def smcombsn():
     ...

I believe the inout comb detection is a mechanism to
protect against unwanted comb feedback.  If you have
a complicated @always_comb it probably make sense to
split it out rather than remove the comb feedback
detection.

Regards,
Chris

Re: [myhdl-list] myhdl.AlwaysCombError: signal (startmpy) used as inout in always_comb function argument

[Josy B. <jos...@gm...>]

Christopher Felton <chris.felton <at> gmail.com> writes:

> 
> Why have a single comb vs. two comb?

Because they target the same outputs. I'll augment the simplified 
example a bit:

def contrivedtwostatemachineexample( Clk, Reset,  StartP, Status, ... )
    states_first = enum( "IDLE" , "ONE" , "TWO")
    smfp, smfn = [Signal( states_first.IDLE ) for _ in range( 2 )]
    states_second = enum( "IDLE" , "BUSY")
    smfp, smfn = [Signal( states_second .IDLE ) for _ in range( 2 )]
    startsecond, donesecond = [Signal(bool(0)) _ for _ in range(2)}

     <at> always_comb
    def smcomb():
        startsecond.next = 0
        donesecond.next = 0
        Status.next = 0

        if smfp == states_first.IDLE:
           Status.next = 1
           if StartP:
               smfn.next = states_first.ONE
               startsecond.next = 1
           else:
               smfn.next = states_first.IDLE

        elif smfp == states_first.ONE:
           Status.next = 2
           if donesecond:
               smfn.next = states_first.TWO
               startsecond.next = 1
           else:
              smfn.next = states_first.ONE

        elif smfp == states_first.TWO:
           Status.next = 3
           if donesecond:
               smfn.next = states_first.IDLE
           else:
              smfn.next = states_first.TWO

       if smsp == states_second.IDLE:
           Status.next = 10
           if startsecond:
               smsn.next = states_second.BUSY
           else:
               smsn.next = states_second.IDLE

       elif smsp == states_second.BUSY:
           Status.next = 11
           donesecond.next = 1
           # must catch an immediate start command
           if startsecond:
               smsn.next = states_second.BUSY
           else:
               smsn.next = states_second.IDLE

     <at> always_seq(Clk.posedge, reset=Reset)
    def smprocess():
        smfp.next = smfn
        smsp.next = smsn

   return smcomb, smprocess

If we split it over two processes, VHDL will complain about multiple 
drivers ...

> 
>      def smcombfp():
>      ...
> 
>      def smcombsn():
>      ...
> 
> I believe the inout comb detection is a mechanism to
> protect against unwanted comb feedback.  If you have
> a complicated  <at> always_comb it probably make sense to
> split it out rather than remove the comb feedback
> detection.

What is **unwanted comb feedback**? Is this in the Verilog LRM? Or is 
the comb feedback in the code to protect us from ourselves? If it is not 
against Verilog, the **comb feedback detection** should, IMHO, be 
reworked. If Verilog has a problem with this ... you tell me.

Splitting my actual code in two processes would add clumsy workarounds, 
making the code not any prettier.

Regards,

Josy

Re: [myhdl-list] myhdl.AlwaysCombError: signal (startmpy) used as inout in always_comb function argument

[Christopher F. <chr...@gm...>]

<snip>
>> I believe the inout comb detection is a mechanism to
>> protect against unwanted comb feedback.  If you have
>> a complicated  <at> always_comb it probably make sense to
>> split it out rather than remove the comb feedback
>> detection.
>
> What is **unwanted comb feedback**?

Combinational loops [1][2] as a result of a complicated
expression.

> Is this in the Verilog LRM? Or is
> the comb feedback in the code to protect us from ourselves? If it is not
> against Verilog, the **comb feedback detection** should, IMHO, be
> reworked. If Verilog has a problem with this ... you tell me.

It is not a Verilog thing and I am not 100% sure if the
original design intent was to protect us from ourselves
but that is my guess.  How / why should it be reworked?

>
> Splitting my actual code in two processes would add clumsy workarounds,
> making the code not any prettier.
>

I'm not so sure about the dual-state machine driving
same outputs.  It's interesting, it would be the
equivalent of a single state-machine with two state
registers, which is just another way of logically
organizing a more complex set of states ... hmmmm

Regards,
Chris

[1] 
http://quartushelp.altera.com/13.1/mergedProjects/verify/da/comp_file_rules_loop.htm

[2] http://fpga-hdl.blogspot.com/2012/07/test.html

Re: [myhdl-list] myhdl.AlwaysCombError: signal (startmpy) used as inout in always_comb function argument

[Christopher F. <chr...@gm...>]

On 2/11/2015 1:44 PM, Josy Boelen wrote:
> Christopher Felton <chris.felton <at> gmail.com> writes:
>
>>
>> Why have a single comb vs. two comb?
>
> Because they target the same outputs. I'll augment the simplified
> example a bit:

This modified code isn't a good example because the
second state-machine always overwrites the first, so
you can eliminate the first state-machines assignments
to /Status/.

You would need states in each state-machine that doesn't
assign and doesn't overlap - but ugh how do you keep
track of that as the complexity grows?

Regards,
Chris

Re: [myhdl-list] myhdl.AlwaysCombError: signal (startmpy) used as inout in always_comb function argument

[Josy B. <jos...@gm...>]

Christopher Felton <chris.felton <at> gmail.com> writes:

> 
> <snip>
> >> I believe the inout comb detection is a mechanism to
> >> protect against unwanted comb feedback.  If you have
> >> a complicated  <at> always_comb it probably make sense to
> >> split it out rather than remove the comb feedback
> >> detection.
> >
> > What is **unwanted comb feedback**?
> 
> Combinational loops [1][2] as a result of a complicated
> expression.
> 
> > Is this in the Verilog LRM? Or is
> > the comb feedback in the code to protect us from ourselves? If it is 
not
> > against Verilog, the **comb feedback detection** should, IMHO, be
> > reworked. If Verilog has a problem with this ... you tell me.
> 
> It is not a Verilog thing and I am not 100% sure if the
> original design intent was to protect us from ourselves
> but that is my guess.  How / why should it be reworked?
> 
> >
> > Splitting my actual code in two processes would add clumsy 
workarounds,
> > making the code not any prettier.
> >
> 
> I'm not so sure about the dual-state machine driving
> same outputs.  It's interesting, it would be the
> equivalent of a single state-machine with two state
> registers, which is just another way of logically
> organizing a more complex set of states ... hmmmm
> 
> Regards,
> Chris
> 
> [1] 
> 
http://quartushelp.altera.com/13.1/mergedProjects/verify/da/comp_file_ru
les_loop.htm
> 
> [2] http://fpga-hdl.blogspot.com/2012/07/test.html

Chris,

you are a great source of knowledge (read this as praise)
[1] : Quartus will tell me when I'm (that) stupid. I have seen this 
warning once, when I made a **ring oscillator** to generate random 
numbers (based on one of their app-notes)
I posted the code for your perusal-> 
https://github.com/josyb/KalmanFilter
I've written it this way, as this is what I would have written in VHDL. 
One other example where I used this was for a DDR2 SDRAM controller 
where one state-machine handles the initialisation and the second 
handles the actual read/write/refresh operation. Of course this could be 
done in a single state machine, but this approach of divide and conquer 
generates smaller and faster code (which I can't prove right-away ...)

I think that the rework is actually removing the **check** as I showed. 
At least that is what I did locally. I'l see tomorrow how the VHDL 
synthesizes when plugged into the **real** project.

Regards,

Josy

Re: [myhdl-list] myhdl.AlwaysCombError: signal (startmpy) used as inout in always_comb function argument

[Josy B. <jos...@gm...>]

Christopher Felton <chris.felton <at> gmail.com> writes:


> >>
> >> Why have a single comb vs. two comb?
> >
> > Because they target the same outputs. I'll augment the simplified
> > example a bit:
> 
> This modified code isn't a good example because the
> second state-machine always overwrites the first, so
> you can eliminate the first state-machines assignments
> to /Status/.

Yes, that's what happens with untested small example code. But you could 
assume that the **Status** was for monitoring (using SignalTap or some 
other method), it would me make wonder what happened '2' and '3' 

> 
> You would need states in each state-machine that doesn't
> assign and doesn't overlap - but ugh how do you keep
> track of that as the complexity grows?
> 
In this particular case, see https://github.com/josyb/KalmanFilter,  I 
could have added a third state machine which together would use the 
shared resource on an exclusive basis and the first state machine would 
only deal the cards. 

If the complexity would grow above this, I'd consider building a 
microprogrammed machine ;)

Best regards,

Josy