zachjs / sv2v

SystemVerilog to Verilog conversion
BSD 3-Clause "New" or "Revised" License
498 stars 50 forks source link

Accessing interface parameters fails when module is instantiated with identical port names #212

Closed dsdve closed 1 year ago

dsdve commented 1 year ago

The following example fails to convert:

interface foo #(int WIDTH = 8);
endinterface

module bar(foo f);
    if (f.WIDTH > 1)
        $error("oh no!");
endmodule

module test();
    foo f();
    bar b(.f(f));
endmodule

With the error

sv2v: inlining instance "b" of module "bar" would make expression "f.WIDTH" used in "b" resolvable when it wasn't
previously, within scope bar, near iftest.sv:5:2

CallStack (from HasCallStack):
  error, called at src/Convert/Scoper.hs:377:22 in main:Convert.Scoper

Though clearly, f.WIDTH is resolvable before inlining.

However, if the foo-instance in test is renamed to anything but the port name on bar, it converts just fine. This behavior seems to be limited to interface parameters (including localparam), since doing a similar thing with interface signals works just fine.

Given some pointers I'd be happy to try and fix this myself, though my Haskell skills might be lacking.

dsdve commented 1 year ago

The same issue arises with functions:

interface foo #(int WIDTH = 8);
    function get_width();
        return WIDTH;
    endfunction
endinterface

module bar(foo f);
    if (f.get_width() > 1)
        $error("oh no!");
endmodule

module test();
    foo f();
    bar b(.f(f));
endmodule

Assigning the parameter to a signal in the interface seems to be a decent workaround, though at least Verilator will complain when accessing the signal:

interface foo #(int WIDTH = 8);
    int width;
    assign width = WIDTH;
endinterface
zachjs commented 1 year ago

[sorry for the earlier mis-click] Thanks for filing this! Indeed the errors raised here are incorrect. I have an idea of how to fix this properly, but I'll need some time to work on it. If you are blocked in the meantime, you could short circuit the check in checkExprResolution locally (e.g., add False &&).

dsdve commented 1 year ago

Thanks! The workaround seems to work quite well for now.

zachjs commented 1 year ago

I pushed 59b416f9b4a79e83b295793fd4807d7ca96c51c2, which should fix the original issue. Can you let me know if it works for you?

dsdve commented 1 year ago

It does work for me. Thank you!