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

x64: add x64 encoding benchmarks #2888

Merged
merged 4 commits into from
May 13, 2021
Merged

Conversation

abrown
Copy link
Contributor

@abrown abrown commented May 10, 2021

This is a follow-up to #2819 to add a benchmark comparing the different encoding approaches (builder vs function) used to encode EVEX instructions. The measurements I observed indicate that the builder pattern is faster, as noted in #2819. The more interesting improvement of this PR is to show how to measure parts of the cranelift-codegen crate. To do so with criterion, I had to make some of the x64 module public:

  • the first commit moves the encoding module out of x64::inst so that x64::inst does not have to become public
  • the second makes the x64 module itself public
  • the third adds the benchmark.

@abrown abrown requested a review from cfallin May 10, 2021 18:15
@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator cranelift:area:x64 Issues related to x64 codegen labels May 10, 2021
@abrown
Copy link
Contributor Author

abrown commented May 11, 2021

@cfallin, I went ahead and disabled the cargo deny check for the crates that criterion transitively pulls in. This should be ready to review.

Copy link
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

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

Looks good! If you want to look into it, I think there's a way to feature-gate the whole bench on x86 being compiled in, but since it's not built by default (needs a specific cargo bench invocation) I think it's not the end of the world if that doesn't work either.

cranelift/codegen/benches/x64-evex-encoding.rs Outdated Show resolved Hide resolved
abrown added 4 commits May 13, 2021 09:52
In order to benchmark the encoding code with criterion, the functions
and structures must be public. Moving this code to its own module
(instead of keeping as a submodule to `inst`), allows `inst` to remain
private. This avoids having to expose and document (or ignore
documenting) the numerous instruction variants in `inst` while allowing
access to the encoding code. This commit changes no functionality.
In order to benchmark portions of the x64 module, this change makes it a
public module of cranelift-codegen.
This change adds a criterion-enabled benchmark, x64-evex-encoding, to
compare the performance of the builder pattern used to encode EVEX
instructions in the new x64 backend against the function pattern
used to encode EVEX instructions in the legacy x86 backend. At face
value, the results imply that the builder pattern is faster, but no
efforts were made to analyze and optimize these approaches further.
Until japaric/cast.rs#26 is resolved, the `cast`
crate will pull in older versions of the `rustc_version`, `semver`, and
`semver-parser` crates. `cast` is a build dependency of `criterion`
which is used for benchmarking and is itself a dev dependency, not a
normal dependency.
Copy link
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

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

Looks good!

@abrown abrown merged commit 6fb2a24 into bytecodealliance:main May 13, 2021
@abrown abrown deleted the inst-format-4 branch May 13, 2021 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift:area:x64 Issues related to x64 codegen cranelift Issues related to the Cranelift code generator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants