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

[SOL] Change encoding of callx instruction #78

Merged
merged 3 commits into from
Jan 4, 2024

Conversation

LucasSte
Copy link
Collaborator

@LucasSte LucasSte commented Jan 2, 2024

This PR is one of the tasks in solana-labs/solana#34250 to update the SBF target with the new instructions. Especially, this modification is the equivalent of solana-labs/rbpf#491 in LLVM, changing the encoding of the callx instruction.

There was a small refactoring in the table gen files to encompass a new SubtargetFeature. As FeatureCallxRegSrc was required both in SBF.td and in SBFInstrInfo.td, I put all the feature in a new file to avoid a circular dependency.

@LucasSte LucasSte requested a review from dmakarov January 3, 2024 12:53
@LucasSte LucasSte marked this pull request as ready for review January 3, 2024 12:53
@LucasSte LucasSte marked this pull request as draft January 3, 2024 14:42
Copy link
Collaborator

@dmakarov dmakarov left a comment

Choose a reason for hiding this comment

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

LGTM, but it's still a Draft. Are planning more changes in this PR?

llvm/test/MC/SBF/sbf-jmp.s Outdated Show resolved Hide resolved
@LucasSte
Copy link
Collaborator Author

LucasSte commented Jan 3, 2024

I tested this change against the programs in programs/sbf on the Solana's main repository. Besides the instruction count that has slightly changed (we now have two instructions for lddw: mov32 and hor64), all tests have passed.

I'm waiting to merge #77 so that I can rebase this PR and fix the conflicts before the review.

@LucasSte
Copy link
Collaborator Author

LucasSte commented Jan 3, 2024

LGTM, but it's still a Draft. Are planning more changes in this PR?

I converted to a draft because I wanted to test it with real programs first. I also noticed I got the encoding wrong 😅

@dmakarov
Copy link
Collaborator

dmakarov commented Jan 4, 2024

LGTM, please feel free to merge when the CI is done.

@LucasSte LucasSte marked this pull request as ready for review January 4, 2024 14:53
@LucasSte
Copy link
Collaborator Author

LucasSte commented Jan 4, 2024

LGTM, please feel free to merge when the CI is done.

Merging is blocked without an approval.

Copy link
Collaborator

@dmakarov dmakarov left a comment

Choose a reason for hiding this comment

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

Sorry, forget to approve.

@LucasSte LucasSte merged commit 5502543 into anza-xyz:solana-rustc/16.0-2023-06-05 Jan 4, 2024
14 checks passed
@LucasSte LucasSte deleted the callx branch January 4, 2024 17:01
LucasSte added a commit to LucasSte/llvm-project that referenced this pull request Jan 31, 2024
* Change encoding of callx instruction
LucasSte added a commit that referenced this pull request Feb 16, 2024
* Change encoding of callx instruction
LucasSte added a commit to LucasSte/llvm-project that referenced this pull request Jun 28, 2024
* Change encoding of callx instruction
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