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

Lower contract creation using the new operator to $zk_create intrinsics #7

Open
wants to merge 1 commit into
base: 0.8.19
Choose a base branch
from

Conversation

abinavpp
Copy link
Collaborator

No description provided.

@hedgar2017 hedgar2017 force-pushed the app-cpr-812-move-the-memory-pointer-to-globals branch from 6883fe7 to 3f2b564 Compare February 19, 2023 15:50
@abinavpp abinavpp force-pushed the app-cpr-812-move-the-memory-pointer-to-globals branch from 3f2b564 to e326165 Compare February 20, 2023 03:16
@abinavpp abinavpp force-pushed the app-cpr-1002-replace-the-create2-instructions-emitted branch from 5ae794d to 90f2dc7 Compare February 20, 2023 03:48
@abinavpp
Copy link
Collaborator Author

This depends on the zkevm extension infrastructure introduced in #1 . Either we need to add #1 to all the other release branches, or we should make a separate PR containing this change and all its dependencies in 1 commit that can be applied to all the other release branches. This is currently based on #2 only due to the newly added test in the ZKEVM directory

@hedgar2017 hedgar2017 force-pushed the app-cpr-812-move-the-memory-pointer-to-globals branch from e326165 to 9dd63d1 Compare February 20, 2023 14:15
@abinavpp abinavpp force-pushed the app-cpr-812-move-the-memory-pointer-to-globals branch 3 times, most recently from f391103 to 9685ec4 Compare February 23, 2023 13:21
@abinavpp abinavpp force-pushed the app-cpr-1002-replace-the-create2-instructions-emitted branch from 90f2dc7 to a342196 Compare March 3, 2023 03:20
@abinavpp abinavpp changed the base branch from app-cpr-812-move-the-memory-pointer-to-globals to 0.8.17 March 3, 2023 03:23
@abinavpp abinavpp force-pushed the app-cpr-1002-replace-the-create2-instructions-emitted branch from a342196 to 05eceba Compare March 6, 2023 14:07
@zksync-admin-bot2
Copy link

Benchmark results:

╔═╡ Size (-%) ╞═════════════════════╡ All ╞═╗
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                                 NaN ║
╠═╡ Cycles (-%) ╞═══════════════════╡ All ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                                 NaN ║
╚═══════════════════════════════════════════╝

@abinavpp
Copy link
Collaborator Author

abinavpp commented Mar 8, 2023

@hedgar2017 When I check one of the CI failure, say: cargo run --release --bin compiler-tester -- -vD --mode='Y-M1B3 0.8.17' --path=tests/solidity/ethereum/salted_create/salted_create.sol. I'm getting the following error:

LLVM IR generator definition pass error: 141:40 Undeclared function `$zk_create2`

The diff in tests_solidity_ethereum_salted_create_salted_create.sol.A.yul only shows some instance of create2 replaced with $zk_create2, I didn't see any other changes

@hedgar2017
Copy link
Collaborator

@hedgar2017 When I check one of the CI failure, say: cargo run --release --bin compiler-tester -- -vD --mode='Y-M1B3 0.8.17' --path=tests/solidity/ethereum/salted_create/salted_create.sol. I'm getting the following error:


LLVM IR generator definition pass error: 141:40 Undeclared function `$zk_create2`

The diff in tests_solidity_ethereum_salted_create_salted_create.sol.A.yul only shows some instance of create2 replaced with $zk_create2, I didn't see any other changes

Ah, well, I haven't added it yet.
Easy fix, will do today.

@abinavpp
Copy link
Collaborator Author

abinavpp commented Mar 8, 2023

@hedgar2017 When I check one of the CI failure, say: cargo run --release --bin compiler-tester -- -vD --mode='Y-M1B3 0.8.17' --path=tests/solidity/ethereum/salted_create/salted_create.sol. I'm getting the following error:

LLVM IR generator definition pass error: 141:40 Undeclared function `$zk_create2`

The diff in tests_solidity_ethereum_salted_create_salted_create.sol.A.yul only shows some instance of create2 replaced with $zk_create2, I didn't see any other changes

Similarly, --mode=E+M2B1 is failing with the error " Failed to get main contract: solc-bin//solc-0.8.17 subprocess output parsing error: data did not match any variant of untagged enum Data at line 1 column 18563"

For some reason, I'm not able to see any $zk_create* operation in any file in the debug directory, but I do see it in the verbose output of compiler-tester, like:

                 {                                                                                                                                                                            
                    "begin": 133,                                                                                                                                                              
                    "end": 155,                                                                                                                                                                
                    "name": "$ZK_CREATE2",                                                                                                                                                     
                    "source": 0                                                                                                                                                                
                  },

@hedgar2017
Copy link
Collaborator

@hedgar2017 When I check one of the CI failure, say: cargo run --release --bin compiler-tester -- -vD --mode='Y-M1B3 0.8.17' --path=tests/solidity/ethereum/salted_create/salted_create.sol. I'm getting the following error:

LLVM IR generator definition pass error: 141:40 Undeclared function $zk_create2

The diff in tests_solidity_ethereum_salted_create_salted_create.sol.A.yul only shows some instance of create2 replaced with $zk_create2, I didn't see any other changes

Similarly, --mode=E+M2B1 is failing with the error " Failed to get main contract: solc-bin//solc-0.8.17 subprocess output parsing error: data did not match any variant of untagged enum Data at line 1 column 18563"

For some reason, I'm not able to see any $zk_create* operation in any file in the debug directory, but I do see it in the verbose output of compiler-tester, like:


                 {                                                                                                                                                                            

                    "begin": 133,                                                                                                                                                              

                    "end": 155,                                                                                                                                                                

                    "name": "$ZK_CREATE2",                                                                                                                                                     

                    "source": 0                                                                                                                                                                

                  },

Yes, the same reason.

@abinavpp abinavpp force-pushed the app-cpr-1002-replace-the-create2-instructions-emitted branch from 05eceba to 76faed8 Compare March 14, 2023 15:45
@abinavpp abinavpp marked this pull request as draft March 14, 2023 15:46
@abinavpp abinavpp force-pushed the app-cpr-1002-replace-the-create2-instructions-emitted branch from 76faed8 to 00ebab8 Compare March 15, 2023 04:57
libevmasm/Instruction.cpp Outdated Show resolved Hide resolved
@abinavpp abinavpp force-pushed the app-cpr-1002-replace-the-create2-instructions-emitted branch 2 times, most recently from e1c3eb8 to ff80408 Compare March 24, 2023 15:32
@abinavpp abinavpp marked this pull request as ready for review March 24, 2023 15:33
@abinavpp
Copy link
Collaborator Author

@hedgar2017, can you check the generated code in yul and evmla and let me know if further changes are required?

@hedgar2017
Copy link
Collaborator

@hedgar2017, can you check the generated code in yul and evmla and let me know if further changes are required?

Yes, will test soon.

@abinavpp abinavpp force-pushed the app-cpr-1002-replace-the-create2-instructions-emitted branch from ff80408 to ef7f067 Compare April 6, 2023 13:23
@abinavpp abinavpp changed the base branch from 0.8.17 to 0.8.19 April 6, 2023 13:24
This commit enables the generation of $zk_create[2],
$zk_data{size|offset|copy} zkevm intrinsics when lowering the new
operator
@hedgar2017 hedgar2017 force-pushed the app-cpr-1002-replace-the-create2-instructions-emitted branch from ef7f067 to 830b5d9 Compare August 27, 2023 15:36
hedgar2017
hedgar2017 previously approved these changes Aug 27, 2023
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.

3 participants