nickg / nvc

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

Pure function calling 'impure' procedure #848

Closed avelure closed 4 months ago

avelure commented 4 months ago

There seems to be some discrepancies for the checking of pureness. In the MWE below the functionality is the same, for the function calls test, test2 and test3, but proc calls an impure function to fetch the shared variable value and thus gets away without an error.

library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;

package test_pkg is
  shared variable v_test : natural;
  impure function func return natural;
  procedure proc(var : out natural);
  pure function test return natural;
  procedure proc2(var : out natural);
  pure function test2 return natural;
  procedure proc3(var : out natural);
  pure function test3 return natural;

end package test_pkg;

package body test_pkg is
  impure function func return natural is
  begin
    return v_test;
  end function;

  procedure proc(var : out natural) is
  begin
    var := func;
  end procedure;

  procedure proc2(var : out natural) is
  begin
    var := v_test;
  end procedure;

  procedure proc3(var : out natural) is
  begin
    proc2(var);
  end procedure;

  pure function test return natural is
    variable v_var : natural;
  begin
    proc(v_var);
    return v_var;
  end function;

  pure function test2 return natural is
    variable v_var : natural;
  begin
    proc2(v_var);
    return v_var;
  end function;

  pure function test3 return natural is
    variable v_var : natural;
  begin
    proc3(v_var);
    return v_var;
  end function;
end package body;
$ nvc --std=08 -a --relaxed test_pure3.vhd
** Warning: shared variable V_TEST must have protected type
   > C:\proj\public\minimal\test_pure3.vhd:6
   |
 6 |   shared variable v_test : natural;
   |                   ^^^^^^
** Error: pure function TEST2 cannot call procedure PROC2 which references a shared variable
    > C:\proj\public\minimal\test_pure3.vhd:48
    |
 48 |     proc2(v_var);
    |     ^^^^^^^^^^^^^
** Error: pure function TEST3 cannot call procedure PROC3 which references a shared variable
    > C:\proj\public\minimal\test_pure3.vhd:55
    |
 55 |     proc3(v_var);
    |     ^^^^^^^^^^^^^

I also notice that the pureness check creates an error even with --relaxed. I guess it should be possible to just mark the function as impure when it is encountered and add a warning with --relaxed? There is a few instances of this issue in UVVM that needs to be fixed.

Another thing; I don't know how much the impure functions affects performance, but maybe it would be useful to warn the user about impure functions that don't need to be impure. I can create a new issue if you think it is useful and doable to implement.

nickg commented 4 months ago

There seems to be some discrepancies for the checking of pureness. In the MWE below the functionality is the same, for the function calls test, test2 and test3, but proc calls an impure function to fetch the shared variable value and thus gets away without an error.

I've fixed this so the code above now produces an error but it's quite easily defeated by changing the order so that the "proc" bodies come after the "test" bodies. I noticed ModelSim has exactly the same behaviour though.

I also notice that the pureness check creates an error even with --relaxed. I guess it should be possible to just mark the function as impure when it is encountered and add a warning with --relaxed? There is a few instances of this issue in UVVM that needs to be fixed.

Yes it should be fine, I've changed this.

Another thing; I don't know how much the impure functions affects performance, but maybe it would be useful to warn the user about impure functions that don't need to be impure. I can create a new issue if you think it is useful and doable to implement.

At the moment no, the pureness isn't used by the code generator. Although it ought to be possible to optimise, say, two calls to F(3) to a single call and then reuse the result.