Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Case expression with for loop variable #902

Closed
Blebowski opened this issue Jun 25, 2024 · 3 comments
Closed

Case expression with for loop variable #902

Blebowski opened this issue Jun 25, 2024 · 3 comments

Comments

@Blebowski
Copy link
Sponsor Contributor

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
Copy link
Owner

nickg commented Jun 25, 2024

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
Copy link
Sponsor Contributor Author

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.

@nickg
Copy link
Owner

nickg commented Jul 10, 2024

--relaxed turns it into a warning now.

@nickg nickg closed this as completed Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants