Mixing comb and reg part in one process

V

valentin tihomirov

Is it a good style to write code like the following

process (CLK, Reset)
variable RegNext: STD_LOGIC;
begin
-- comb part
RegNext := calculate();

-- reg part
if Reset = '1' then
RegNext <= '0';
elsif CLK'event and CLK = '1' then
REG <= RegNext;
end if;
end.

????

I've asked this question to myself trying to define the sensetivity list and
type (signal vs. variable) of RegNext value. This style would allow for
combining related logic into one process rather than divide one process into
comb and reg declaring lots ot specific signals at architecture scope.
 
D

David Jones

Is it a good style to write code like the following

process (CLK, Reset)
variable RegNext: STD_LOGIC;
begin
-- comb part
RegNext := calculate();

-- reg part
if Reset = '1' then
RegNext <= '0';

REG <= '0';
elsif CLK'event and CLK = '1' then
REG <= RegNext;
end if;
end.

Other than the one correction above, this is OK.

But what's wrong (in this case) with:


process (CLK, Reset)
begin
-- reg part
if Reset = '1' then
REG <= '0';
elsif CLK'event and CLK = '1' then
REG <= calculate();
end if;
end.


If calculate() is complex (i.e. it is a sequence of statements),
then put it closer to where it will be used:


process (CLK, Reset)
variable RegNext: STD_LOGIC;
begin
-- reg part
if Reset = '1' then
REG <= '0';
elsif CLK'event and CLK = '1' then
RegNext := calculate();
REG <= RegNext;();
end if;
end.
 
V

valentin tihomirov

But what's wrong (in this case) with:
process (CLK, Reset)
begin
-- reg part
if Reset = '1' then
REG <= '0';
elsif CLK'event and CLK = '1' then
REG <= calculate();
end if;
end.

The consern is - reuse. It often happends that there are signals in
combinatorial part that muliple others comb signals depend on. For example,
several registers depend on CNT = 0 comparator's out. If you enclose
combinatorial signals behind CLK, several comparators will be generated.
Most synthesis tools can detect this and allow reuse of resources, you say.
But when logic becomes more comlex (for ex, one replaces that comparator
with two comparators), the style based on reuse leads to less verbose and
easier-to-maintain code.
I did not adopt using of functions (a sort of reuse in a certain way), this
is an idea though. Thanks.
 
O

Oggie

valentin tihomirov said:
Is it a good style to write code like the following

process (CLK, Reset)
variable RegNext: STD_LOGIC;
begin
-- comb part
RegNext := calculate();

-- reg part
if Reset = '1' then
RegNext <= '0';
elsif CLK'event and CLK = '1' then
REG <= RegNext;
end if;
end.

????

I've asked this question to myself trying to define the sensetivity list and
type (signal vs. variable) of RegNext value. This style would allow for
combining related logic into one process rather than divide one process into
comb and reg declaring lots ot specific signals at architecture scope.

Assuming you are trying to define FSM you could do that in one of the
two ways:
1. Using two processes (one combinational for the Next_State and one
clocked for the Current_State).
2. Combine the above two processes in one CLOCKED process (similar to
your code). In this case the process should look like:

**************************************************
type SM_TYPE is (state_1, state_2, ....);
signal sm_present_state : SM_TYPE;

process (CLK, Reset)
variable sm_next_state : SM_TYPE;
begin
if Reset = '1' then
sm_present_state <= state_X; -- this is the initial state
elsif CLK'event and CLK = '1' then
sm_present_state <= sm_next_state;
sm_next_state := calculate_next_state();
end if;
end process;
**************************************************

The signals involved in "calculate_next_state()" do not have to be in
the sensitivity list because this is clocked process. Both 1 and 2
processes FSM definitions are legal, but the two processes definition
is considered more readable and is the preferred way.

Cheers,
Oggie
 
V

valentin tihomirov

Thanks to all, I think I have worked out a good style without next and
clocked process. Please, appretiate my code below. As you see, it is neat;
no _NEXT internal logic signals are shown at architecture scope. Avtive-HDL
succesefully compiles the file but I did not ask a synthesier about it yet.


-- signals READ and CLEANING are used in this and in a separate process
-- signals are external nets, variables are used inside process
-- signals are named in UPPERCASE, variables in MixedCase.
SHIFT_REG: process (RESET, CLK, SHIFTING, AVAIL)
variable Empty: boolean;
variable Cnt, CntNext: Integer range 0 to WIDTH; -- amount of bits in
shift_reg
begin

-- combinatorial part
Empty := Cnt = 0;

CntNext := Cnt;

if not Empty and SHIFTING = '1' then
CntNext := CntNext - 1;
end if;

CLEANING <= TO_STD_LOGIC(SHIFTING = '1' and CntNext = 0);

READ <= '0';
if (Empty or CLEANING = '1') and AVAIL = '1' then
READ <= '1';
CntNext := WIDTH;
end if;

-- clocked part
if RESET= '1' then -- Reset
Cnt := 0;
elsif Rising_Edge(CLK) and ENABLE = '1' then

if READ = '1' then
BUF <= FOUT;
elsif SHIFTING = '1' then
BUF <= '1' & BUF(WIDTH-1 downto 1);
end if;

Cnt := CntNext;

end if; -- clock

end process;
 
D

David R Brooks

It is legal VHDL (saving, as others have noted, RegNext := '0' as
it's declared a variable).
However, it likely isn't synthesisable. If you split it into 2 parts,
each performs a clearly defined action, & the synthesiser can see what
you mean. Of course, RegNext is then a signal.
Bear in mind that most synthesisers work by matching your code to a
list of templates: when they find a match, that defines the code they
produce. If you write "creative" VHDL, it may be syntactically OK, but
it won't match a template, & the synthesiser will fail.
Get a copy of the "Libraries Guide" and "XST User's Guide" from
www.xilinx.com - they contain examples of the proper code to use to
infer each of the Xilinx hardware primitives. Of course, these are
specific to XST (the Xilinx tool), but the principles are general.


:Is it a good style to write code like the following
:
:process (CLK, Reset)
:variable RegNext: STD_LOGIC;
:begin
: -- comb part
: RegNext := calculate();
:
: -- reg part
: if Reset = '1' then
: RegNext <= '0';
: elsif CLK'event and CLK = '1' then
: REG <= RegNext;
: end if;
:end.
:
:????
:
:I've asked this question to myself trying to define the sensetivity list and
:type (signal vs. variable) of RegNext value. This style would allow for
:combining related logic into one process rather than divide one process into
:comb and reg declaring lots ot specific signals at architecture scope.
:
 
O

Oggie

valentin tihomirov said:
Thanks to all, I think I have worked out a good style without next and
clocked process. Please, appretiate my code below. As you see, it is neat;
no _NEXT internal logic signals are shown at architecture scope. Avtive-HDL
succesefully compiles the file but I did not ask a synthesier about it yet.


-- signals READ and CLEANING are used in this and in a separate process
-- signals are external nets, variables are used inside process
-- signals are named in UPPERCASE, variables in MixedCase.
SHIFT_REG: process (RESET, CLK, SHIFTING, AVAIL)
variable Empty: boolean;
variable Cnt, CntNext: Integer range 0 to WIDTH; -- amount of bits in
shift_reg
begin

-- combinatorial part
Empty := Cnt = 0;

CntNext := Cnt;

if not Empty and SHIFTING = '1' then
CntNext := CntNext - 1;
end if;

CLEANING <= TO_STD_LOGIC(SHIFTING = '1' and CntNext = 0);

READ <= '0';
if (Empty or CLEANING = '1') and AVAIL = '1' then
READ <= '1';
CntNext := WIDTH;
end if;

-- clocked part
if RESET= '1' then -- Reset
Cnt := 0;
elsif Rising_Edge(CLK) and ENABLE = '1' then

if READ = '1' then
BUF <= FOUT;
elsif SHIFTING = '1' then
BUF <= '1' & BUF(WIDTH-1 downto 1);
end if;

Cnt := CntNext;

end if; -- clock

end process;


If your intention is to synthesize hardware implementation, then the
above code has multiple problems. The different synthesis tools will
produce different results due to the way the code is written. The most
important is the interpretation of multiple "IF-END IF" before
"Rising_Edge(CLK)" and "elsif Rising_Edge(CLK) and ENABLE = '1' then".
While it is perfectly OK for simulation, the different synthesis tools
will have hard time to guess the intention of this code and as a
result some will generate errors, some will generate warnings, some
will make assumptions and try correct it. In addition to that the
resulted hardware (if synthesized at all) will definately behave
different from the RTL simulation.
1. Think about the sensitivity list(if there is one) - a process only
triggers when there is transition on at liast one signal in its
sensitivity list. A local variable (declared inside the process) CAN
NOT trigger a process.
2. Given the above, all signals which are read in the combinational
portion of the process MUST be in the sensitivity list. Keep in mind
that all signals assigned in a process DO NOT change their value until
after the process is terminated, Thus the signals "CLEANING" and
"ENABLE" must be in the sensitivity list.
3. A process which describes sequential element (FF) may also describe
combinational logic between the input of the described FF and:
** the output of other FFs or LATCHEs or
** its own output or
** module inputs.
To do that the process must have only one "[IF]-[ELSIF]-IF/ELSIF
edge(clk)-END IF" statement. There may be multiple embedded "IF-END
IF" statements under each clause, but the process can not have more
then one non-embedded "IF-END IF" between the "BEGIN" and "END
PROCESS". The first "[IF]" and "[ELSIF]" clauses (if they exist)
define the asynchronous set/reset combinational logic (it may be
function or only connection of the external to the process signals)
thus the signals read in this portion MUST be in the sensitivity list.
The "IF/ELSIF edge(clk)" (clocked portion) clause defines the
combinational logic (it may be function or only connection of the
external to the process signals and or local variables) which drives
the data input of the FF or LATCH.
4. Considering the above, it is obvious that for FSM the NEXT_STATE
(combinational logic) could be defined under "IF/ELSIF edge(clk)"
clause, but not in separate (non-embedded) "IF-END IF" statement.
Since this portion is only executed upon CLOCK transition, there is no
need for the signals read in this portion to be in the sensitivity
list (the result of the combinational function is only used when CLOCK
transitions). Even more, it is not recommended those signals to be in
the sensitivity list, because it may result in unintentional
generation of LATCHES.
5. A process can define combinational logic only or sequencial element
only or both - sequencial element and combinational logic. When a
process does define sequencial element only or both - sequencial
element and combinational logic the rules described above apply. When
a process defines combinational logic only, then no "'EVENT" attribute
can be used in it and every time the process is triggered all of the
signals driven in this process MUST be assigned otherwise LATCH will
be generated. In your code:
-- clocked part
if RESET= '1' then -- Reset
Cnt := 0;
elsif Rising_Edge(CLK) and ENABLE = '1' then

looks like "if RESET= '1' then" is the asynchronous reset (this is not
the clocked portion). The variable Cnt will be written when the
process is triggered and RESET is '1' or there is Rising_Edge(CLK)
(the portion "and ENABLE = '1'" will be discussed later for now I will
assume it being a separate IF statement under "elsif Rising_Edge(CLK)
then"). It all looks OK if we assume that the process will never
trigger if there is no transition on either RESET or CLK and this is
the only "IF-...-END IF" statement in the process. But the process
will trigger upon SHIFTING and AVAIL, thus making it possible for Cnt
to not be written - reason for LATCH generation, but the intention is
for Cnt to be FF??? Furthermore "elsif Rising_Edge(CLK) and ENABLE =
'1' then" is not welcomed by some synthesis tools - some may not even
recognize the intention for FF generation and optimize out Cnt and all
signals related to it (like CLK FOUT, ENABLE).

I would suggest you review my previous post.
Cheers,
Oggie
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top