-
Notifications
You must be signed in to change notification settings - Fork 19
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
systemverilog-plugin: Blackparrot support (was: ERROR: Failed to detect width for parameter) #1064
Comments
@rkapuscik it is an error coming from the plugin. |
Hi, is there any additional information I could give to help debug this? |
@rkapuscik was out sick, should be back at it soon |
@dpetrisko file lists on the surelog script and the yosys script differ slightly. Files missing in the yosys script:
and there's extra Anyway, even with updated file list, I've reproduced your problem.
|
Hi yes, I also see these FILE INDEX MISMATCH lines |
(In both yosys and surelog) |
The "FILE INDEX MISMATCH" is not the problem at all. please make sure your build of Surelog is up to date. |
That is what I see in Surelog:
|
Using the proces_design.sh surelog (Which has a very different file list from the original one from Dan) I get:
|
I'm not sure why we have so many top level modules, that does not sound right. Those file lists are not representative of a "design" more like a library. |
The extra top levels are included from BaseJump STL library files as "abstract modules" (*__abstract). Basically each module is instantiated in an if (0) block in its source file, in order to avoid elaborating with default parameters which may be invalid. You can see the discussion here: https://github.com/bespoke-silicon-group/basejump_stl/blob/8ccd91a5cfb252e096a70640d3dd1bb9e0e1fa46/bsg_misc/bsg_defines.v#L12. There's only 1 BlackParrot-specific toplevel |
Ignoring the FILE_INDEX_MISMATCH, I don't see any other errors from Surelog/Yosys in the logs except for the ERROR: Failed to detect width for parameter \proc_param_lp! |
@dpetrisko that is correct. @mglb please help fix the issue in the plugin code. |
I'm working on it. |
The bogus error message FILE_INDEX_MISMATCH |
Turns out the problem is (most probably) caused by another issue. The design uses union as a struct member, which is not supported by the plugin (EDIT: in expressions with a dot). EDIT: relevant issue: chipsalliance/yosys-f4pga-plugins#385 |
Thanks for the update! We fairly heavily rely on struct of unions as a construct, so if that's on the roadmap we'll wait rather than re-writing the RTL |
PR adding support for unions in structs: chipsalliance/yosys-f4pga-plugins#389 I've also found the main (and hopefully final) problem. Turns out the reported symbol is most probably OK. It is just evaluated too early, and this early evaluation is caused by potentially unrelated code. Bug details:Consider the following example: // --- test.sv: ---
module test(input clk_i, input reset_i);
typedef enum bit[1:0] { test_e_0 = 0, test_e_1 = 1 } test_e;
parameter int test_p_int_vec [1:0] = '{ 4, 8 };
// localparam test_e test_lp_index = 1; // (A1) OK
localparam test_e test_lp_index = test_e_1; // (A2) "ERROR: Failed to detect width for parameter \test_lp_val!" when used with B2.
localparam test_lp_val = test_p_int_vec[test_lp_index];
typedef struct packed {
logic [test_lp_val-1:0] test_s_member_0;
logic [test_lp_val:0] test_s_member_1;
} test_typedef_s;
test_helper test_helper_instance();
endmodule // --- test_helper.sv ---
module test_helper;
typedef struct packed {
integer unsigned helper_s_member;
} helper_typedef_s;
// localparam integer unsigned helper_lp = 0; // (B1) ALWAYS OK
localparam helper_typedef_s helper_lp = '{ helper_s_member: 0 }; // (B2) causes error when used with A2
endmodule Differences between assignment of integer iteral (A1) vs enum value (A2) to
|
FYI: support for unions in struct is merged: chipsalliance/yosys-f4pga-plugins#389 |
The reported problem is technically fixed! chipsalliance/yosys-f4pga-plugins#399 |
Awesome! Do we want to close this and make a new tracking issue? Or just treat this page as the "BlackParrot issue?" :) |
Lets keep this as a "summary" issue. I've changed the title to reflect this. |
Hi, update from my end: The parsing completes! But then segfaults during elaboration
|
@dpetrisko I pulled the latest from black-parrot master and the file lists attached to this case don't work anymore, can you update the file list? [FTL:CM0008] Verilog File "/home/alain/black-parrot/external/basejump_stl/bsg_cache/bsg_cache_sbuf_queue.v" does not exist. |
Hi Alain, Attached: flist.txt Here are the tags for what I'm using (master): |
@dpetrisko I'm getting a syntax error. /home/alain/black-parrot/external/basejump_stl/bsg_misc/bsg_circular_ptr.v:60:5: Syntax error: extraneous input 'else' expecting {';', 'default', 'module', 'endmodule', 'extern', 'macromodule', 'interface', 'program', 'virtual', 'class', 'timeunit', 'timeprecision', 'checker', 'type', 'clocking', 'defparam', 'bind', 'const', 'function', 'static', 'constraint', 'if', 'automatic', 'localparam', 'parameter', 'specparam', 'import', 'genvar', 'typedef', 'enum', 'struct', 'union', 'string', 'chandle', 'event', '[', 'byte', 'shortint', 'int', 'longint', 'integer', 'time', 'bit', 'logic', 'reg', 'shortreal', 'real', 'realtime', 'supply0', 'supply1', 'tri', 'triand', 'trior', 'tri0', 'tri1', 'wire', 'uwire', 'wand', 'wor', 'trireg', 'signed', 'unsigned', 'interconnect', 'var', '$', 'export', DOLLAR_UNIT, '(*', 'assert', 'property', 'assume', 'cover', 'not', 'or', 'and', 'sequence', 'covergroup', 'pulldown', 'pullup', 'cmos', 'rcmos', 'bufif0', 'bufif1', 'notif0', 'notif1', 'nmos', 'pmos', 'rnmos', 'rpmos', 'nand', 'nor', 'xor', 'xnor', 'buf', 'tranif0', 'tranif1', 'rtranif1', 'rtranif0', 'tran', 'rtran', 'generate', 'case', 'for', 'global', 'initial', 'assign', 'alias', 'always', 'always_comb', 'always_latch', 'always_ff', 'restrict', 'let', 'this', 'randomize', 'final', 'task', 'specify', 'sample', '=', 'nettype', Escaped_identifier, Simple_identifier, '`pragma', SURELOG_MACRO_NOT_DEFINED}, |
Fixed, -sverilog on the command line fixed it (Treat .v as systemverilog files). |
Elaboration works fine in Surelog: .... [NTE:EL0508] Nb Top level modules: 146. [NTE:EL0509] Max instance depth: 16. [NTE:EL0510] Nb instances: 2003. [NTE:EL0511] Nb leaf instances: 494. [INF:UH0706] Creating UHDM Model... [INF:UH0708] Writing UHDM DB: /home/alain/slpp_all/surelog.uhdm ... [ FATAL] : 0 |
@dpetrisko can you send me the updated Yosys script you are using? |
Hi Alain, I'm using the script that @mglb provided above:
to run |
@dpetrisko how did you merge chipsalliance/yosys-f4pga-plugins#377 |
@dpetrisko I can confirm the segfault on my end. I did look at it briefly; it seems to be caused by invalid handling of parameters without default value which are used in port's range. This is the plugin issue. |
Issue for the segfault: chipsalliance/yosys-f4pga-plugins#424 |
Thank you for the update! Let me know if there's anything I can help with the provided tests! |
Turns out chipsalliance/yosys-f4pga-plugins#424 is not an issue here. The segfault comes from lack of support for streaming operators in the plugin: #968 |
@dpetrisko with the latest source of black-parrot, I get the following error with the compilation script process_design.txt: [ FATAL] : 1 Can you check in that script in black-parrot and maintain it in sync with the source code? |
Hi Alain, if you are pulling from master of BlackParrot, you'll need to update the filelist which changes as the code changes: https://github.com/black-parrot/black-parrot/blob/master/bp_top/syn/flist.vcs For that specific file, I can tell you that the changeset is |
Hi Dan, I'd rather use the official flist.vcs then. How do I get the values of $BASEJUMP_STL_DIR and other variables? |
Sounds good, easier to keep in sync that way. We set those variables here: https://github.com/black-parrot/black-parrot/blob/master/Makefile.common#L1
|
Actually, this flow with flist.vcs is for simulation. |
This is the synth flist. There are some extra files which are used for other parameterizations of BP, but everything in there should be synth |
ok, thanks |
Since I want to debug the Surelog step stand alone first, I create the following script: source black-parrot.sl
There are a couple of problems with -f in Surelog with the comments, so I removed them manually. That used to work but somehow got broken. I'll fix that. I'll start by debugging the following errors:
I'm also looking at the html report for UHDM code coverage to inspect what parts of the design we are not covering. I'll need your help to review some too later on. |
Sure thing, let me know. At a glance, those look like they are caused by default parameters of modules which are not in the elab tree for the default build |
Hi, trying to track down an issue, I believe it's in the plugin but it could also be in surelog.
I am using the default submodules except for https://github.com/chipsalliance/yosys-f4pga-plugins/pulls/377 to fix chipsalliance/yosys-f4pga-plugins#380 and chipsalliance/Surelog#3208 to fix chipsalliance/Surelog#3203.
To get the code:
Here is a script which can successfully parse BlackParrot:
surelog.txt
bash surelog.txt
Here is a script which fails to link BlackParrot.
yosys.txt
yosys -s blackparrot.txt
Here is the tail of the output:
We can see 0 FATAL, SYNTAX or ERROR during parsing, but an ERROR during the linking step. bp_common_aviary_defines.svh is basically a macro which assigns localparams from a large localparam array. Oddly, the line in question is about halfway through the file and there doesn't appear to be anything special about it. I would expect the strategy to either completely succeed or completely fail. What could be going on here? Thanks!
The text was updated successfully, but these errors were encountered: