Skip to content
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

commit 477301ae regressed my code with obscure error message #234

Open
tommythorn opened this issue Oct 22, 2022 · 6 comments
Open

commit 477301ae regressed my code with obscure error message #234

tommythorn opened this issue Oct 22, 2022 · 6 comments

Comments

@tommythorn
Copy link
Contributor

I originally abandoned Silice when this happened, but I finally regressed the commit. My sample
yarvi3.ice
compiled prior to this change, but post that commit

$ silice-make.py -s yarvi3.ice -b verilator -p basic -t shell -o /tmp/BUILD_yarvi3

fails with

...
        dualport_bram_template =     dualport_bram_generic.v.in
                templates_path = /home/tommy/projects/silice-examples/Silice/frameworks/templates
                frameworks_dir = /home/tommy/projects/silice-examples/Silice/frameworks
  dualport_bram_wenable0_width =                              1
   dualport_bram_wenable0_type =                           uint
simple_dualport_bram_supported =                            yes
   dualport_bram_wenable1_type =                           uint
assembling source /home/tommy/projects/silice-examples/Silice/frameworks/libraries/memory_ports.si.
assembling source /home/tommy/projects/YARVI3/yarvi3/yarvi3.ice.
main
preprocessing 
[preprocessor] 179]  ')' expected (to close '(' at line 178) near ';'
error: the preprocessor was interrupted

which doesn't seem to match any source lines.

@sylefeb
Copy link
Owner

sylefeb commented Oct 23, 2022

Thanks for the report! The code should have worked, but replacing $display by __display fixes the problem on master (all intrinsic Verilog calls in Silice now are using the '__' prefix, like __display, __write, __signed, etc.).

However the grammar still supports $display precisely for backward compatibility, I am investigating the problem but wanted to share the fix asap.

Also, regarding pipelines, there is a documentation start in the wip branch: https:/sylefeb/Silice/blob/wip/learn-silice/Documentation.md#pipelines ; pipelines are still evolving actively.

@sylefeb
Copy link
Owner

sylefeb commented Oct 23, 2022

Found the problem (due to new pre-processor parser) and added a proper error message indicating to replace $display with __display. This is pushed to master.

@tommythorn
Copy link
Contributor Author

Thanks. How does the three additional assignment operators work? The description is pretty terse.

@sylefeb
Copy link
Owner

sylefeb commented Oct 25, 2022

Yes, the documentation is far from finished ; this is where I stopped last, but I'll resume on it shortly and expand with examples. I also have to fix a couple pending issues regarding these operators, so things may still change a bit).

But the idea is that a ^= assignment is immediately visible (within same cycle) to earlier stages, the v= assignment is immediately visible to later stages, and the vv= assignment will be visible to all stages at the next cycle. When assigned with these operators the variable is not captured by the pipeline. This is primarily useful for dealing with pipeline hazards.

@tommythorn
Copy link
Contributor Author

Hmm, I’ll need to experiment. FWIW, it’s a concern I have with Silice that the semantics aren’t always clear and especially with how features interact.

I’ll test and close, thanks.

@sylefeb
Copy link
Owner

sylefeb commented Oct 27, 2022

Noted - keeping everything consistent is a top priority. Pipelines are taking time to develop due to this, and this is the reason these features are mostly still in wip branch (with their final form being developed in draft) ; and also why they are not fully documented yet.

Please let me know how your tests go (latest wip branch contains fixes). There's one minimal example of the ^= and v= operators here: https:/sylefeb/Silice/blob/wip/tests/pipeline_ops.si

Thanks for the feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants