Skip to content

Optimize more reduction-of-an-extend cases#7711

Merged
alexcrichton merged 2 commits intobytecodealliance:mainfrom
scottmcm:reduce-of-extend
Jan 3, 2024
Merged

Optimize more reduction-of-an-extend cases#7711
alexcrichton merged 2 commits intobytecodealliance:mainfrom
scottmcm:reduce-of-extend

Conversation

@scottmcm
Copy link
Contributor

@scottmcm scottmcm commented Dec 20, 2023

I was exploring ireduce patterns more after I removed them from #7693 (comment) at fitzgen's request.

In doing so, I noticed that there were some simpler cases missing first, like ireduce.i8 sextend.i32 my_i16ireduce.i8 my_i16 and ireduce.i16 sextend.i32 my_i8sextend.i16 my_i8.

So this PR adds those (and the unsigned equivalents).

@scottmcm scottmcm requested a review from a team as a code owner December 20, 2023 04:19
@scottmcm scottmcm requested review from abrown and removed request for a team December 20, 2023 04:19
@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator isle Related to the ISLE domain-specific language labels Dec 20, 2023
@github-actions
Copy link

Subscribe to Label Action

cc @cfallin, @fitzgen

Details This issue or pull request has been labeled: "cranelift", "isle"

Thus the following users have been cc'd because of the following labels:

  • cfallin: isle
  • fitzgen: isle

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

@fitzgen
Copy link
Member

fitzgen commented Dec 20, 2023

Thanks for splitting this out!

Would you mind enabling the trace-log feature and looking at this trace log when running wasmtime compile sightglass/benchmarks/spidermonkey/benchmark.wasm1 and comparing the numbers before and after this PR? I want to make sure we aren't accidentally blowing up the size of the egraph.

Footnotes

  1. github.com/bytecodealliance/sightglass

@scottmcm
Copy link
Contributor Author

I don't really know what I'm doing, but here's what I got: 7711.diff.txt

I ran .\target\release\wasmtime.exe compile ..\spidermonkey_benchmark.wasm -D log-to-files -C parallel-compilation=n before and after, which meant all the logs were in "wasmtime.dbg.main".

Looks plausible? Nothing that seems like a huge blow-up, but a bunch of places with a couple more CLIF instructions that end up as fewer lowered vcode instructions
image

Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

Stats look good -- thanks for getting those!

@fitzgen fitzgen added this pull request to the merge queue Jan 2, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 2, 2024
@alexcrichton alexcrichton added this pull request to the merge queue Jan 2, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 2, 2024
@alexcrichton
Copy link
Member

The first failure above was spurious, but the second failure looks like it might be related to this PR

@scottmcm
Copy link
Contributor Author

scottmcm commented Jan 3, 2024

Yup, conflicted with myself from #7719. Should be good now.

@alexcrichton alexcrichton added this pull request to the merge queue Jan 3, 2024
Merged via the queue into bytecodealliance:main with commit 2bd9002 Jan 3, 2024
@scottmcm scottmcm deleted the reduce-of-extend branch January 3, 2024 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cranelift Issues related to the Cranelift code generator isle Related to the ISLE domain-specific language

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants