Skip to content

SIMD-0166: SBPF Dynamic stack frames #166

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

Merged
merged 9 commits into from
Jun 30, 2025

Conversation

LucasSte
Copy link
Contributor

@LucasSte LucasSte commented Aug 19, 2024

Dynamic stack frames are available by building a Solana program using cargo-build-sbf --arch v1.

@LucasSte LucasSte changed the title SIMD-XXX: Dynamic stack frames SIMD-0166: Dynamic stack frames Aug 19, 2024
@LucasSte LucasSte changed the title SIMD-0166: Dynamic stack frames SIMD-0166: SBPF Dynamic stack frames Oct 2, 2024
Copy link
Contributor

@0x0ece 0x0ece left a comment

Choose a reason for hiding this comment

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

This looks a huge improvement. My only recommendation is alignment, I tried to explain why I'd push it up to 64 bytes vs just 16.

call convention obviates the need to use R5 for retrieving the caller’s frame
pointer address to access those parameters.

### Changes in the verifier
Copy link
Contributor

Choose a reason for hiding this comment

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

I would simplify this section. ATM it seems to describe the whole process of validating registers, not just the changes due to this SIMD.

The verifier should include a new rule to accept R11 as a destination register under the following conditions:

  1. Dynamic stack frames are enabled
  2. The corresponding instruction is add64_imm (opcode 0x07)
  3. The corresponding value of imm is a multiple of the alignment (i.e. multiple of 64).

Copy link
Contributor

Choose a reason for hiding this comment

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

As a comment to my comment :) Based on the current code, I'd simply add an extra "and" to the "if" condition that returns Ok() for R11. Therefore I don't think we need an error for alignment. Either R11 is allowed (if all 3 conditions pass), or it's not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I re-wrote the section, based on the new design we devised with only R10.

Copy link
Collaborator

@Benhawkins18 Benhawkins18 left a comment

Choose a reason for hiding this comment

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

I see approvals from @Lichtso and @0x0ece, merging

Copy link
Contributor Author

@LucasSte LucasSte left a comment

Choose a reason for hiding this comment

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

Ready to be merged!

@Benhawkins18 Benhawkins18 merged commit 0a210df into solana-foundation:main Jun 30, 2025
2 checks passed
@LucasSte LucasSte deleted the dyn-frames branch June 30, 2025 19:55
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.

6 participants