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.

Sign up for GitHub

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

Jump to bottom

Add Dyn block to MAST #1055

Merged
merged 4 commits into from
Sep 6, 2023
Merged

Add Dyn block to MAST #1055

merged 4 commits into from
Sep 6, 2023

Conversation

grjte
Copy link
Contributor

@grjte grjte commented Aug 25, 2023 •
edited
Loading

This PR adds a new DYN block to the MAST and the decoder. It includes the following

  • dyn node test
  • dyn node implemented
  • update the program docs with the new MAST node (excluding the decoder docs)

next steps:

  • define the decoder constraints + update the decoder constraint docs (future PR)
  • add an assembly instruction for dynamic code

@grjte grjte changed the base branch from main to next August 25, 2023 14:48
bobbinth
bobbinth reviewed Aug 28, 2023
View reviewed changes
core/src/operations/mod.rs Outdated Show resolved Hide resolved
core/src/program/blocks/dyn_block.rs Outdated Show resolved Hide resolved
docs/src/design/programs.md Outdated Show resolved Hide resolved
@grjte grjte force-pushed the grjte-dyn-block branch 2 times, most recently from 5d1c86c to 61bf464 Compare September 4, 2023 14:23
bobbinth
bobbinth reviewed Sep 4, 2023
View reviewed changes
processor/src/decoder/block_stack.rs Outdated Show resolved Hide resolved
core/src/program/blocks/dyn_block.rs Outdated Show resolved Hide resolved
core/src/program/blocks/dyn_block.rs Outdated Show resolved Hide resolved
processor/src/lib.rs Outdated Show resolved Hide resolved
processor/src/decoder/mod.rs Outdated Show resolved Hide resolved
@grjte grjte marked this pull request as ready for review September 5, 2023 15:19
@grjte grjte changed the title WIP: dyn block Add Dyn block to MAST Sep 5, 2023
@grjte
Copy link
Contributor Author

grjte commented Sep 5, 2023

@bobbinth since we seem to be in agreement about this direction, I was going to leave the decoder constraints to a separate PR, but let me know if you'd prefer to have them in this one

@grjte grjte requested a review from bobbinth September 5, 2023 15:20
bobbinth
bobbinth approved these changes Sep 5, 2023
View reviewed changes
Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thank you! I left a couple of small comments inline. After these are addressed, we can merge.

docs/src/design/programs.md Show resolved Hide resolved
core/src/program/blocks/dyn_block.rs Show resolved Hide resolved
processor/src/lib.rs Outdated Show resolved Hide resolved
@grjte grjte merged commit 1d380e1 into next Sep 6, 2023
@grjte grjte deleted the grjte-dyn-block branch September 6, 2023 19:17
@grjte grjte mentioned this pull request Sep 6, 2023
Merged
@bobbinth bobbinth mentioned this pull request Sep 22, 2023
Closed
@bobbinth bobbinth mentioned this pull request Oct 4, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bobbinth bobbinth bobbinth approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@grjte @bobbinth