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

Optimize JUMPDEST analysis #306

Merged
merged 1 commit into from
Apr 22, 2021
Merged

Optimize JUMPDEST analysis #306

merged 1 commit into from
Apr 22, 2021

Conversation

chfast
Copy link
Member

@chfast chfast commented Apr 22, 2021

Optimize the build_jumpdest_map() procedure for JUMPDEST analysis.

  • "is push opcode" conditions has been changed to (op & 0xE0) == 0x60.
  • the op == JUMPDEST condition has been marked as unlikely.

@chfast chfast requested review from axic, gumb0, holiman and yperbasis April 22, 2021 10:21
holiman
holiman approved these changes Apr 22, 2021
View reviewed changes
Copy link

@holiman holiman left a comment

Choose a reason for hiding this comment

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

lgtm

@chfast
Copy link
Member Author

chfast commented Apr 22, 2021

Benchmarks

Time are in µs.

Haswell 4.0 GHz, Clang 12

Comparing o/ba-0 to o/ba-opt                                                                                                                                                                                                      
Benchmark                                                                Time             CPU      Time Old      Time New       CPU Old       CPU New                                                                             
-----------------------------------------------------------------------------------------------------------------------------------------------------
baseline/analyse/main/blake2b_huff_mean                               -0.6025         -0.6025            15             6            15             6
baseline/analyse/main/blake2b_shifts_mean                             -0.6836         -0.6836            11             4            11             4
baseline/analyse/main/sha1_divs_mean                                  -0.7625         -0.7625             2             1             2             1
baseline/analyse/main/sha1_shifts_mean                                -0.7681         -0.7681             2             1             2             1
baseline/analyse/main/weierstrudel_mean                               -0.5738         -0.5738            18             8            18             8
baseline/analyse/micro/beginsub_push1s_0xffff_mean                    +0.0411         +0.0411            62            65            62            65
baseline/analyse/micro/beginsubs_0xffff_mean                          +0.3286         +0.3286            25            33            25            33
baseline/analyse/micro/jumpdests_0xffff_mean                          +0.0700         +0.0700            80            86            80            86
baseline/analyse/micro/loop_with_many_jumpdests_mean                  +0.0730         +0.0730            30            32            30            32
baseline/analyse/micro/push1s_0xffff_mean                             +0.0363         +0.0363            66            68            66            68
baseline/analyse/micro/push32s_0xffff_mean                            +0.0407         +0.0407             4             4             4             4
baseline/analyse/micro/zeros_0xffff_mean                              +0.3301         +0.3301            25            33            25            33

Haswell 4.0 GHz, GCC 10

Comparing o/ba-0 to o/ba-opt                                                                                                                                                                                                      
Benchmark                                                                Time             CPU      Time Old      Time New       CPU Old       CPU New                                                                             
-----------------------------------------------------------------------------------------------------------------------------------------------------
baseline/analyse/main/blake2b_huff_mean                               -0.2021         -0.2021             8             6             8             6
baseline/analyse/main/blake2b_shifts_mean                             -0.0415         -0.0415             4             4             4             4
baseline/analyse/main/sha1_divs_mean                                  -0.0608         -0.0608             1             1             1             1
baseline/analyse/main/sha1_shifts_mean                                -0.0591         -0.0591             1             1             1             1
baseline/analyse/main/weierstrudel_mean                               -0.0665         -0.0665             9             8             9             8
baseline/analyse/micro/beginsub_push1s_0xffff_mean                    -0.0131         -0.0131            71            70            71            70
baseline/analyse/micro/beginsubs_0xffff_mean                          +0.0005         +0.0005            33            33            33            33
baseline/analyse/micro/jumpdests_0xffff_mean                          +0.0153         +0.0153            89            90            89            90
baseline/analyse/micro/loop_with_many_jumpdests_mean                  +0.0171         +0.0171            33            34            33            34
baseline/analyse/micro/push1s_0xffff_mean                             -0.0083         -0.0083            75            74            75            74
baseline/analyse/micro/push32s_0xffff_mean                            -0.0059         -0.0059             5             5             5             5
baseline/analyse/micro/zeros_0xffff_mean                              -0.0006         -0.0006            33            33            33            33

gumb0
gumb0 reviewed Apr 22, 2021
View reviewed changes
lib/evmone/baseline.cpp Outdated Show resolved Hide resolved
gumb0
gumb0 reviewed Apr 22, 2021
View reviewed changes
lib/evmone/baseline.cpp Outdated
JumpdestMap m(code_size);
// Bitmask for PUSH instructions identification.
// It removes the lower 5 bits of a PUSH instruction with information about the data length.
// The remaining top 3 bits of every PUSH instruction is 0b011, i.e. 0x60, OP_PUSH1.
Copy link
Member

@gumb0 gumb0 Apr 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This sentence confused is what confused me, but probably it's just me.

Copy link
Member Author

Choose a reason for hiding this comment

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

Better now?

Base automatically changed from baseline_export_build_jumpdest_map to master April 22, 2021 13:56
@chfast chfast force-pushed the build_jumpdest_map branch from b04b001 to 05d6e10 Compare April 22, 2021 14:04
@chfast
Optimize build_jumpdest_map()
d7abe19 
Optimize the build_jumpdest_map() procedure for JUMPDEST analysis.
- "is push opcode" conditions has been changed to (op & 0xE0) == 0x60.
- the op == JUMPDEST condition has been marked as unlikely.
@chfast chfast force-pushed the build_jumpdest_map branch from 05d6e10 to d7abe19 Compare April 22, 2021 15:44
@chfast chfast merged commit 9029ece into master Apr 22, 2021
@chfast chfast deleted the build_jumpdest_map branch April 22, 2021 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gumb0 gumb0 gumb0 approved these changes

@yperbasis yperbasis yperbasis approved these changes

@holiman holiman holiman approved these changes

@axic axic Awaiting requested review from axic

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

Successfully merging this pull request may close these issues.
4 participants
@chfast @holiman @gumb0 @yperbasis