Skip to content

[Compiler] Refactor vm.Value and related operations to be independent of the vm.Config #3817

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 2 commits into from
Apr 1, 2025

Conversation

SupunS
Copy link
Member

@SupunS SupunS commented Mar 21, 2025

Work towards #3693

Description

Similar to the refactor in the interpreter, avoid the vm.Config from being passed into value related functions (StaticType, Transfer, etc.) at the VM side. Instead, pass only what is needed for each function, as an interface (re-use interpreter's interfaces wherever possible).

In the follow-up PR #3819, similar interfaces are defined at the interpreter (e.g: ValueTransferContext) and decoupled more of interpreter.Values.

The idea is to gradually decouple values of both sides (both vm and interpreter), and make bring the interfaces defined in both sides to an exact match, which will allow us to switch and replace vm-values from the interpreter-values.


  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@SupunS SupunS self-assigned this Mar 21, 2025
Copy link

Cadence Benchstat comparison

This branch with compared with the base branch onflow:feature/compiler commit e6f27ac
The command for i in {1..N}; do go test ./... -run=XXX -bench=. -benchmem -shuffle=on; done was used.
Bench tests were run a total of 7 times on each branch.

Collapsed results for better readability

@SupunS SupunS force-pushed the supun/refactor-vm-values branch from 96e5663 to 8008a2d Compare March 21, 2025 18:38
@SupunS SupunS marked this pull request as ready for review March 21, 2025 20:04
@SupunS SupunS requested a review from turbolent as a code owner March 21, 2025 20:04
@SupunS SupunS marked this pull request as draft March 21, 2025 21:09
@SupunS SupunS changed the title Refactor vm.Value and related operations to be independent of the vm.Config [Compiler] Refactor vm.Value and related operations to be independent of the vm.Config Mar 21, 2025
@SupunS SupunS force-pushed the supun/refactor-vm-values branch from 1d249be to 6e1e8f5 Compare March 21, 2025 22:36
@SupunS SupunS marked this pull request as ready for review March 26, 2025 00:28
Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Great idea and great work! 👏

@SupunS SupunS merged commit 969ed9b into feature/compiler Apr 1, 2025
7 of 9 checks passed
@SupunS SupunS deleted the supun/refactor-vm-values branch April 1, 2025 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants