-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
cranelift: Add x < 0
-> x >>> bit_size(ty) - 1
lowering rule
#4607
Comments
x < 0
-> x >>> it_size(ty) - 1
peephole rulex < 0
-> x >>> bit_size(ty) - 1
peephole rule
x < 0
-> x >>> bit_size(ty) - 1
peephole rulex < 0
-> x >>> bit_size(ty) - 1
peephole rule
This wouldn't help on platforms without flag registers, right? Those have icmp and bint merged together already. If anything it would prevent icmp+brz/brnz fusion I think, even on platforms with flag registers. |
Hmm, so perhaps it's better to do on lower level (may be ISLE) for only specific platforms after icmp+brz/brnz fusion? |
I think so. |
We can definitely incorporate this as a lowering rule case in the backends. The ISLE and lowering machinery is smart enough to do the right thing for the |
x < 0
-> x >>> bit_size(ty) - 1
peephole rulex < 0
-> x >>> bit_size(ty) - 1
lowering rule
Feature
Simple optimization which rewrite
x < 0
(icmp_imm slt x, 0
) tox >>> bit_size(ty) - 1
(ushr_imm x, 31 | 63
).Benefit
x < 0
is pretty common operation. Although LLVM itself applies this rule, but some code generators / optimizers such as Binaryen (wasm-opt / wasm-pack use it) don't such peephole rewrites because replacing the constant0
to31
or63
has a negative effect on entropy when compressing wasm module via gzip or brotli.Besides, such optimization will improve cranelift IR itself, because it will remove the finalizing
bint.i32
/bint.i64
aftericmp
. Forushr
/ushr_imm
it is not needed.Implementation
filetests/simple_preopt/simplily32.clif
andfiletests/simple_preopt/simplily64.clif
Updated Implementation Plan
Write necessary rules on ISLE instead in
simple_opt.rs
The text was updated successfully, but these errors were encountered: