-
Notifications
You must be signed in to change notification settings - Fork 158
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
Simple assembly debugger #693
Conversation
50a98b6
to
4598fac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally, works as advertised. This is pretty awesome :)
I left one comment, but that is just a detail. I'm not going to merge so other people can review it too.
One thing that I haven't look at, do we have any sort of stack available? A way of figuring out where we are in the program.
ff11bd2
to
8253edf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thank you for doing this! I left a couple of comments inline, but I also thing we should update the docs to let people know about this. The docs could be updated in this PR or in a different PR (but then we should create an issue for this).
For doc updates, I'd add a bullet point in the "Running Miden VM" section for the debug
command, but also I wonder if a separate section (similar to the one we have for REPL) is warranted. If we do add a separate section, we should probably break out REPL and Debug sections into their own files as (still sub-sections under "Usage").
8253edf
to
0b58bbe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thank you! I left a couple more comments inline - but to summarize:
- Let's remove
VmState
display logic from theprocessor
crate. This should also fix the WASM compilation errors. - Could you fix this minor issue: Simple assembly debugger #693 (comment)
- Let's create an issue for updating documentation (unless it has already been created).
221f30c
to
ecb348c
Compare
Done.
I've removed all changes to the REPL module such that the debugger implementation is standalone. I've made this issue which states that we should consolidate documentation and logic between the debugger and REPL #699. It would also be cool to allow the REPL to be feature of the debugger. Say you get to a particular clock cycle and you then enter repl mode where you can specify new instructions against the wm.
|
ecb348c
to
0306691
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks good! Thank you! I think we just need to resolve the merge conflict and then we can merge.
5468b4b
to
0306691
Compare
This is a work in progress. This PR introduces a primitive miden assembly debugger. It allows the user to step through code and print out the vm state at each clock cycle. The debugger supports the following commands:
It can be invoked via the CLI using:
./miden debug -a ASSEMBLY_FILE.masm -i INPUT_FILE
Example: