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

Implement element-addressable memory #1598

Open
wants to merge 8 commits into
base: next
Choose a base branch
from

Conversation

plafer
Copy link
Contributor

@plafer plafer commented Dec 10, 2024

Closes #1064

Note that I changed the batch column to hold 4 * previous_defn_of_batch (now effectively the first address in the batch). This is to avoid a needless division per row in the processor.

This PR does not address the concern raised #1064 (comment); we should do that one in a subsequent PR, as it is a backend thing, and getting this PR out the door will allow miden-base and the compiler to start working fixing their code.

Left to do:

  • document the hashing behavior of pipe_double_words_to_memory and pipe_words_to_memory
  • docs
    • Make sure to document that procedure locals in MASM are words
  • address all TODO(plafer)s

@plafer plafer force-pushed the plafer-1064-element-addressable-mem branch 2 times, most recently from 6eadbd9 to 74efdb5 Compare December 12, 2024 21:17
@plafer plafer requested a review from bobbinth December 12, 2024 21:17
@plafer
Copy link
Contributor Author

plafer commented Dec 12, 2024

@bobbinth I am done implementing the processor part of the change, which is ready for review. All tests in the processor pass (except 2 which run the verifier, or verify the correctness of the bus).

Left to do after (high-level):

  • fix bus
  • update air constraints
  • update the book
  • a bit of cleanup (the TODO(plafer)'s)

@plafer plafer force-pushed the plafer-1064-element-addressable-mem branch 3 times, most recently from 83c63ef to ec1094f Compare December 18, 2024 22:05
@plafer plafer force-pushed the plafer-1064-element-addressable-mem branch from 31c3e2a to f07da49 Compare December 19, 2024 20:53
@plafer plafer marked this pull request as ready for review December 21, 2024 18:40
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.

1 participant