steveicarus / iverilog

Icarus Verilog
https://steveicarus.github.io/iverilog/
GNU General Public License v2.0
2.8k stars 520 forks source link

Verilog: function without result won't hold previously returned value #413

Open Lesmiscore opened 3 years ago

Lesmiscore commented 3 years ago

Hi,

Given following code:

module multiplexer(din,sel,dout);
  input [3:0] din;
  input [1:0] sel;
  output dout;

  function func;
    input [3:0] in;
    input [1:0] s;

    begin
      if(s==2'h0)
        func=in[0];
      if(s==2'h1)
        func=in[1];
      if(s==2'h2)
        func=in[2];
      if(s==2'h3)
        func=in[3];
    end
  endfunction
  assign dout=func(din,sel);
endmodule

module multiplexer_test;
  wire [3:0] din=4'b1001;
  reg [1:0] sel;
  initial begin
    $dumpfile("dump.lxt");
    $dumpvars(0,multiplexer_test);
    #0 sel=2'b00;
    #2 sel=2'b10;
    #2 sel=2'bxx;
    #2 sel=2'bzz;
    #2 $stop;
  end

  wire dout;
  multiplexer c1(din,sel,dout);
endmodule

and command line:

$ iverilog -g2001 -s multiplexer_test -o multiplexer_test.v

When multiplexer got value on sel that fails all ifs (e.g. 2'bxx), the returned value is not same as previous one.

As IEEE1364: 2001 states,

10.3.2 Returning a value from a function

The function definition shall implicitly declare a variable, internal to the function, with the same name as the function. This variable either defaults to a 1-bit reg or is the same type as the type specified in the function declaration. The function definition initializes the return value from the function by assigning the function result to the internal variable with the same name as the function. It is illegal to declare another object with the same name as the function in the scope where the function is declared. Inside a function, there is an implied variable with the name of the function, which may be used in expressions within the function. It is, therefore, also illegal to declare another object with the same name as the function inside the function scope.

the function should hold the last value at begin of function on -g2001 flag.

Thanks.

caryr commented 3 years ago

I will need to do some testing because there is some subtlety in this. Also the standard reference you pasted does not address this holding the value.

caryr commented 3 years ago

It looks like Icarus is working as expected. I think your misunderstanding is that there is a single function call that somehow is remembering the value between calls and that is not the case. The function is called whenever din or sel change. The implicit function variable defaults to x and since it is never assigned a value for the x/z sel cases that is what gets assigned to dout.

I tested a simplified version of this on a commercial simulator and it gave the same result as Icarus.

Unless you can provide a reference to why this should work differently I will close this as an invalid report in a few days to give you some time to reply while it is still open.

Lesmiscore commented 3 years ago

image With the same code above on Cver and ModelSim Quartus II (w/o simplification), they give me different result than Icarus Verilog.

I'm sorry if I'm confusing you, this issue is about return value of func function, so it's not about multiplexer module itself.

martinwhitaker commented 3 years ago

@caryr, no, in a non-automatic function, all function variables, including the implicit one, are static and shared by all activations of that function within a module.

I wrote this to Steve back in 2016:

Whilst adding support for assignment operators on real values, I noticed you've recently changed the vvp code generator to keep function return values on the stack rather than in a local variable. Whilst this is no doubt good for performance, it does break some aspects of behaviour, namely that the implicit variable that holds the function return variable is static and is visible from outside the function. The attached (admittedly contrived) example demonstrates this. v10 gives the expected behaviour, devel fails.

I think you could safely apply the optimisation to automatic functions.

P.S. Before you ask, yes, other (big-name) simulators give the expected result.

caryr commented 3 years ago

Okay, that makes sense. VCS gives the same behavior as Icarus and I checked with xprop turned off. In general I trust incisive and VCS more than the others, but it seems like this is a VCS bug also.

Lesmiscore commented 3 years ago

any progress on this?