Various type(FooType) == some_pkg::(foo_32_t) do not work in ModelsimASE

[andrew-cogni]

And similar. This affects:

• oc_cos_test.sv
• oc_csr_tree_splitter.sv
• oc_csr_check_selected.sv

This affects top/tests/DEPS b/c they use these CSR infra modules. There are no unit tests in lib/tests/ that use these.

I currently stubbed out the design modules with an assert if they are used in a define where we know this is ModelsimASE.

Long term these will either need to be refactored (need to figure out exactly what they're trying to do) or we simply don't support ModelsimASE. I believe this SV style is not supported by most ASIC tools (SynopysDC). We could pass an enumerated type indicating the struct type of the various ports (similar to passing a Width or AddrWidth parameter), which while this seems redundant, is probably the path of least resistance. Even with SV Interfaces, you normally aren't allowed to query the $bits(intf.some_fileld), but you are allowed to query a parameter for SomeFieldWidth.

[netchipguy]

Did you see OC_TYPES_EQUAL in oclib_defines.vh?

I think what maybe happened here is that I ran into this issue testing an older Vivado or Questa, and converted the type comparisons enough to get unit tests passing again. IIRC it was Questa because I remember thinking "I'm not getting top level sims going yet, until I have targets that need Questa i.e. Altera ... or ... some Xilinx user wants to get the IPs working in Questa".

Hopefully ASE works OKAY with the macro otherwise I think you'd get more failures.

Now my BELIEF is that type comparison the specific way it's being done here is supposed to work. I understand it will have more issues with some struct-like hierarchy involved, and that it cannot be used to assign another type, it can only be used to select a block of code. I think it's really good for that, it means I can just decide on an AXI type I'm going to use ONCE, and not have to set another matching param, remember to always set both, etc. Unfortunately I think we always have to pass the struct type (and it's return type) because types can't depend on a param, else I would've done an enum param to select both of them.

Comparing types by number of bits is not great for two reasons though, obviously they eventually risk type collisions, but also it discourages use of types for things we really should be using them for (like typing a backpressure as ready vs afull vs credit ... except they are all single bit signals)

It's really important to figure out if this is the direction the language is going. I am OKAY leaving behind tools that aren't keeping up (iverilog) but I def don't wanna be choosing coding styles that just happen to work in the first tool or two. When we are truly enterprise and we tell someone they should use this library (or let AI use it !!) then we need to be ready for all kinds of tools (formal, lint, internal stuff, etc).

The long term solution is a flow that elaborates an SV design. I did such a thing back in Verilog 2008 days, it preprocessed and then pushed all parameters down into the code, which ended up with no defines or params. SV makes it harder but I suspect Verilator could be coaxed into doing it, assuming Verilator understands the code (which it should). Verific I'm sure already has this or could build it pretty easily. This elaboration flow could also handle things like renaming (i.e. take a block of generated code and stick a "cognichip_" in front of all global names, because not only does customer have their own code, but also potentially a fork of opencos with their own oclib_fifo and such)

On Sat, Sep 7, 2024 at 8:19 AM andrew-cogni @.***> wrote:

And similar. This affects:

• oc_cos_test.sv
• oc_csr_tree_splitter.sv
• oc_csr_check_selected.sv

This affects top/tests/DEPS b/c they use these CSR infra modules. There are no unit tests in lib/tests/ that use these.

I currently stubbed out the design modules with an assert if they are used in a define where we know this is ModelsimASE.

Long term these will either need to be refactored (need to figure out exactly what they're trying to do) or we simply don't support ModelsimASE. I believe this SV style is not supported by most ASIC tools (SynopysDC). We could pass an enumerated type indicating the struct type of the various ports (similar to passing a Width or AddrWidth parameter), which while this seems redundant, is probably the path of least resistance. Even with SV Interfaces, you normally aren't allowed to query the $bits(intf.some_fileld), but you are allowed to query a parameter for SomeFieldWidth.

— Reply to this email directly, view it on GitHub https://github.com/netchipguy/opencos/issues/8, or unsubscribe https://github.com/notifications/unsubscribe-auth/AH3STYMOGCD4MBGQUQWB3N3ZVMKPFAVCNFSM6AAAAABN2DLSBGVHI2DSMVQWIX3LMV43ASLTON2WKOZSGUYTCNZVGY2TIOI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[andrew-cogni]

To document this, I tried many things including using $bits(some_type_t) or $bits(some_signal) to determine the type and how to handle the generate if-else conditions.

Typical error message is:

# -- Compiling module oclib_csr_check_selected
# ** Fatal: Unexpected signal: 11.
###### /home/blahblahblah/opencos/lib/oclib_csr_check_selected.sv(77): endmodule : oclib_csr_check_selected

I do not have this problem with the various axi4st_WIDTH_s types, but on those I also pass the WIDTH as a parameter passed through the modules because I'm wary of the compatibility, and that effectively can be used as an enum to tell me what the struct Type is. But with those structs, all of the same fields exist, but some have different widths. With the CSR structs, some fields exist or not, which is why this is problematic.

What I mean by this needs to be refactored - make the CSR struct types all have the same member fields. If you have a Tree or NOC or something fancy, then those need to be side-band signals, or every struct needs to have them.