XST vhdl adder with carry out : broken carry chain

Hi Bart,

I have observed exactly the same symptoms, but with another synthesiser (Synplify Pro). I think the problem is with MAP rather than the synthesiser. MAP doesn't seem to handle the last bit in an odd length carry chain well.

My solution (as usual) was to create a module that instantiated the unisim components directly (with RLOC attributes), so that I got exactly what I wanted.

BTW, this bug has been around for at least three years. At the time, I was having a lot of trouble implementing a 17 and 33 bit adders. I came to the conclusion that Xilinx's test suite probably only has even length adders.

Regards, Allan.

--
Rick "rickman" Collins

rick.collins@XYarius.com

Adding

to

it

Rick, it was a timing issue due to poor mapping, rather than a functional issue.

Regards, Allan.

That would be the problem the OP had. But the problem Symon described is a coding issue. But then he replied to my post and I may not have understood what he meant.

--
Rick "rickman" Collins

rick.collins@XYarius.com

Thanks for the help guys.

I agree with Symon : it's a timing issue, functionality is 100% ok. And yes, it only happens when the carry out is the odd bit, not even. I also believe that the problem is MAP related, not XST (although I can not prove this). I tried with the "KEEP" attribute (which is XST equivalent of the synplify "syn_keep") : unfortunately this does not change anything. Also USE_CARRY_CHAIN and some other attributes I tried didnt do the trick. I was hoping there would be a clean and simple workaround, because I have a design with some 120 - 140 adders in it, I would hate to give up on readable code. But right now I would be happy with something that would work at all.

If I do find some solution I will surely post it here.

Bart.

You don't need to change a line of your HDL source.

Write a (e.g. Perl) script to read the EDIF and locate the carry chains. From that you can generate a UCF which has an RPM for each carry chain. This will force MAP to do the right thing.

Regards, Allan.

Hello Bart,

We've run your code and there is indeed a map packing problem. The MSB is simply a FF driven by COUT of the previous slice. I see two related problems with the packing:

1. FF "tmp1_16" should be packed into a slice utilizing the CIN pin through the XORCY BEL as an extension of the carry chain but is not.

1. Instead, FF "tmp1_16" is being packed into the FFY BEL of the carry chain slice that is driving it, displacing "tmp1_15" from its correct packing location.

I've logged CR 192265 for these issues. Meanwhile, I don't see a work around for first issue except to extend the carry chain by one bit as you mentioned or possibly by instantiating an XORCY and FF to terminate the carry chain. The second issue can be controlled with a map packing constraint such as:

INST "tmp1_16" XBLKNM = XLNX_WA ;

I'll post again when we have a fix date scheduled.

Regards, Bret Wade Xilinx Product Applications

Thanks Bret, makes you wonder why Allan or I never opened a webcase about this rather than work around the problem. Could've been fixed years ago. Bloody hardware engineers! Cheers, Syms.

On July 30 I posted:

Just to follow up on this issue, it has been fixed for the next service pack, 6.3i SP2 which will become available in mid October.

Bret

I am not sure about verilog. If I assign to a variable the same width as the operands of +, I get warnings about truncated bits.

One book says that {cout, sum} = in1 + in2 + cin; will generate a full adder.

Another book says that the width for + is maximum of the width of the two operands.

cross posted to comp.lang.verilog, to see if anyone there can say.

-- glen

Hello Bart,

Please find hereafter a piece of code that works for an incrementer. I had exactly the same problem as you have until I finally coded that way. I am not sure that works for A + B as it does for A + 1.

Hope it can help.

A. Beaujean

Library ieee ; Use ieee.std_logic_1164.all ; Use ieee.std_logic_unsigned.all ; Use ieee.std_logic_arith.all ;

-- Package TDC Is -- Package for Time to Digital Conversion

-- Component TDC_SUM_RE Is -- Builds the sum of all ones and ((all ones -

1) + Tdc_In -- to force the mapper to use the carry chain. -- Outputs of the carry chain are registered in the same slice -- (Rising Edge) Generic(Width : integer) ; -- Width of Component, nbr of stages Port (Reset_Not : in std_logic ; Clock : in std_logic ; -- Clock Tdc_In : in std_logic ; Output : out std_logic_vector((Width-1) downto 0)) ;

-- End Component ;

-- Component TDC_SUM_FE Is -- Builds the sum of all ones and ((all ones -

1) + Tdc_In -- to force the mapper to use the carry chain. -- Outputs of the carry chain are registered in the same slice -- (Falling Edge) Generic(Width : integer) ; -- Width of Component, nbr of stages Port (Reset_Not : in std_logic ; Clock : in std_logic ; -- Clock Tdc_In : in std_logic ; Output : out std_logic_vector((Width-1) downto 0)) ;

-- End Component ;

-- End TDC ;

-- Library ieee ; Use ieee.std_logic_1164.all ; Use ieee.std_logic_unsigned.all ; Use ieee.std_logic_arith.all ;

--

--

---------------------------------------------------------------------------------------

--

-- HANDLING THE TDC_SUM_RE COMPONENT

--

---------------------------------------------------------------------------------------

-- Entity TDC_SUM_RE Is -- Builds the sum of all ones and ((all ones -

1) + Tdc_In -- to force the mapper to use the carry chain. -- Outputs of the carry chain are registered in the same slice -- (Rising Edge) Generic(Width : integer) ; -- Width of Component, nbr of stages Port (Reset_Not : in std_logic ; Clock : in std_logic ; -- Clock Tdc_In : in std_logic ; Output : out std_logic_vector((Width-1) downto 0)) ;

-- End TDC_SUM_RE ;

Architecture Arch_Of_TDC_SUM_RE Of TDC_SUM_RE Is

--

-- Components declarations

-- Component FDC -- Rising edge FF Port (D,C,CLR: in std_logic ; Q : out std_logic ); End Component ;

--

-- Signals

-- Signal Chain : std_logic_vector((Width-1) downto 0); Signal Regs : std_logic_vector((Width-1) downto 0); Signal Reset : std_logic ; Signal All_Ones : std_logic_vector((Width-1) downto 0) ; Signal All_Zeroes_M1 : std_logic_vector((Width-1) downto 1) ;

--

-- Attributes

-- Attribute KEEP : string ; Attribute KEEP of Chain : signal is "true" ;

-- Begin

--

-- Components instantiation

-- FDC_All: For I in 0 to (Width-1) Generate FDC_One: FDC Port Map (C => Clock, D => Chain(I), CLR => Reset, Q => Regs(I)); End Generate ;

--

--

--------------------------------------------------------------------------------------

--

-- CONCURRENT STATEMENTS

--

--------------------------------------------------------------------------------------

--

-- The trick used is to generate a sum of all ones and the concatenation of

-- (all ones minus 1) and the input signal.

-- All_Ones '1') ; All_Zeroes_M1 '0') ; Chain

In verilog,

{cout, sum} = in1 + in2 + cin;

will perform an addition of width max({cout, sum}, in1, in2, cin).

The statement that "the width for + is maximum of the width of the two operands" is true if "a+b" is in isolation ('self-determined' in Verilog terms).

However, for c = a+b, then the operands are extended to width max(a,b,c), and only then is the addition performed.

Shalom

glen herrmannsfeldt wrote:

-- Shalom Bresticker Shalom.Bresticker @freescale.com Design & Verification Methodology Tel: +972 9 9522268 Freescale Semiconductor Israel, Ltd. Fax: +972 9 9522890 POB 2208, Herzlia 46120, ISRAEL Cell: +972 50 5441478

[ ]Freescale Internal Use Only [ ]Freescale Confidential Proprietary