01-24-2017 02:38 PM
As unlikely as it sounds, it appears that I have encountered a Boolean optimization error in Vivado. Here is the Verilog code
wire SumThresh = (Threshold | Threshold);
always @ (posedge dataclk) begin // To Filter threshold (prevent accidental resets)
SumThreshd <= SumThresh; // and insure that it only occurs on first sample above Threshold
SumThreshd2 <= SumThreshd; // Note that first two samples above threshold aren't going
end // to be peaks, because they average at least one sample below threshold
wire SumReset = SumThresh & !SumThreshd & !SumThreshd2;
wire SumPeakDet = SumThresh | SumThreshd | SumThreshd2;
wire [6:0] SumOpMode = (SumPeakDet) ? 7'b0110011 : 7'b0000011;
.CEA1(SumReset | (!SumCarryOut & SumPeakDet)), // Clock Enable for upper bits (Chan 2 - refl)
While it shouldn't matter to the resolution of this, Threshold is the P output of a DSP and the bits in question are sign bits of segmented parts of the register used to indicate when a threshold is exceeded (they work as expected). CEA1 is the A1 register chip enable of a DSP48E1 (7Z010). It needs to allow clocking to latch data, and to support a synchronous reset, hence the two terms ORd together. SumCarryOut is from the same DSP, used to detect a new peak that needs to be stored in A1, replacing the previous one.
The optimizer created a 5 input LUT and pulled the two terms of the first wire above into it. But the equations and truth table do not match the above Verilog. To keep this more readable let me make the following abbreviations:
I0 = SumCarryOut, called C3 below
I1 = SumThreshD2, called D2 below
I2 = Threshold, called T15 below
I3 = Threshold, called T23 below
I4 = SumThreshD, called D below
The equation generated (according to the properties truth table for the LUT in the schematic) was:
!D2 & T23 & !D + !C3 & D + !C3 & D2 & !D +!D2 & !T15 & !D // (equation 1)
The first and last terms are related to the reset portion and the second and third are related to the Data clocking portion. What I would have expected is
(T23 | T15 | D2 | D) & !C3 + (T23 | T15) * !D2 & !D // (equation 2)
which should factor to
T23 & !C3 + T15 * !C3 + D2 & !C3 + D & !C3 + T23 & !D2 & !D + T15 * !D2 & !D // (equation 3)
with some simplification possible. But equation 1 is not equivalent to equation 2 or 3. The truth table reveals three errors out of the 32 possible combinations:
!D & !T15 & T23 & D2 & C3 show up as 1 in the truth table. They should be 0. Since C3 is high, all the terms in equation 3 involving C3 are 0, leaving T23 & !D2 & !D + T15 & !D2 & !D both of which require D2 to be low. Since it is high, they also are 0.
The second one is:
D & T15 & !T23 & D2 & !C3. This shows up as 0 in the truth table, and should be 1. The second, third and fourth term of equation 3 are all true for this set of inputs, so the output should be 1.
And the third one is:
D & T15 & T23 & D2 & C3. It showed up as 1 and should have been 0. Because C3 is 1, those terms drop out, and because D is 1 the remaining terms drop out (They would also drop out because D2 was 1).
When I edited the LUT and put in equation 3, the rows of the table became correct and the equation it showed is:
!D2 & T2 & !D + !C3 & D + !C3 & D2 & !D + !D2 & !T2 & T3 ! D
I will try to refactor my Verilog to circumvent the problem, but the Boolean optimizer should probably be looked at to see why it generated this equation. (unless, of course I'm missing something).
01-24-2017 02:52 PM
Note: Changing the order of evaluation of one line resolved the problem:
.CEA1((!SumCarryOut & SumPeakDet) | SumReset), // Clock Enable for upper bits (Chan 2 - refl)
01-29-2017 08:04 AM
@whelm I haven't verified this but make sure that you realize Verilog uses short-circuiting in evaluation. Ie if SumReset is high the rest of the expression doesn't need to be evaluated (in your original line). This might change how logic is generated.
01-29-2017 03:28 PM
Somebody needs to verify the results. I don't think short-circuiting should impact the results of logic optimization. The bottom line is that the equations get mapped into a 5 input LUT with 32 possible states and the output for three of the states do not represent the logic equations. By reversing the order of the last line as shown, the optimization produces different output such that all 32 states have correct outputs. That's rather scary. In one sense it means every LUT in the design should be check to see if it was optimized correctly. More pragmatically, if anything does not appear to work right, related LUTs should be examined for correctness. It isn't something I would have expected. One would like to think that Boolean optimization of combinatorial expressions could be taken for granted. The process has been automated for over three decades. But code bugs can always be introduced--and need to be caught and eliminated.