nickg / nvc

VHDL compiler and simulator
https://www.nickg.me.uk/nvc/
GNU General Public License v3.0
589 stars 75 forks source link

Case expression with for loop variable #902

Open Blebowski opened 5 days ago

Blebowski commented 5 days ago

I ran across an interesting issue. NVC does not seem to determine the range of iteration variable i correctly.

Consider following example:

entity iterator_bounds is
end entity iterator_bounds;

architecture test of iterator_bounds is

begin

    process
        variable higher_bound : integer := 3;
    begin
        for i in 1 to higher_bound loop
            case i is
            when 1 => report "'i' is 1";
            when 2 => report "'i' is 2";
            when 3 => report "'i' is 3";
            end case;
        end loop;

        wait;
    end process;

end architecture;

i can only get values from 1 to 3. However, NVC expects full integer range:

 nvc -a iterator_bounds.vhd -e iterator_bounds -r
** Error: missing choice for element -2147483648 to 0, 4 to 2147483647 of type INTEGER
    > iterator_bounds.vhd:12
    |
 12 |             case i is
    |             ^

If I replace the higher_bound with literal constant 3, then no error occurs. I guess NVC determines range at compile time in such case.

Obviously, adding others choice solves the issue, but I am not sure it should be needed. In GHDL this "works" without complaining. If I bump-up the value of higher_bound to 4 without adding corresponding case choice, GHDL accepts it too, and over-flows and when i=4, it prints the report for i=1 (which seems to me a GHDL bug).

The code works OK also in VCS. In Riviera PRO, I get:

Case expression is not of a locally static subtype. Use the choice 'others' or compile with -relax." "iterator_bounds.vhd"

If I recompile with -relax Riviera PRO takes the code. If I bump-up the higher_bound value to 4 without adding corresponding case alternative, it takes the code too, but does not print anything in the iteration when i=4.

I am not sure what does the LRM says on this topic, so this issue might be rubbish, but I would guess it is valid since GHDL, VCS and Riviera accept the code.

nickg commented 5 days ago

I'm pretty sure this error is correct. The subtype is not static (as the RiveraPRO message says), so the choices need to cover the full range since the value of higher_bound could change.

Questa reports basically the same error for this code:

Questa Intel Starter FPGA Edition-64 vcom 2021.2 Compiler 2021.04 Apr 14 2021
Start time: 23:04:42 on Jun 25,2024
vcom test.vhd 
-- Loading package STANDARD
-- Compiling entity iterator_bounds
-- Compiling architecture test of iterator_bounds
** Error (suppressible): test.vhd(12): (vcom-1339) Case statement choices cover only 3 out of 4294967296 cases.
** Note: test.vhd(22): VHDL Compiler exiting
End time: 23:04:42 on Jun 25,2024, Elapsed time: 0:00:00
Errors: 1, Warnings: 0

Because higher_bound is a variable its value could change e.g.

    process
        variable higher_bound : integer := 3;
    begin
        for i in 1 to higher_bound loop
            case i is
            when 1 => report "'i' is 1";
            when 2 => report "'i' is 2";
            when 3 => report "'i' is 3";
            end case;
        end loop;
        higher_bound := higher_bound + 1;
        wait;
    end process;

GHDL doesn't error on this either which is definitely wrong.

If you make higher_bound a constant then NVC and Questa accept it without errors.

Blebowski commented 5 days ago

OK, I see. Thanks for explanation.

Do you think it would be good to accept the behavior with -relax similar to Riviera ? If not, feel free to close the issue.