False combinational loops in Synplicity -

[Newdaddy06]

I have two combinational blocks, fnA.vhd and fnB.vhd. I have a higher level of hierarchy, test.vhd, which instantiates these two functions, and also infers a clocked register, each with an input mux. There it a top-level port, contrlAnotB, that drives the control inputs to all three muxes.

If input port contrlAnotB = 1, then the output of fnA will drive the input to fnB, but the output of fnB is not allowed to drive the input to fnA (rather, the register output stateReg does.) So, when contrlAnotB = 1, there can be no combinational loop.

If input port contrlAnotB = 0, then the output of fnB drives the input to fnA, but the output of fnA is not allowed to drive the input to fnB (rather, it is driven by stateReg.) So when contrlAnotB = 0, there can be no combinational loop.

Synplicity finds lots of combinational paths of type fnA -> fnB -> fnA, and also fnB -> fnA -fnB, even though I know there is no value of contrlAnotB which would allow these paths.

How can I resolve this? There are lots of cases where one may want to execute two functions, in either order, and register the result. (This instance is extracted from a block encryptor/decryptor. If encrypting, the steps are executed in one order, if decrypting, then the reverse.) I'd like to use define_false_path to disallow consideration of these paths, but can't find a syntax that will work (as there are no registers, top-level ports, or black boxes in the combinational loops it is seeing.)

Any advice or suggestions would be greatly appreciated. Thanks -View attachment testForOutside.txt

- - - Updated - - -

-- So, I tried to attach the source files, but somehow failed. I will just paste test.vhd below.

library ieee;
use ieee.std_logic_1164.all;
use ieee.std_logic_unsigned.all;

entity test is
port (clk : in std_logic;
reset : in std_logic;

loadState : in std_logic;
byteIn : in std_logic_vector(7 downto 0);
contrlAnotB : in std_logic;

outRequest : in std_logic;
outByte : out std_logic_vector(7 downto 0)
);
end test;

architecture rtl of test is

-- component declarations

component fnA
port (x : in std_logic_vector(7 downto 0);
y : out std_logic_vector(7 downto 0));
end component fnA;

component fnB
port (x : in std_logic_vector(7 downto 0);
y : out std_logic_vector(7 downto 0));
end component fnB;

attribute syn_hier : string;
-- attribute syn_hier of rtl: architecture is "Hard";
attribute syn_hier of rtl: architecture is "Firm";


-- main data path
signal Ain : std_logic_vector(7 downto 0);
signal Aout : std_logic_vector(7 downto 0);
signal Bin : std_logic_vector(7 downto 0);
signal Bout : std_logic_vector(7 downto 0);
signal stateIn : std_logic_vector(7 downto 0);
signal stateReg : std_logic_vector(7 downto 0);

begin

aInMux_proc : process (contrlAnotB, Bout,
stateReg)
begin
if (contrlAnotB = '1') then -- A first, then B
Ain <= stateReg(7 downto 0) ;
else -- first B, then A
Ain <= Bout;
end if;
end process aInMux_proc;

-- Here is A

fnA_0 : fnA port map(
x => Ain,
y => Aout
);

bInMux_proc : process (contrlAnotB, Aout,
stateReg)
begin
if (contrlAnotB = '1') then -- A first, then B
Bin <= Aout;
else -- first B, then A
Bin <= stateReg(7 downto 0) ;
end if;
end process bInMux_proc;

-- Here is B

fnB_0 : fnB port map(
x => Bin,
y => Bout
);

stateRegMux_proc : process (contrlAnotB, Aout, Bout,
byteIn, loadState)
begin
if (loadState = '1') then
stateIn <= byteIn;
else
if (contrlAnotB = '1') then -- A first, then B
stateIn <= Bout;
else -- first B, then A
stateIn <= Aout;
end if;
end if;
end process stateRegMux_proc;

stateReg_proc : process (clk, reset)
begin
if (reset = '1') then
stateReg <= (others => '0');
elsif (rising_edge(clk)) then
stateReg <= stateIn;
end if;
end process stateReg_proc;

outMux_proc : process (stateReg, outRequest)
begin
if (outRequest = '1') then
outByte <= stateReg;
else
outByte <= (others => '0');
end if;
end process outMux_proc;

end rtl;

- - - Updated - - -

-- My apologies for how this reads, somehow the formatting removed all the white space.

 

[kornukhin]

You can re-implement your code like this (I rewrite it using verilog to have more compact syntax).

Source code:

assign Ain = contrlAnotB ? stateReg[7:0] : Bout;
assign Aout = Ain[0] ? {Ain[0], Ain[1], Ain[2], Ain[3], Ain[4], Ain[5], Ain[6], Ain[7]} : {Ain[6:0], Ain[7]};
assign Bin = contrlAnotB ? Aout : stateReg[7:0] ;
assign Bout = Bin[0] ? {Bin[6], Bin[6], Bin[4], Bin[4], Bin[2], Bin[2], Bin[0], Bin[0]} : {Bin[7], Bin[7], Bin[5], Bin[5], Bin[3], Bin[3], Bin[1], Bin[1]};

always@(posedge clk or posedge reset)
if(reset)
   stateReg <= 0;
else
   stateReg <= loadState ? byteIn : (contrlAnotB ? Bout : Aout);

assign outByte = outRequest ? stateReg : 0;

Modified code:

// contrlAnotB == 0
assign Aout0 = Bout0[0] ? {Bout0[0], Bout0[1], Bout0[2], Bout0[3], Bout0[4], Bout0[5], Bout0[6], Bout0[7]} : {Bout0[6:0], Bout0[7]};
assign Bout0 = stateReg[0] ? {stateReg[6], stateReg[6], stateReg[4], stateReg[4], stateReg[2], stateReg[2], stateReg[0], stateReg[0]} : {stateReg[7], stateReg[7], stateReg[5], stateReg[5], stateReg[3], stateReg[3], stateReg[1], stateReg[1]};
// contrlAnotB == 1
assign Aout1 = stateReg[0] ? {stateReg[0], stateReg[1], stateReg[2], stateReg[3], stateReg[4], stateReg[5], stateReg[6], stateReg[7]} : {stateReg[6:0], stateReg[7]};
assign Bout1 = Aout1[0] ? {Aout1[6], Aout1[6], Aout1[4], Aout1[4], Aout1[2], Aout1[2], Aout1[0], Aout1[0]} : {Aout1[7], Aout1[7], Aout1[5], Aout1[5], Aout1[3], Aout1[3], Aout1[1], Aout1[1]};

always@(posedge clk or posedge reset)
if(reset)
   stateReg <= 0;
else
   stateReg <= loadState ? byteIn : (contrlAnotB ? Bout1 : Aout0);

assign outByte = outRequest ? stateReg : 0;

Just re-implement FnA and FnB for 2 cases when 'contrlAnotB' equal to 0 and 1. 'contrlAnotB' will be used once before DFF and no combinational loops will be.
Also you will get the same resource usage.

 

[Newdaddy06]

Just re-implement FnA and FnB for 2 cases when 'contrlAnotB' equal to 0 and 1. 'contrlAnotB' will be used once before DFF and no combinational loops will be.
Also you will get the same resource usage.

I don't believe this last sentence is correct. Your code implements fnA twice, and fnB twice.

You have
"assign Aout0 = Bout0[0] ? {Bout0[0], ..."
and also
"assign Aout1 = stateReg[0] ? {stateReg[0], ..."
which are both performing fnA.

In the code you've written, this may not result in any more logic, as fnA and fnB are trival as written. But I am looking for a general solution for any two arbitrarily-large combinational functions fnA and fnB.

Thank you for looking at this code and even more for coding a response. I may have to use your solution if I can find no better work-around, but in my case, the actual functions fnA and fnB are 'large' and so I'd like not to replicate them if I can avoid it.

 

[FvM]

In fact there's no other problem than getting an obviously inappropriate combinational loop warning. It must be considered as a restriction of the synthesis tool that is particularly designed to analyze asnychronous logic. Altera Quartus has the same problem here, by the way.

The normal way is to ignore some warnings. I don't think that anybody would instantiate extensive logic twice to get rid of a warning.

It seems to me that the problem is a bit theoretical though. If the the combinational logic blocks are arbitrarily-large as you say, you'll usually want pipeline registers around the functions to achieve reasonable design speed. This also eliminates the problem.

 

[Newdaddy06]

Does the existence of these loops detract from the optimization of "real" delay paths though? If so, that would be a problem.

Also, just having these loops appear in the logfiles and timing reports is a problem, in that other people on the project see them, and then they need to be explained. These loops will need to be inspected after every synthesis, to make sure they're not masking the presence of other, more serious timing loops. These kinds of erroneous errors add long-term costs to supporting and reusing code.

I agree with your statement regarding pipelining. But again, it seems expensive to add pipelining to a design simply to remove tool errors.

Someone had suggested that (at least for ASIC synthesis tools) there may be a way to define multiple cases to the synthesis tool, (e.g. contrlAnotB = '1', contrlAnotB = '0') such that it never has to consider the entire loop during an optimization. I will be investigating that later today.

 

[FvM]

Someone had suggested that (at least for ASIC synthesis tools) there may be a way to define multiple cases to the synthesis tool, (e.g. contrlAnotB = '1', contrlAnotB = '0') such that it never has to consider the entire loop during an optimization. I will be investigating that later today.

If actually intend to operate both functions without any pipelining, multiple cases is probably the only way to get the timing evaluated without the false combinational pathes. This won't remove the combational loop warnings though.

 

[Newdaddy06]

Unfortunately, I can't seem to find reference to synthesizing cases in the Synplify Pro UG. If someone knows howto do this, please let me know. Otherwise, I'll be stuck with combinational loop warnings for the rest of forever.

Thx, -