Interesting XST warning

[Andy Peters]

Here's one. Is XST overly cautious, or correct?

Given:

signal foo : std_logic_vector(15 downto 0);
alias foo_lo : std_logic_vector(7 downto 0) is foo(7 downto 0);
this : std_logic_vector(7 downto 0);
that : std_logic_vector(7 downto 0);
theother : std_logic_vector(7 downto 0);
sel : std_logic_vector(1 downto 0);
mux : std_logic_vector(7 downto 0);

....
mymux : process (foo_lo, this, that, theother, sel) is
begin
muxsel : case sel is
when "00" => mux <= foo_lo; -- this is line 666
when "01" => mux <= this;
when "10" => mux <= that;
when "11" => mux <= theother;
end case muxsel;
end process mymux;

XST gives me the following warning:

WARNING:Xst:819 - "foobar.vhdl" line 666: The following signals are
missing in the process sensitivity list:
foo<15>,
foo<14>,foo<13>,foo<12>,foo<11>,foo<10>,foo<9>,foo<8>,foo<7>,foo<6>,foo<5>,
foo<4>,foo<3>,foo<2>,foo<1>,foo<0>.

Clearly, XST doesn't like the alias foo_lo in the sensitivity list and
wants to see the original foo. Is this correct behaviour or another
XST quirk?

-a

 

[Mike Treseler]

Andy Peters wrote:

mymux : process (foo_lo, this, that, theother, sel) is
begin
muxsel : case sel is
when "00" => mux <= foo_lo; -- this is line 666
when "01" => mux <= this;
when "10" => mux <= that;
when "11" => mux <= theother;
end case muxsel;
end process mymux;

The signal sel has 9*9 possible states and the
example above covers only four of those.
A default case like:
when others => mux <= (others => '0');
would eliminate the latches and might make
XST happy as well. I agree the warning
emitted makes no sense.
One advantage to putting everything in
a synchronous process, would be not having
to edit sensitivity lists.

-- Mike Treseler

 

[radarman]

Our department policy is to use a two-process model, where you keep the
synchronous logic (registers) and combinatorial logic in separate
processes - with one registered process per clock domain. We consider
clock and *clock separate clock domains as well. There are a number of
reasons, some political, others technical - but I have found that we
have fewer errors when people follow that rule. For one, it makes it
very difficult to unintentionally write a latch and not get a warning.
It also makes it easier to find bad cross-clock domain crossings.

Another good reason to write this way is that it forces you to think in
terms of D and Q. In fact, I often specifically use _d and _q notation.
This makes the flops "pop" when you look at the code later.

BTW, simply adding "when others => null" would achieve the same
results. I add this even when I fully define every state just in case I
later alter something and no longer have every case covered.

 

[KJ]

Just out of curiousity...

but I have found that we
have fewer errors when people follow that rule. For one, it makes it
very difficult to unintentionally write a latch

Although there are other ways to create a latch as well, in the code that
I've inspected, it seems that the three most common ways to create an
unintentional latch are
- An 'if' statement without an 'else' clause.
- A 'case' statement without a 'when others' clause
- Either an 'if' or a 'case' statement where not every signal gets assigned
under every condition (i.e. signals 'a', 'b' and 'c' get assigned under one
'if' condition but only 'a' and 'c' under some other condtion).

Since all three of those statements can only occur within a process, by
allowing/encouraging that design style it would also seem to encourage the
'top three' causes of unintended latches. When you run across 'unintended
latches' at your company are they typically of some other form? If not,
then I don't follow how the company rule on the two process model would
actually cause fewer errors of that sort, seems like it would create more.

I recognize of course that one can also write latches without a process
(i.e. a <= (b and c) or (not(b) and a); among others) but from what I've
seen they don't seem to happen nearly as frequently.

and not get a warning.

I'd rather avoid the whole thing then weed through looking for a
warning...specially since many times it will compile and simulate just fine
only later during synthesis will the warning pop out.

It also makes it easier to find bad cross-clock domain crossings.
Can you elaborate a bit? I can see how a signal naming convention might

help find bad cross-clock domain crossings but I'm missing on how the two
process model could help.

Another good reason to write this way is that it forces you to think in
terms of D and Q. In fact, I often specifically use _d and _q notation.
This makes the flops "pop" when you look at the code later.

I played with something like that as well but I after trying it conclueded
that I didn't see any real value in having the 'D' and 'Q' 'pop' out at me.
Since doing it that way I now had twice the number of signals to deal with
in the source code then is really required overall I found that to be a
negative. Could you elaborate about what you gain by having 'D' and 'Q' pop
out?

KJ

 

[Andy]

I'm with KJ and Treseler on this one. Single, synchronous process (per
clock) descriptions virtually eliminate the possibility of inferring
unintentional latches. Two process (combinatorial + clocked)
descriptions virtually encourage creation of latches! Two process
descriptions make it more difficult to use clock enable functions on
register primitives, since some of the "logic" must now be moved over
to the synchronous process anyway. Two process descriptions also
simulate much slower than all-synchronous process descriptions, since
most modern simulators merge processes with like sensitivity lists into
one process, approaching cycle based performance. The random
sensitivity lists of combinatorial processes thwarts this optimization.

With regards to clock domain crossings, if both domains are described
with a single process each, and both processes use signals only for
external communication (using variables for all internal data, whether
it be combinatorial or registered), then it is much easier to
control/audit which values cross between the domains, since it would be
impossible for the variables to do so. Otherwise, one can use blocks
and/or entity/architecture boundaries to limit visibility between
domains.

With variables in synchronous processes, it is possible to describe any
logic function except a combinatorial input-to-output (no registers at
all) function. Combinatorial logic on register inputs, register
outputs, or in between registers, can all be described efficiently with
single synchronous processes.

Two-process descriptions were developed when the earliest synthesis
tools could not infer storage (flops). Thus storage had to be
instantiated, necessitating separating storage from combinatorial
logic. I think the state of the art has expanded beyond that now.

I know there are a lot of departmental policies that advocate doing
things the way they've always been done, and for a lot of good reasons,
but in this case, there's a better way...

Treating both edges of one clock as independent clock domains is not a
good idea. They are best handled by simply treating the two edges as
related clocks with a defined timing relationship, and enforcing timing
rules on them. Handling them with traditional asynchronous methods
(and ignoring timing paths between them) can cause problems related to
pseudo-synchronous circuits, since the probability of metastability can
be much higher (~=> 100%) than with random, unrelated clocks.

Andy

 

[Andy Peters]

Mike Treseler wrote:

Andy Peters wrote:

mymux : process (foo_lo, this, that, theother, sel) is
begin
muxsel : case sel is
when "00" => mux <= foo_lo; -- this is line 666
when "01" => mux <= this;
when "10" => mux <= that;
when "11" => mux <= theother;
end case muxsel;
end process mymux;

The signal sel has 9*9 possible states and the
example above covers only four of those.
A default case like:
when others => mux <= (others => '0');
would eliminate the latches and might make
XST happy as well. I agree the warning
emitted makes no sense.

Mike,

I omitted the default case to save a bit of typing in my example; the
mux in my real code has eight choices (selector is three bits) plus a
when others default . Interestingly, when handling my real code, XST
throws an INFO:

INFO:Xst:1561 - "c:/foo/bar/bletch/grue.vhdl" line 669: Mux is complete
: default of case is discarded

So go figure.

One advantage to putting everything in
a synchronous process, would be not having
to edit sensitivity lists.

True, but in this case, I don't have a master clock so I have no
choice.

-a

 

[radarman]

Our departmental policies definitely come from a time long ago. The guy
that did the style guide was around before HDL synthesis existed.
However, failing to follow it will result in stern warnings during a
code review. I learned VHDL when I joined the department, so I never
had a chance to do it differently until I started coding at home, and
most of it carried over.

Personally, I avoid latches by never using the else in an if/else
statement. I always have the "else" statements before the "if". Ie:

process: (A, B)
begin
C <= '0'; -- default value
if( A = '1')
C <= B;
end if;
end process;

For registers, I always make sure the first line in the process is the
D <= Q; Likewise, I always have a "when others =>" line in a case
structure. Essentially, I make sure there is default path in every case
such that no combinatorial process can infer storage.

 

[Mike Treseler]

Andy Peters wrote:

I omitted the default case to save a bit of typing in my example; the
mux in my real code has eight choices (selector is three bits) plus a
when others default . Interestingly, when handling my real code, XST
throws an INFO:
INFO:Xst:1561 - "c:/foo/bar/bletch/grue.vhdl" line 669: Mux is complete
: default of case is discarded
So go figure.

Yes, XST will let you off the hook with
8 cases, but modelsim won't.
Consider replacing one of the 8 /when/ clauses
with /when others/ to make all the tools happy.

One advantage to putting everything in
a synchronous process, would be not having
to edit sensitivity lists.

True, but in this case, I don't have a master clock so I have no
choice.

OK, since I derailed your thread,
I'll pop my stack and say
try eliminating the alias
and declare foo_hi and foo_lo separately.

-- Mike Treseler

 

[Andy Peters]

Mike Treseler wrote:

Andy Peters wrote:

I omitted the default case to save a bit of typing in my example; the
mux in my real code has eight choices (selector is three bits) plus a
when others default . Interestingly, when handling my real code, XST
throws an INFO:
INFO:Xst:1561 - "c:/foo/bar/bletch/grue.vhdl" line 669: Mux is complete
: default of case is discarded
So go figure.

Yes, XST will let you off the hook with
8 cases, but modelsim won't.
Consider replacing one of the 8 /when/ clauses
with /when others/ to make all the tools happy.

Gotcha. Instead of the eight discrete states plus the when-others,
just make one of the states the when-others.

One advantage to putting everything in
a synchronous process, would be not having
to edit sensitivity lists.

True, but in this case, I don't have a master clock so I have no
choice.

OK, since I derailed your thread,
I'll pop my stack and say
try eliminating the alias
and declare foo_hi and foo_lo separately.

The reason for this alias is that I have a lower-level module (a shift
register) with a multibit output port. This port's bits get used in
different places, and

u_sr : entity work.sr
port map (
SClk => SClk,
SCs_l => SCs_l,
SDat => SDat,
PDat => (foo_hi & foo_lo & otherbits));

isn't legal. (But it is in Verilog ...)

-a

 

[Mike Treseler]

Andy Peters wrote:

The reason for this alias is that I have a lower-level module (a shift
register) with a multibit output port. This port's bits get used in
different places, and

u_sr : entity work.sr
port map (
SClk => SClk,
SCs_l => SCs_l,
SDat => SDat,
PDat => (foo_hi & foo_lo & otherbits));

When I run into something like this,
I merge the entities and processes.

-- Mike Treseler