scottmcm requested wasmtime-compiler-reviewers for a review on PR #7711.
scottmcm requested abrown for a review on PR #7711.
scottmcm opened PR #7711 from scottmcm:reduce-of-extend
to bytecodealliance:main
:
I was exploring
ireduce
patterns more after I removed them from https://github.com/bytecodealliance/wasmtime/pull/7693#discussion_r1430497487 at fitzgen's request.In doing so, I noticed that there were some simpler cases missing first, like
(char)(long)my_int
→(char)my_int
and(long)(short)my_char
→(long)my_char
.So this PR adds those (and the unsigned equivalents).
scottmcm edited PR #7711:
I was exploring
ireduce
patterns more after I removed them from https://github.com/bytecodealliance/wasmtime/pull/7693#discussion_r1430497487 at fitzgen's request.In doing so, I noticed that there were some simpler cases missing first, like
ireduce.i8 sextend.i32 my_i16
→ireduce.i8 my_i16
andireduce.i16 sextend.i32 my_i8
→sextend.i16 my_i8
.So this PR adds those (and the unsigned equivalents).
github-actions[bot] commented on PR #7711:
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 <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
fitzgen commented on PR #7711:
Thanks for splitting this out!
Would you mind enabling the
trace-log
feature and looking at this trace log when runningwasmtime compile sightglass/benchmarks/spidermonkey/benchmark.wasm
[^0] 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.
scottmcm commented on PR #7711:
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](https://github.com/bytecodealliance/wasmtime/assets/18526288/b863d2a4-1926-4bb5-9bd2-f82202f62edb)
fitzgen submitted PR review:
Stats look good -- thanks for getting those!
alexcrichton commented on PR #7711:
The first failure above was spurious, but the second failure looks like it might be related to this PR
scottmcm updated PR #7711.
scottmcm commented on PR #7711:
Yup, conflicted with myself from #7719. Should be good now.
alexcrichton merged PR #7711.
Last updated: Nov 22 2024 at 16:03 UTC