surfer: Initial inclusion #3626
Conversation
|
@infinitymdm thanks. You'll need to add the tool to the |
|
I've got surfer working with the heartbeat_sim.py example. Looking for more information on how to more thoroughly test this. |
| # Don't exit on show | ||
| chip.set('tool', tool, 'task', task, 'var', 'show_exit', False, | ||
| step=step, index=index, clobber=False) |
There was a problem hiding this comment.
I don't see anything using this.
We use this flag in the other tools to indicate the tool should exit after opening, but it requires the tool driver doing something.
| @@ -34,6 +35,7 @@ def setup(chip): | |||
| chip.register_showtool('vg', yosys_screenshot) | |||
|
|
|||
| chip.register_showtool('vcd', gtkwave_show) | |||
| chip.register_showtool('vcd', surfer_show) | |||
There was a problem hiding this comment.
this will require a little though, since this is really just replacing the gtkwave as the viewer (which might be alright if this is a better viewer, but want to avoid making that change suddenly)
There was a problem hiding this comment.
I see what you're saying; this change may not be needed. Is there another mechanism to allow users to select which vcd showtool they'd like to use?
There was a problem hiding this comment.
we don't (atleast not anything particularly nice). Maybe the thing to so is to check if surfer is installed and use that otherwise default back to gtkwave?
if shutil.which("surfer") is not None:
chip.register_showtool('vcd', surfer_show)
else:
chip.register_showtool('vcd', gtkwave_show)
|
@infinitymdm I'll trigger the tool build since that requires write permissions: https://github.com/siliconcompiler/siliconcompiler/actions/runs/15326634211 Passes: https://github.com/siliconcompiler/siliconcompiler/actions/runs/15327813860 You also need to mark the scripts as executable, ie: |
|
I notice the tests are failing, but I'm not sure what's going wrong. Can you make anything of this? https://github.com/siliconcompiler/siliconcompiler/actions/runs/15326634211/job/43126098152#step:6:95 It looks to me like surfer successfully loads the vcd file, then the test monitor dies. Is this perhaps related to the |
Yes, I think you need to add a small script to just exit. |
|
In its current implementation, surfer only supports passing startup scripts. This means that the The next release should address this, as the relevant issue was recently closed: https://gitlab.com/surfer-project/surfer/-/issues/413 |
|
@infinitymdm I see, thanks for clarifying, in that case I think it would be best to have the tests for the CI in there, but disable them for now: |
I can't think of anything. In practice, surfer with siliconcompiler is a pretty great experience. I was thinking you would want to wait until the automated testing would work; that's why I converted this PR back to draft. I'll rebase on main, disable the test, add the check for whether surfer is installed, and then mark this ready for review. |
|
@infinitymdm that sounds good to me, I am okay waiting on the testing since it requires a new version of surfer and that could leave this hanging. When ready just mark it ready and I'll review. |
Summary
This PR aims to add support for surfer, an up-and-coming waveform viewer similar to gtkwave.
Details
Surfer is a pretty fancy upgrade over gtkwave. It supports a lot of nice features, such as automatic recognition of RISC-V instructions. It's also significantly snappier and IMO easier to use, though its current release lacks customizable keybinds. It's still in pretty early stages, but lots of open source projects have started adding support for it.
Perhaps its most interesting feature is the web-based viewer. This PR does not target that interface, instead making use of the native-installed viewer binary for simplicity's sake, but it's probably worth exploring the web viewer in the future.
My primary motivations for adding this tool are
Most of the work I've done so far is heavily based on the gtkwave tool interface, plus a smattering of observations from other tool interface code. Not sure if I'm doing this right; we'll see if I can get it working.
To do before Ready for Review:
Figure out what sc_show.cmd script is supposed to do (assuming add waves)Not needed, as far as I can tell; removed from latest commits