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

Support legacy file descriptors for $fgetc #2190

Open
thoughtpolice opened this issue Mar 13, 2020 · 1 comment
Open

Support legacy file descriptors for $fgetc #2190

thoughtpolice opened this issue Mar 13, 2020 · 1 comment

Comments

@thoughtpolice
Copy link

@thoughtpolice thoughtpolice commented Mar 13, 2020

While using Bluespec to generate my Verilog testbench, I found out that Verilator does not support legacy file descriptors (described by constant values) when calling $fgetc). My use case here is that my testbench reads data from stdin and writes it to stdout so I can diff it.

More concretely, this does not seem to work in the generated Verilog (dut is the harness containing the $fgetc call):

$ verilator ... --cc --exe --trace --top-module top testbench.cc tbtop.v dut.v keccak.v

%Error: dut.v:696: Internal: Unexpected Call
                 : ... In instance top.uut
   b__h12407 = $fgetc(32'h80000000);
               ^~~~~~
%Error: Internal Error: dut.v:696: ../V3AstNodes.h:4717: Unexpected Call
                                 : ... In instance top.uut
   b__h12407 = $fgetc(32'h80000000);
               ^~~~~~

It is probably possible to change the Bluespec compiler to fix this. However, there is some support for these constant legacy descriptors already, I think; for example, #961 describes this same basic problem, only with $fwrite instead.

I would be willing to write a patch for this, but the relevant code seems to have changed quite a bit since #961 was fixed, so I'm not sure where to start...

@thoughtpolice thoughtpolice added the new label Mar 13, 2020
@wsnyder

This comment has been minimized.

Copy link
Member

@wsnyder wsnyder commented Mar 13, 2020

Sure, would be good to have a patch in this area. For details on process see the internals.adoc. The process would be roughly:

  1. Write a test_regress test of the MCD format calls, you can probably clone t_sys_file or something using the new format calls. Make sure this works (and self-checks) on another simulator e.g. Icarus.
  2. Fix parser (verilog.y and whatever else breaks) to make the calls get through into C++.
  3. Fix include/verilated.h VL_CVT_I_FP and whatever else to properly convert a MCD to a FILE*.

The hard part will be to support output to two streams in parallel (the "M" in MCD), perhaps punt on that?

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

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.