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

bug: fixes on indexing of relocation #68

Merged
merged 1 commit into from
Dec 16, 2024

Conversation

AntoineFONDEUR
Copy link
Collaborator

Relocation indexing bug

Time spent on this PR: 3h

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

The indexing of the relocation hasn't been tested resulting in many bugs.

What is the new behavior?

Resolved following bugs and made following updates:

  • added compatibility with the previous PR (cell-like registers),
  • builtin order: the builtin segments can't be randomly stacked up on each-other, there is an order that is now enforced (this order must also be respected on the first line of the main.cairo file),
  • added binary files of relocated trace and memory with "recursive" layout to do some testing,
  • EQUIV formula in GS: there were errors of indexing (formula must be of form : "EQUIV(;;0)", the 0 wasn't present).
  • final builtin pointers: after the complete use of a builtin instance, the reference of the first cell of the new following instance is stored in the execution stack. If the builtin is not used after this, the cell referenced is empty. Hence the double "IFERROR" for the address matching.

This final change induces a large formula which could gain in conciseness by doing some pre-computation on the backend level, happy to discuss about it.

@ClementWalter ClementWalter merged commit ead0063 into ClementWalter:main Dec 16, 2024
1 check passed
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.

2 participants