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

Support unpacked array parameters in functions with complex assign #2639

Closed
aignacio opened this issue Nov 12, 2020 · 9 comments
Closed

Support unpacked array parameters in functions with complex assign #2639

aignacio opened this issue Nov 12, 2020 · 9 comments
Labels
area: elaboration Issue involves elaboration phase resolution: fixed Closed; fixed type: feature-IEEE Request to add new feature, described in IEEE 1800

Comments

@aignacio
Copy link

Hi all,

I'd like to understand from the snippet below what's the unsupported feature I'm trying to use cause there's nothing special on the implementation. It only iterates through the array defined as a localparam in a function based on an input parameter...

function automatic s_noc_addr_t axi_dec_noc(logic [`AXI_ADDR_WIDTH-1:0] axi_addr);
    s_noc_addr_t noc_addr;
    noc_addr.x_dest = x_width_t'('0);
    noc_addr.y_dest = y_width_t'('0);
    noc_addr.invalid = '1;
    for (int i=0;i<(NOC_SIZE-1);i++) begin
      if (axi_addr >= noc_addr_map[`ADDR_BASE][i] && axi_addr <= noc_addr_map[`ADDR_UPPER][i]) begin
        noc_addr.x_dest  = x_width_t'(noc_addr_map[`X_ADDR][i]);
        noc_addr.y_dest  = y_width_t'(noc_addr_map[`Y_ADDR][i]);
        noc_addr.invalid = '0;
      end
    end

    return noc_addr;
endfunction
`ifndef ADDR_NOC_MAPPING
    `define ADDR_VEC_MAPPING      1
    `define ADDR_BASE             0
    `define ADDR_UPPER            1
    `define X_ADDR                2
    `define Y_ADDR                3
    localparam [`AXI_ADDR_WIDTH-1:0] noc_addr_map [4][(`NOC_SIZE)-1:0] = '{
      '{'h0000, 'h1000, 'h2000, 'h3000, 'h4000, 'h5000, 'h6000, 'h7000, 'h8000, 'h9000, 'hA000, 'hB000},
      '{'h0FFF, 'h1FFF, 'h2FFF, 'h3FFF, 'h4FFF, 'h5FFF, 'h6FFF, 'h7FFF, 'h8FFF, 'h9FFF, 'hAFFF, 'hBFFF},
      '{   'd0,    'd0,    'd0,    'd0,    'd1,    'd1,    'd1,    'd1,    'd2,    'd2,    'd2,    'd2},
      '{   'd0,    'd1,    'd2,    'd3,    'd0,    'd1,    'd2,    'd3,    'd0,    'd1,    'd2,    'd3}
    };
`endif
verilator -CFLAGS " -g3 -Wall -Werror -DWAVEFORM=\"/tmp/ravenoc.fst\" -DEN_TRACE=\"1\"" --top-module ravenoc --Mdir output --Wno-UNOPTFLAT -O3 --exe --threads  4 --trace --clk                 clk --trace-fst --trace-structs --trace-threads 4 --trace-underscore --trace-depth                      10000 --trace-max-array   10000 --trace-max-width 10000 --cc incdir+src/include +define+SIMULATION src/include/ravenoc_defines.svh src/include/ravenoc_structs.svh src/include/ravenoc_axi_structs.svh src/include/ravenoc_pkg.sv  src/ni/axi_slave_if.sv src/ravenoc.sv src/include/ravenoc_pkg.sv src/router/fifo.sv src/router/output_module.sv src/router/router_if.sv src/router/router_ravenoc.sv src/router/rr_arbiter.sv src/router/vc_buffer.sv src/router/input_router.sv src/router/input_module.sv src/router/input_datapath.sv tb/testbench.cpp -o ravenoc
%Error-UNSUPPORTED: src/include/ravenoc_defines.svh:99:25: Unsupported: Parameters in functions with complex assign
                                                         : ... In instance ravenoc
   99 |     localparam [32-1:0] noc_addr_map [4][(3*4)-1:0] = '{
      |                         ^~~~~~~~~~~~
%Error: Exiting due to 1 error(s)
        ... See the manual and https://verilator.org for more assistance.
make: *** [output/Vravenoc.mk] Error 1
@aignacio aignacio added the new New issue not seen by maintainers label Nov 12, 2020
@wsnyder
Copy link
Member

wsnyder commented Nov 12, 2020

Looking at the error source, this suggests the localparam is declared inside a function - is it? If so, move to outside the function into the module itself.

@aignacio
Copy link
Author

Hi @wsnyder it's not, these defines are part of a package that's imported at the top of the module in a local scope.

@wsnyder
Copy link
Member

wsnyder commented Nov 12, 2020

Can you please convert this into a standalone test case and I'll take a look. Thanks.

@wsnyder wsnyder added status: asked reporter Bug is waiting for reporter to answer a question and removed new New issue not seen by maintainers labels Nov 18, 2020
@aignacio
Copy link
Author

aignacio commented Nov 18, 2020

Tks @wsnyder
Here's a standalone test.
https://www.edaplayground.com/x/Cdp3

package test_pkg;
    localparam [31:0] test_arr [4][11:0] = '{
        '{'h0000, 'h1000, 'h2000, 'h3000, 'h4000, 'h5000, 'h6000, 'h7000, 'h8000, 'h9000, 'hA000, 'hB000},
        '{'h0FFF, 'h1FFF, 'h2FFF, 'h3FFF, 'h4FFF, 'h5FFF, 'h6FFF, 'h7FFF, 'h8FFF, 'h9FFF, 'hAFFF, 'hBFFF},
        '{   'd0,    'd0,    'd0,    'd0,    'd1,    'd1,    'd1,    'd1,    'd2,    'd2,    'd2,    'd2},
        '{   'd0,    'd1,    'd2,    'd3,    'd0,    'd1,    'd2,    'd3,    'd0,    'd1,    'd2,    'd3}
    };

    typedef struct packed{
        logic   [7:0]   val_1;
        logic   [7:0]   val_2;
    } test_ret_t;
endpackage

module tb import test_pkg::*; ();
    logic clk;
  

    initial begin : clk_gen
        clk = 0;
        forever clk = #5 ~clk;
    end


    function automatic test_ret_t test_f(logic [31:0] val);
        test_ret_t temp;

        temp = test_ret_t'(0);
        for (int i=0;i<12;i++) begin
            if (val >= test_arr[0][i] && val <= test_arr[1][i]) begin
                temp.val_1 = test_arr[2][i];
                temp.val_2 = test_arr[3][i];
            end
        end
        return temp;
    endfunction

  	test_ret_t temp;
    logic [31:0] random;
  
    initial begin
        for (int i=0;i<10;i++) begin
          random <= $urandom_range(0,'hbfff);
          temp <= test_f(random);
          $display("rand: %h / Values -> val_1: %d / val_2: %d",random,temp.val_1,temp.val_2); 
          @(posedge clk);
        end
      $finish;
    end
  
endmodule

@wsnyder wsnyder added area: elaboration Issue involves elaboration phase status: ready Issue is ready for someone to fix; then goes to 'status: assigned' type: feature-IEEE Request to add new feature, described in IEEE 1800 and removed status: asked reporter Bug is waiting for reporter to answer a question labels Nov 18, 2020
@wsnyder
Copy link
Member

wsnyder commented Nov 18, 2020

Thanks, that makes sense, probably multidimensional arrays need some extra work in V3Simulate.h.

Would you be willing to take a look at patching this?

@aignacio
Copy link
Author

Hey @wsnyder I don't know if I have enough bkg to change such scope but I can take a look for sure, just let me know which file specific that deals with array manip.

@aignacio
Copy link
Author

btw, is there any diff if it's packed vs unpacked for verilator compatibility on this example?

@wsnyder
Copy link
Member

wsnyder commented Nov 18, 2020

Packed should work fine, it's forming the unpacked array that causes the error.

Let me run it and suggest a path.

@wsnyder wsnyder changed the title Unsupported: Parameters in functions with complex assign Support unpacked array parameters in functions with complex assign Nov 18, 2020
@wsnyder wsnyder added resolution: fixed Closed; fixed and removed status: ready Issue is ready for someone to fix; then goes to 'status: assigned' labels Nov 19, 2020
@wsnyder
Copy link
Member

wsnyder commented Nov 19, 2020

Turned out just the assertion was broken. Fixed. Thanks for the test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: elaboration Issue involves elaboration phase resolution: fixed Closed; fixed type: feature-IEEE Request to add new feature, described in IEEE 1800
Projects
None yet
Development

No branches or pull requests

2 participants