Skip to content
This repository has been archived by the owner on Mar 29, 2024. It is now read-only.

Tracing improvements #21

Open
yupferris opened this issue Dec 7, 2020 · 2 comments
Open

Tracing improvements #21

yupferris opened this issue Dec 7, 2020 · 2 comments

Comments

@yupferris
Copy link
Owner

yupferris commented Dec 7, 2020

Some sharp edges I've noticed while using it (in addition to some of the notes in #4):

  • I miss inner module inputs/outputs. These are essentially ignored when flattening, but perhaps we should include them when tracing is enabled.
  • It would be nice to make the Trace instance optional, even when tracing is enabled, perhaps by taking an Option<T: Trace> in the ctor. This may have issues with generics, though, since you'd still have to provide a Trace type for the None case.
  • Owning the Trace instance may not be the best thing, since it means the module can no longer implement Clone / Copy trivially (without the underlying Trace impl also implementing these, which for Copy probably isn't feasible and doesn't seem very clean in the first place).
  • Performance seems surprisingly bad. I haven't measured or pinpointed where things are going awry, but it's surprising, so it should be investigated.
  • Due to the current lack of name uniqueness checks in the compiler (which should be fixed!), tracing sometimes ends up with duplicated signals if instance/regs are reused, which is surprisingly easy to do by accident when writing generators it seems. This should lead to invalid verilog codegen, but the rust sim compilation (due to graph flattening) succeeds logically, even though the trace isn't quite right.
@jdonszelmann
Copy link
Contributor

I could do some performance analysis. However, for that I'd need some code written in the language. Of course I can make some myself but it sounds like you already have some code about which you're unhappy with its performance. Would you mind sharing that? In general a larger examples folder (apart from the rust docs which are really good already btw!) would be great. But this may be part of #14. I might be making a start on that by the way.

@yupferris
Copy link
Owner Author

yupferris commented Aug 26, 2021

Re perf, until #23 is solved/changed/landed, there may be significant changes in the Verilog generation code (see this issue comment for a mini-status-update on that). However, if you'd still like to look into this now, I suspect the bottleneck is in VCD crate we use and/or the way we use it, so it's probably still worth verifying if that's the case or not, and if so, possibly looking into fixes/alternatives (I'm not against writing our own, for example, but it's probably best to use something off-the-shelf here).

As for test code, the xenowing project's current master branch should be a good candidate. make generated-rtl (though I think you also need to install a RISC-V toolchain and run make in sw/boot_rom first; I'm moving stuff around there too so it's a bit messy) should generate a single Verilog file with all of the necessary system components (minus fringe top-level stuff to get it onto the FPGA), and it's the largest/most extensive kaze project that I know of (by far), so it should be a good candidate for profiling.

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

No branches or pull requests

2 participants