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

Span block decoding update #212

Merged
merged 1 commit into from
May 23, 2022
Merged

Span block decoding update #212

merged 1 commit into from
May 23, 2022

Conversation

bobbinth
Copy link
Contributor

This PR breaks out several changes from #195 to make the PRs more manageable.

Major changes include:

  • Refactored how operation batches are processed within a SPAN block. Specifically, the assembler no longer inserts NOOPs to ensure proper alignment - this is now done at the processor level. This means that duplicate NOOP insertion mentioned in Optimize the logic for inlining procedures #109 should no longer be an issue.
  • Changed operation alignment rules. Now, a PUSH operation cannot be the last operation in an operation group (previously, it couldn't be the first operation).
  • Added HALT processor instruction, though it doesn't do much yet.

Copy link
Contributor

@grjte grjte 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! I like the can_accept_op changes - I think it's quite clear now. I left one issue inline as well as a couple of nits & a question :)

core/src/program/blocks/span_block.rs Outdated Show resolved Hide resolved
processor/src/lib.rs Outdated Show resolved Hide resolved
processor/src/lib.rs Outdated Show resolved Hide resolved
core/src/operations/mod.rs Show resolved Hide resolved
Copy link
Contributor

@grjte grjte left a comment

Choose a reason for hiding this comment

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

It looks good to me now!

Regarding #109 - should we update this issue to verify that procedure inlining is now happening optimallyl? I guess that would require checking that the old issue is resolved by this change (as it seems it should be) and that nothing new & unexpected pops up

@grjte grjte merged commit 98bb53c into next May 23, 2022
@grjte grjte deleted the span-decoding branch May 23, 2022 12:10
@grjte grjte mentioned this pull request May 23, 2022
39 tasks
@tohrnii tohrnii mentioned this pull request May 23, 2022
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants