Overflow Detected in case statement

[koog1000]

I have a testbench setup that reads from a plain text file and performs some action based on the information in that plain text file.

I compile and elaborate my testbench with:

datetime=`date`
GHDL_OPTS="-frelaxed-rules -P$(dirname $PRJ)/../xilinx-vivado --std=08 --ieee=synopsys -g"

ghdl -i $GHDL_OPTS --work=$TOP $PRJ_FILES
ghdl -m $GHDL_OPTS --work=$TOP $TOP
ghdl -r $GHDL_OPTS -v --work=$TOP $TOP -gdatetime="$datetime" --wave=$TOP.ghw

which produces the following output:

max2084/../../hdl/max2084_lvds_if.vhd:226:13:warning: instance "iserdese3_inst" of component "iserdese3" is not bound [-Wbinding]
max2084/../../hdl/max2084_lvds_if.vhd:57:14:warning: (in default configuration of max2084_lvds_if(rtl))
max2084/../../hdl/max2084_lvds_if.vhd:226:13:warning: instance "iserdese3_inst" of component "iserdese3" is not bound [-Wbinding]
max2084/../../hdl/max2084_lvds_if.vhd:57:14:warning: (in default configuration of max2084_lvds_if(rtl))
Linking in memory
Starting simulation
...
<several warnings from the precompiled Vivado unisim library that state the simulation behavior of the IDELAYE3 may not match real behavior, which is irrelevant for this simulation>
...
ghdl:error: overflow detected
in process .tb_max2084_ctrl_logic(bench).stimulus
  from: process tb_max2084_ctrl_logic.tb_max2084_ctrl_logic(bench).stimulus at tb_max2084_ctrl_logic.vhd:218
ghdl:error: simulation failed

The relevant snippet from the testbench is (the error line isn't actually commented out in the real code):

        case sig_name is
            -- Outputs
            when "rdata               " => sim_package.check_output(out_file, test_name(1 to test_name_len), "rdata", rdata, value);  -- JUST EXECUTED
            when "m2amm_addr          " => sim_package.check_output(out_file, test_name(1 to test_name_len), "m2amm_addr", m2amm_addr, value);  -- ERROR HERE
            when "m2amm_init_axi      " => tmp := '0' & m2amm_init_axi; sim_package.check_output(out_file, test_name(1 to test_name_len), "m2amm_init_axi", tmp, value);
            when "m2amm_burst_len     " => sim_package.check_output(out_file, test_name(1 to test_name_len), "m2amm_burst_len", m2amm_burst_len, value);
            when others => assert false report "Unknown signal name: '" & sig_name & "'" severity failure;
        end case;

The specific case that it just ran was the "rdata" case and the line that errors is the "m2amm_addr". If I switch the test to check any other case then the 'overflow error' occurs always on the line after the line that just executed, except if the case is the last case before the others clause. I can reorder the test cases and whichever case is right before the others clause will always work and any other case will always report and overflow error on the line after it. This simulation can also be tested with Vivado Simulator and it runs as expected. This makes me think it isn't an issue with my VHDL but some corner case with GHDL.

I can't figure out how to get any more debugging information out of this. How do I go about narrowing down where the problem is?

[koog1000]

I should add my GHDL info

$ ghdl -v

GHDL 0.37 (tarball) [Dunoon edition]
 Compiled with GNAT Version: 7.5.0
 mcode code generator
Written by Tristan Gingold.

Copyright (C) 2003 - 2020 Tristan Gingold.
GHDL is free software, covered by the GNU General Public License.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

[tgingold]

Is it possible to either publish the whole design or to create a small reproducer ? In particular, what does happen if you replace the call statements in the case by an assertion ?

[eine]

This might not be relevant for this issue, but I recall previous problems because of the order of the options: -frelaxed-rules -P$(dirname $PRJ)/../xilinx-vivado --std=08 --ieee=synopsys -g. The point is that --std might reset some options (such as the relaxed rules). Hence, it is better to use it first: --std=08 --ieee=synopsys -frelaxed-rules -P$(dirname $PRJ)/../xilinx-vivado -g. See https://ghdl.github.io/ghdl/using/InvokingGHDL.html?highlight=std#cmdoption-ghdl-std.

[koog1000]

I was in process of writing a minimal example and when I run with the minimal example GHDL is now going between two different error messages: "overflow detected" or "bound check error". This seems promising that it will lead to towards a solution (and potentially points to some issue in my VHDL). I'm going to continue debugging this.

In the meantime I uploaded the minimal reproducer if you want to take a look and find something obvious. You can run the sim with the following command in the extracted directory:

./run_sim_ghdl.sh -p full_adder/tb_full_adder.prj -t tb_full_adder

full_adder.zip

[tmeissner]

Just a hint to the adder code - I know It's only a MWE, but maybe the advice is valuable: In clocked processes you should only put the (potential) async reset and the clock signals in the sensitivity list.

This is a classic example of code which behaviour could differ between simulation & synthesis:

adder: process(all)
begin
    if rising_edge(Clk) then
        S <= A XOR B XOR Cin ;
        Cout <= (A AND B) OR (Cin AND A) OR (Cin AND B);
    end if;
end process;

Because the assigments are guarded by the if it shouldn't imply none desired behaviour, but better:

adder: process(Clk)
begin
    if rising_edge(Clk) then
        S <= A XOR B XOR Cin ;
        Cout <= (A AND B) OR (Cin AND A) OR (Cin AND B);
    end if;
end process;

If you already know that: forget my advice 😆

[koog1000]

Ok, I figured it out the issue is with my read_data procedure. This procedure has a couple variable <name>: out integer; parameters that are used to signify the length of certain sub strings. However, not all of these integers are updated every time the procedure is called, they only update if the relevant sub string is in the string that was read in, but every time the procedure is called GHDL reinitializes these integers to the minimum value. In the Vivado simulator these integers hold their previous value if they are not modified in the procedure, but this was easy enough to work around so the issue is fixed now.

[tgingold]

I think that unset out parameters are one of the most obscur point of the language. I am not sure it is well defined. So for portability, don't use an unset out parameter.

[tgingold]

I don't really see the difference. The assignments will be executed only in case of rising edge.

[koog1000]

So for portability, don't use an unset out parameter.

Yeah that makes sense.

I don't really see the difference. The assignments will be executed only in case of rising edge.

I'm assuming you're talking about @tmeissner 's comment. I agree that I don't see how those two cases would simulate differently. @tmeissner , if you could elaborate (or make an example) that would potentially be insightful.

This might not be relevant for this issue, but I recall previous problems because of the order of the options ...

@eine Thanks for putting this in, you're right it didn't make a difference in this case but I did run into that issue earlier today and it was helpful to know to try to reorder the options.

[tmeissner]

As I wrote it isn't a problem in this case. Maybe a performance issue during simulation, as the process is activated each time a signals which is read in the process changes. But I'm not sure with the signal reads inside the if condition, as I don't completely understand the LRM in this case.

VHDL-08 LRM, 11.3:

If the process sensitivity list is specified using the reserved word all, then the sensitivity list of the wait statement is constructed by taking the union of the sets constructed from each of the statements in the process by applying the following rules: ... — For each assignment statement, apply the rule of 10.2 to each expression occurring in the assignment, including any expressions occurring in the index names or slice names in the target, and construct the union of the resulting sets. — For each if statement, apply the rule of 10.2 to each condition and apply this rule recursively to each sequence of statements within the if statement, and construct the union of the resulting sets.

I assume, the if condition is equivalent to the condition clause in the wait statement definition in 10.2:

wait_statement ::= [ label : ] wait [ sensitivity_clause ] [ condition_clause ] [ timeout_clause ] ; ... The condition clause specifies a condition that shall be met for the process to continue execution. If no condition clause appears, the condition clause until TRUE is assumed.

So, in this case there even wouldn't be a performance difference. If I understood that correctly.

[eine]

As far as I understand, @tmeissner's suggestion is a good practice that we should all apply, instead of relying on tools being too clever. "Ensure that the sensitivity list of a sync register is only the clock and the (optional) async reset signals" is a good recommendation from an experienced user. My understanding is that all is mostly useful for large combinatorial processes, such as the ones that arise when FSM machines are described.

I had a similar experience with defining the range of integers/std_logic_vectors, or "just using 16 bits and letting the synthesiser optimise". From a software perspective, it might be pointless to do an effort for thinking the range of each signal in the design. However, IMHO that's what a HDL is meant for.

[koog1000]

I'm still not clear on when using the all reserved word in a process would cause a difference in behavior between the simulation and synthesis. I really like this feature of VHDL-08 and I use it essentially for every process I write to remove annoying warnings. I used to only put the clock in the sensitivity list, but that creates a lot of warnings (at least in Xilinx toolsets).

What is an example where using <label>: process(all) would cause issues?

[eine]

I think it's more of a theoretical issue than a practical one. That is, preferring explicit descriptions over implicit behaviour. If something goes wrong with all, you cannot really know whether the issue is in the tool or in your code. However, if something goes wrong with Clk, almost certainly the issue is somewhere else. That's why it is desirable to use all when it provides an advantage (reduce verbosity and/or increase readability), not as a catch 'em all solution. Of course, this is just style preference.

[eine]

to remove annoying warnings. I used to only put the clock in the sensitivity list, but that creates a lot of warnings (at least in Xilinx toolsets).

I'm curious about which warnings you get by using Clk instead of all, since the former is the very traditional way of describing registers.

[koog1000]

That's why it is desirable to use all when it provides an advantage (reduce verbosity and/or increase readability)

I'm curious about which warnings you get by using Clk instead of all

If you only put Clk in your sensitivity list the xilinx toolset will create a warning for every other signal that is in the RHS of an assignment. To get rid of the warnings you need to place every signal that is used in an assignment in the sensitivity list, or use all.

Using all is much easier. It also increases readability by not having overly verbose sensitivity lists and, as you've said, doesn't have any practical issues.

[eine]

I'd say that's a poor implementation on Xilinx's side... Does it happen with VHDL 2008 only or with VHDL 1993 too?

[koog1000]

It produces warnings for both VHDL-93 and -08.