MikePopoloski / slang

SystemVerilog compiler and language services
MIT License
579 stars 129 forks source link

type support for parameterized virtual interfaces #1042

Closed shivang-chipstack closed 2 months ago

shivang-chipstack commented 2 months ago
          > It sounds like you're inspecting an uninstantiated instance of the interface? slang will automatically create these to ensure that the body gets some checking even if the interface is unused in the design, but parameters are forced to invalid values since it's not a real instantiation and no param values were provided. You can see this in the API by looking at the `isUninstantiated` member of the instance body.

If you inspect an actual instance of that interface it should have the correct types.

Thank you for the explanation on parameterized interfaces. However, rather than assuming some invalid/forced value for an uninstantiated interface, we should provide with default value in type. To give little more context, consider below code:

interface my_intf # (
  IN_WIDTH = 8,
  OUT_WIDTH = 8
  )
  ();
  logic [IN_WIDTH-1:0] d_in;
  logic [OUT_WIDTH-1:0] d_out;

  modport sp (input d_in, output d_out);
  modport mp (output d_in, input d_out);
endinterface

module DUT(
    input logic clk,
    input logic reset,
    my_intf.sp intf_inst
);
    logic [7:0] d_flop1;
    always @(posedge clk) begin
        if (reset) begin
            intf_inst.d_out <= 0;
            d_flop1 <= 0;
        end else begin
            d_flop1 <= intf_inst.d_in;
            intf_inst.d_out <= d_flop1;
        end
    end

endmodule

For above example, when we check the type of my_intf.sp.d_in, it returns \<error>. Technically, we should return \<logic [7:0]> as we can assume default value of IN_WIDTH=8 from interface definition. If the actual DUT instance is provided then we can refer to instantiated interface width, but when there is no instantiation provide we should use default value rather than \<error> for interface signal type.

Can you please update and add this support?

Originally posted by @shivang-chipstack in https://github.com/MikePopoloski/slang/issues/980#issuecomment-2201383393

MikePopoloski commented 2 months ago

You cannot safely assume that the defaults are valid, and doing so could issue otherwise spurious errors or warnings. Some libraries (I think basejump_stl does this) set parameter defaults to huge invalid values so that users get an error if they forget to provide a value when instantiating. That's why slang forces invalid values; type checking and semantic analysis suppress errors / skip analyzing expressions involving invalid values.

shivang-chipstack commented 2 months ago

Thanks for the explanation. Closing this issue as intended behavior.