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

Add support for WASM Global Variables #161

Merged
merged 1 commit into from
Dec 18, 2023
Merged

Add support for WASM Global Variables #161

merged 1 commit into from
Dec 18, 2023

Conversation

aborg-dev
Copy link

@aborg-dev aborg-dev commented Dec 11, 2023

This PR implements support for global variable loading. The necessary steps are:

  1. Generate globals and their initial values in the preamble of ZK ASM
  2. In CLIF, generate accesses to globals as memory loads/stores at special "Globals" Symbol + offset which is the index of the global
  3. In ISLE, detect accesses to this special "Globals" Symbol and replace them with a Global AMode
  4. In Inst::Load and Inst::Store handle the Global AMode with appropriate ZKASM MLOAD and MSTORE

@aborg-dev aborg-dev force-pushed the globals_test branch 4 times, most recently from 6891254 to f00d976 Compare December 18, 2023 15:26
@aborg-dev aborg-dev changed the title WIP: Introduce a simple test for global variables Add support for WASM Global Variables Dec 18, 2023
@aborg-dev aborg-dev marked this pull request as ready for review December 18, 2023 15:28
@aborg-dev aborg-dev requested a review from MCJOHN974 December 18, 2023 15:28
@aborg-dev aborg-dev added this to the ZK WASM: Stage 1 milestone Dec 18, 2023
@MCJOHN974
Copy link

MCJOHN974 commented Dec 18, 2023

LGTM! Just nit: I suggest squash last 3 commits into one

This commit implements support for global variable loading. The necessary steps are:

1. Generate globals and their initial values in the preamble of ZK ASM
2. In CLIF, generate accesses to globals as memory loads/stores at special "Globals" Symbol + offset which is the index of the global
3. In ISLE, detect accesses to this special "Globals" Symbol and replace them with a Global AMode
4. In Inst::Load and Inst::Store handle the Global AMode with appropriate ZKASM MLOAD and MSTORE

This scheme will also extend to heap and table accesses
@MCJOHN974
Copy link

Also, what is performance difference between globals and memory in zkasm? maybe we better should reserve some dedicated memory region for globals, will it faster? (I guess no, but sometimes zkasm provide some surprises)

@aborg-dev aborg-dev added this pull request to the merge queue Dec 18, 2023
Merged via the queue into main with commit a5e414f Dec 18, 2023
23 checks passed
@aborg-dev aborg-dev deleted the globals_test branch December 18, 2023 16:31
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