Different types in generate branches results in error

[mole99]

Using two different types in two different generate branches for the same signal results in the following error:

sv2v: field 'field1' not found in struct packed {
	logic field2;
}, in expression out.field1, within scope test_3A9E3.genblk1 (use -v to get approximate source location)
CallStack (from HasCallStack):
  error, called at src/Convert/Scoper.hs:377:22 in main:Convert.Scoper

Proprietary simulators I have tried seem to ignore the other inactive branch.

This feature is used in the MPU of the cv32e40x.

Here is a minimal example to reproduce the error:

package my_pkg;
    typedef struct packed {
        logic             field1;
    } type1_t;

    typedef struct packed {
        logic            field2;
    } type2_t;
endpackage

module test import my_pkg::*;
#(
    parameter bit  BRANCH  = 1,
    parameter type MY_TYPE = type1_t
)
(
    output MY_TYPE out
);
    generate
        if (BRANCH) begin
            assign out.field1 = '0;
        end else begin
            assign out.field2 = '0;
        end
    endgenerate
endmodule

module top import my_pkg::*; #() ();

    test
    #(  
        .BRANCH  (1),
        .MY_TYPE (type1_t) 
    )
    test_inst1
    (
        .out ()
    );
    
    test
    #(  
        .BRANCH  (0),
        .MY_TYPE (type2_t) 
    )
    test_inst2
    (
        .out ()
    );

endmodule

[zachjs]

In the discussion following #155 (comment), I wondered whether sv2v should need to be able to determine that a particular code path is unreachable to short-circuit code that is invalid or cannot be represented downstream. Based on #226 and openhwgroup/cv32e40x@3438fdb, it doesn't seem like cv32e40x required this behavior until earlier this year.

It might be possible to support this pattern by detecting and replacing the invalid branch with a $fatal or similar. However, I didn't design the struct conversion with this in mind, so adding support would likely take some time. I'm interested in your thoughts on this.

[mole99]

I'm afraid I don't have many thoughts on this topic. For now, I just worked around this issue by duplicating the MPU and enabling one of the paths in each module.

Before, I did not even know it was possible to change types via parameters in SystemVerilog ^^

But it seems that to use type parameters one needs support for this feature, right? Because depending on which type is selected, it has to be handled differently. This could also be handled by the preprocessor (`ifdef/`endif), but is not practical.

The solution you proposed seems to work well for this use case, and I can't think of a scenario where the contents of the invalid branch are needed.

Thanks for taking a look at this! Please know there is no rush to support this right now, since I found a workaround for my use case.

[mole99]

Here's a slightly different issue that seems to be related to this one.

The repository https://github.com/pulp-platform/obi contains various IPs that are compatible with the OBI bus protocol. The OBI bus can be configured with various features enabled or disabled. Depending on the configuration, certain fields of a struct are accessed in some of the modules. If the rready feature is disabled, sv2v still errors out that the field could not be found in the struct.

See obi_mux for example: https://github.com/pulp-platform/obi/blob/00304bc2f6d1a67490c639ffab70af0ea9d77c54/src/obi_mux.sv#L157

This also applies at least to obi_demux and obi_err_sbr.

I think if sv2v would ignore the disabled branch, then this should work too.