Stream: git-wasmtime

Topic: wasmtime / issue #4607 cranelift: Add `x < 0` -> `x >>> b...

view this post on Zulip Wasmtime GitHub notifications bot (Aug 04 2022 at 10:28):

MaxGraey edited issue #4607:

Feature

Simple optimization which rewrite x < 0 (icmp slt_imm x, 0) to x >>> 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 constant 0 to 31 or 63 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 after icmp.
for ushr/ushr_imm it is not needed.

Implementation

view this post on Zulip Wasmtime GitHub notifications bot (Aug 04 2022 at 11:03):

bjorn3 commented on issue #4607:

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.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 04 2022 at 11:12):

MaxGraey commented on issue #4607:

Hmm, so perhaps it's better to do on lower level (may be ISTLE) for only specific platforms?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 04 2022 at 12:15):

MaxGraey edited a comment on issue #4607:

Hmm, so perhaps it's better to do on lower level (may be ISLE) for only specific platforms?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 04 2022 at 12:16):

MaxGraey edited a comment on issue #4607:

Hmm, so perhaps it's better to do on lower level (may be ISLE) for only specific platforms after icmp+brz/brnz fusion?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 04 2022 at 12:44):

bjorn3 commented on issue #4607:

I think so.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 04 2022 at 15:11):

cfallin commented on issue #4607:

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 (brnz (icmp ...)) case and allow for a rule for (icmp (IntCC.NP) ...) when used directly to compute that value. cc @elliottt as he is in the middle of br+compare stuff in x86-64 right now...

view this post on Zulip Wasmtime GitHub notifications bot (Aug 04 2022 at 15:25):

MaxGraey edited issue #4607:

Feature

Simple optimization which rewrite x < 0 (icmp_imm slt x, 0) to x >>> 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 constant 0 to 31 or 63 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 after icmp.
for ushr/ushr_imm it is not needed.

Implementation

view this post on Zulip Wasmtime GitHub notifications bot (Aug 04 2022 at 16:17):

MaxGraey edited issue #4607:

Feature

Simple optimization which rewrite x < 0 (icmp_imm slt x, 0) to x >>> 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 constant 0 to 31 or 63 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 after icmp. For ushr/ushr_imm it is not needed.

Implementation

view this post on Zulip Wasmtime GitHub notifications bot (Aug 04 2022 at 16:18):

MaxGraey edited issue #4607:

Feature

Simple optimization which rewrite x < 0 (icmp_imm slt x, 0) to x >>> 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 constant 0 to 31 or 63 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 after icmp. For ushr/ushr_imm it is not needed.

Implementation

Update Implementation Plan
Write necessary rules on ISLE instead simple_opt.rs

view this post on Zulip Wasmtime GitHub notifications bot (Aug 04 2022 at 16:19):

MaxGraey edited issue #4607:

Feature

Simple optimization which rewrite x < 0 (icmp_imm slt x, 0) to x >>> 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 constant 0 to 31 or 63 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 after icmp. For ushr/ushr_imm it is not needed.

Implementation

Updated Implementation Plan

Write necessary rules on ISLE instead simple_opt.rs

view this post on Zulip Wasmtime GitHub notifications bot (Aug 04 2022 at 16:19):

MaxGraey edited issue #4607:

Feature

Simple optimization which rewrite x < 0 (icmp_imm slt x, 0) to x >>> 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 constant 0 to 31 or 63 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 after icmp. For ushr/ushr_imm it is not needed.

Implementation

Updated Implementation Plan

Write necessary rules on ISLE instead in simple_opt.rs

view this post on Zulip Wasmtime GitHub notifications bot (Aug 04 2022 at 16:19):

MaxGraey edited issue #4607:

Feature

Simple optimization which rewrite x < 0 (icmp_imm slt x, 0) to x >>> 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 constant 0 to 31 or 63 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 after icmp. For ushr/ushr_imm it is not needed.

Implementation

Updated Implementation Plan

Write necessary rules on ISLE instead in simple_opt.rs

view this post on Zulip Wasmtime GitHub notifications bot (Aug 09 2022 at 16:45):

elliottt closed issue #4607:

Feature

Simple optimization which rewrite x < 0 (icmp_imm slt x, 0) to x >>> 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 constant 0 to 31 or 63 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 after icmp. For ushr/ushr_imm it is not needed.

Implementation

Updated Implementation Plan

Write necessary rules on ISLE instead in simple_opt.rs

Last updated: Dec 23 2024 at 12:05 UTC