-
Notifications
You must be signed in to change notification settings - Fork 136
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
Simulator compatibility #57
base: develop
Are you sure you want to change the base?
Conversation
Most files use the Verilog-2001 port declaration style using implicit type declaration. In cases where the port type must be changed, e.g. an output as reg, it was set in the body of the module (a là Verilog-1995), instead of the header after the direction specification. E.g. in user_id_programming.v: ``` module user_id_programming #( ..- ) ( output [31:0] mask_rev ); wire [31:0] mask_rev; wire [31:0] user_proj_id_high; wire [31:0] user_proj_id_low; .... endmodule ``` This seems to be a problem for some simulators, e.g. Modelsim, which think the `wire [31:0] mask_rev;` line declares a new wire. because the name was already used in the scope this results in an error. Thus such cases where made to use the 2001-style port declaration only.
Remove the compiler directives related to default port type
I can confirm that this works fine with QuestaSim and Xilinx XSim after these changes. If dropping default_nettype none is too controversial, I have already started the work to explicitly declare wires for all the verilog ports if that is preferred |
Great! It is probably the safer solution. |
Ouch, you're right. I didn't consider the generated netlists :/ I guess this should be reported to Yosys as well for the future, or maybe that is already done? |
I believe this is fine. The Yosys-generated netlists use the non-ANSI style for portlists -- untyped ports, with directions and types (eg wire) specified in the module body. The issue (IIRC) with some of the caravel IP was that it either mixed ANSI/non-ANSI ports within the same module, or specified both wire and reg for registered output ports. |
Direction yes, but not the type. Thus ``default_nettype none` will generate errors.
Yes, this is fixed with be2a8a6 |
This pull requests contains fixes related to problems with simulation using Modelsim (and probably with other simulators too).
First commit:
Most design files of caracel use the Verilog-2001 port declaration style using implicit wire type declarations.
In cases where the port type must be changed, e.g. to define an output as reg, the type was declared inside the body of the module (à la Verilog-1995), instead of the header.
E.g. in user_id_programming.v:
This seems to be a problem for some simulators, e.g. Modelsim, which think the
wire [31:0] mask_rev;
line declares a new wire. Since the name is already used in the scope of the module, this results in an error.Thus, such cases where made to use the 2001-style port declaration only.
Second commit:
Solves #54
Third commit:
Suggested Fix for #55
Note: All requests do only change syntax / Simulation related parts of the design.