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.

Sign up for GitHub

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

Jump to bottom

Enable the machine outliner for SyncVM #243

Merged
merged 3 commits into from
Jun 22, 2023
Merged

Enable the machine outliner for SyncVM #243

merged 3 commits into from
Jun 22, 2023

Conversation

vladimirradosavljevic
Copy link
Contributor

No description provided.

@vladimirradosavljevic vladimirradosavljevic force-pushed the machine_outliner branch 2 times, most recently from 5d2dcf0 to 9de2f54 Compare June 13, 2023 10:46
@zksync-admin-bot2
Copy link
Collaborator

Benchmark results:

╔═╡ Size (-%) ╞═════════════════════╡ All ╞═╗
║ Mean                                0.898 ║
║ Best                               45.104 ║
║ Worst                               0.000 ║
║ Total                               3.713 ║
╠═╡ Cycles (-%) ╞═══════════════════╡ All ╞═╣
║ Mean                                 -inf ║
║ Best                                0.000 ║
║ Worst                             -49.504 ║
║ Total                              -2.925 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞═══════════════╡ Real life ╞═╗
║ Mean                                8.304 ║
║ Best                               31.070 ║
║ Worst                               0.000 ║
║ Total                              14.194 ║
╠═╡ Cycles (-%) ╞═════════════╡ Real life ╞═╣
║ Mean                               -7.809 ║
║ Best                                0.000 ║
║ Worst                             -19.046 ║
║ Total                              -7.132 ║
╚═══════════════════════════════════════════╝

@zksync-admin-bot2
Copy link
Collaborator

Benchmark results:

╔═╡ Size (-%) ╞═════════════════════╡ All ╞═╗
║ Mean                                0.569 ║
║ Best                               37.020 ║
║ Worst                               0.000 ║
║ Total                               2.049 ║
╠═╡ Cycles (-%) ╞═══════════════════╡ All ╞═╣
║ Mean                                 -inf ║
║ Best                                0.000 ║
║ Worst                             -28.777 ║
║ Total                              -2.108 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞═══════════════╡ Real life ╞═╗
║ Mean                                4.694 ║
║ Best                               31.070 ║
║ Worst                               0.000 ║
║ Total                               9.181 ║
╠═╡ Cycles (-%) ╞═════════════╡ Real life ╞═╣
║ Mean                               -6.407 ║
║ Best                                0.000 ║
║ Worst                             -17.312 ║
║ Total                              -6.160 ║
╚═══════════════════════════════════════════╝

@zksync-admin-bot2
Copy link
Collaborator

Benchmark results:

╔═╡ Size (-%) ╞═════════════════════╡ All ╞═╗
║ Mean                                0.682 ║
║ Best                               37.020 ║
║ Worst                               0.000 ║
║ Total                               2.369 ║
╠═╡ Cycles (-%) ╞═══════════════════╡ All ╞═╣
║ Mean                                 -inf ║
║ Best                                0.000 ║
║ Worst                             -34.286 ║
║ Total                              -2.110 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞═══════════════╡ Real life ╞═╗
║ Mean                                5.588 ║
║ Best                               31.070 ║
║ Worst                               0.000 ║
║ Total                              10.406 ║
╠═╡ Cycles (-%) ╞═════════════╡ Real life ╞═╣
║ Mean                               -6.781 ║
║ Best                                0.000 ║
║ Worst                             -17.573 ║
║ Total                              -6.305 ║
╚═══════════════════════════════════════════╝

@vladimirradosavljevic vladimirradosavljevic force-pushed the machine_outliner branch 3 times, most recently from 89f7b75 to 6e30352 Compare June 16, 2023 07:06
@zksync-admin-bot2
Copy link
Collaborator

Benchmark results:

╔═╡ Size (-%) ╞═════════════════════╡ All ╞═╗
║ Mean                                0.682 ║
║ Best                               37.020 ║
║ Worst                               0.000 ║
║ Total                               2.369 ║
╠═╡ Cycles (-%) ╞═══════════════════╡ All ╞═╣
║ Mean                                 -inf ║
║ Best                                0.000 ║
║ Worst                             -34.286 ║
║ Total                              -2.110 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞═══════════════╡ Real life ╞═╗
║ Mean                                5.588 ║
║ Best                               31.070 ║
║ Worst                               0.000 ║
║ Total                              10.406 ║
╠═╡ Cycles (-%) ╞═════════════╡ Real life ╞═╣
║ Mean                               -6.781 ║
║ Best                                0.000 ║
║ Worst                             -17.573 ║
║ Total                              -6.305 ║
╚═══════════════════════════════════════════╝

@vladimirradosavljevic vladimirradosavljevic changed the title [WIP] Enable the machine outliner for SyncVM Enable the machine outliner for SyncVM Jun 16, 2023
@vladimirradosavljevic vladimirradosavljevic marked this pull request as ready for review June 16, 2023 15:26
@zksync-admin-bot2
Copy link
Collaborator

Benchmark results:

╔═╡ Size (-%) ╞═════════════════════╡ All ╞═╗
║ Mean                                0.682 ║
║ Best                               37.020 ║
║ Worst                               0.000 ║
║ Total                               2.369 ║
╠═╡ Cycles (-%) ╞═══════════════════╡ All ╞═╣
║ Mean                                 -inf ║
║ Best                                0.000 ║
║ Worst                             -34.286 ║
║ Total                              -2.110 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞═══════════════╡ Real life ╞═╗
║ Mean                                5.588 ║
║ Best                               31.070 ║
║ Worst                               0.000 ║
║ Total                              10.406 ║
╠═╡ Cycles (-%) ╞═════════════╡ Real life ╞═╣
║ Mean                               -6.781 ║
║ Best                                0.000 ║
║ Worst                             -17.573 ║
║ Total                              -6.305 ║
╚═══════════════════════════════════════════╝

lialan
lialan suggested changes Jun 16, 2023
View reviewed changes
Copy link
Contributor

@lialan lialan left a comment

Choose a reason for hiding this comment

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

Good quality of code, just some places we need to revisit.

llvm/lib/Target/SyncVM/SyncVMInstrInfo.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/SyncVM/SyncVMInstrInfo.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/SyncVM/SyncVMInstrInfo.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/SyncVM/SyncVMInstrInfo.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/SyncVM/SyncVMInstrInfo.cpp Show resolved Hide resolved
@vladimirradosavljevic vladimirradosavljevic force-pushed the machine_outliner branch 2 times, most recently from 51f804f to 7247c4d Compare June 19, 2023 10:47
@zksync-admin-bot2
Copy link
Collaborator

Benchmark results:

╔═╡ Size (-%) ╞═════════════════════╡ All ╞═╗
║ Mean                                0.684 ║
║ Best                               37.020 ║
║ Worst                               0.000 ║
║ Total                               2.371 ║
╠═╡ Cycles (-%) ╞═══════════════════╡ All ╞═╣
║ Mean                                 -inf ║
║ Best                                0.000 ║
║ Worst                             -34.286 ║
║ Total                              -2.112 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞═══════════════╡ Real life ╞═╗
║ Mean                                5.588 ║
║ Best                               31.070 ║
║ Worst                               0.000 ║
║ Total                              10.406 ║
╠═╡ Cycles (-%) ╞═════════════╡ Real life ╞═╣
║ Mean                               -6.781 ║
║ Best                                0.000 ║
║ Worst                             -17.573 ║
║ Total                              -6.305 ║
╚═══════════════════════════════════════════╝

@zksync-admin-bot2
Copy link
Collaborator

Benchmark results:

╔═╡ Size (-%) ╞═════════════════════╡ All ╞═╗
║ Mean                                0.684 ║
║ Best                               37.020 ║
║ Worst                               0.000 ║
║ Total                               2.371 ║
╠═╡ Cycles (-%) ╞═══════════════════╡ All ╞═╣
║ Mean                                 -inf ║
║ Best                                0.000 ║
║ Worst                             -34.286 ║
║ Total                              -2.112 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞═══════════════╡ Real life ╞═╗
║ Mean                                5.588 ║
║ Best                               31.070 ║
║ Worst                               0.000 ║
║ Total                              10.406 ║
╠═╡ Cycles (-%) ╞═════════════╡ Real life ╞═╣
║ Mean                               -6.781 ║
║ Best                                0.000 ║
║ Worst                             -17.573 ║
║ Total                              -6.305 ║
╚═══════════════════════════════════════════╝

@vladimirradosavljevic
[SyncVM] Add missing implicit Flags reg in expand select pass
2543e01 
Signed-off-by: Vladimir Radosavljevic <vr@matterlabs.dev>
@vladimirradosavljevic
[SyncVM] Remove Flags uses from unconditional jump
2320b1e 
Signed-off-by: Vladimir Radosavljevic <vr@matterlabs.dev>
@zksync-admin-bot2
Copy link
Collaborator

Benchmark results:

╔═╡ Size (-%) ╞═════════════════════╡ All ╞═╗
║ Mean                                0.684 ║
║ Best                               37.020 ║
║ Worst                               0.000 ║
║ Total                               2.371 ║
╠═╡ Cycles (-%) ╞═══════════════════╡ All ╞═╣
║ Mean                                 -inf ║
║ Best                                0.000 ║
║ Worst                             -34.286 ║
║ Total                              -2.112 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞═══════════════╡ Real life ╞═╗
║ Mean                                5.588 ║
║ Best                               31.070 ║
║ Worst                               0.000 ║
║ Total                              10.406 ║
╠═╡ Cycles (-%) ╞═════════════╡ Real life ╞═╣
║ Mean                               -6.781 ║
║ Best                                0.000 ║
║ Worst                             -17.573 ║
║ Total                              -6.305 ║
╚═══════════════════════════════════════════╝

lialan
lialan approved these changes Jun 20, 2023
View reviewed changes
Copy link
Contributor

@lialan lialan left a comment

Choose a reason for hiding this comment

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

LGTM, just do remember to turn it off by default.

.github/workflows/compiler-integration-tests.yml Show resolved Hide resolved
llvm/lib/Target/SyncVM/SyncVMInstrInfo.cpp Outdated Show resolved Hide resolved
llvm/test/CodeGen/SyncVM/machine-outliner.mir Show resolved Hide resolved
@vladimirradosavljevic
Copy link
Contributor Author

@lialan Thanks for a review!

@vladimirradosavljevic
[SyncVM] Enable the machine outliner
0e00df8 
Signed-off-by: Vladimir Radosavljevic <vr@matterlabs.dev>
@zksync-admin-bot2
Copy link
Collaborator

Benchmark results:

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

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

akiramenai
akiramenai approved these changes Jun 21, 2023
View reviewed changes
Copy link
Collaborator

@akiramenai akiramenai left a comment

Choose a reason for hiding this comment

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

LGTM assuming flag handling logic will go off after switching to dynamic jumps.

@vladimirradosavljevic
Copy link
Contributor Author

@akiramenai Yes, we will remove flag handling logic when we switch to jumps. Thanks for a review!

@vladimirradosavljevic vladimirradosavljevic merged commit c8ca79b into dev-15 Jun 22, 2023
5 checks passed
@vladimirradosavljevic vladimirradosavljevic deleted the machine_outliner branch June 22, 2023 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FlashSheridan FlashSheridan FlashSheridan left review comments

@lialan lialan lialan approved these changes

@akiramenai akiramenai akiramenai approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@vladimirradosavljevic @zksync-admin-bot2 @lialan @FlashSheridan @akiramenai