Skip to content

[LocalCSE] Consider size-2 expressions when shrinking #9161

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.

Sign up for GitHub

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

Open
artemkulyk wants to merge 1 commit into
base: main
Choose a base branch
from
+122 −28
Open

[LocalCSE] Consider size-2 expressions when shrinking #9161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Current Trunk

- Add a new wasm-embed tool for extracting Wasm modules embedded as byte arrays
in JS source files or replacing such modules in the JS source (#9127)
- LocalCSE now considers size-2 expressions at -Os and -Oz. (#9161)

v133
----
Expand Down
43 changes: 35 additions & 8 deletions src/passes/LocalCSE.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,12 +358,12 @@ struct Scanner
return false;
}

// If the size is at least 3, then if we have two of them we have 6,
// and so adding one set+one get and removing one of the items itself
// is not detrimental, and may be beneficial.
// TODO: investigate size 2
// A size of 2 is the smallest where reuse can pay off: with two
// appearances we remove one of them while adding a local.set and a
// local.get. The Applier also skips the cases that are clearly not
// profitable, where the number of requests is known (see worthApplying).
auto size = Measurer::measure(curr);
if (options.shrinkLevel > 0 && size >= 3) {
if (options.shrinkLevel > 0 && size >= 2) {
return true;
}

Expand Down Expand Up @@ -586,13 +586,36 @@ struct Checker
struct Applier
: public LinearExecutionWalker<Applier, UnifiedExpressionVisitor<Applier>> {
RequestInfoMap requestInfos;
PassOptions& options;

// Originals that we decided are not worth applying (in size-focused modes,
// where a small expression replaced a single time can end up larger, as we
// add a local set and get pair as well as a new local).
std::unordered_set<Expression*> skipped;

Applier(RequestInfoMap& requestInfos) : requestInfos(requestInfos) {}
Applier(RequestInfoMap& requestInfos, PassOptions& options)
: requestInfos(requestInfos), options(options) {}

// Maps the original expressions that we save to locals to the local indexes
// for them.
std::unordered_map<Expression*, Index> originalLocalMap;

bool worthApplying(Expression* curr, Index requests) {
if (options.shrinkLevel == 0) {
return true;
}
// A tiny expression (two IR nodes, e.g. a load of a local) replaced a
// single time is not worth it: we remove one small expression but add a
// local.set, a local.get, and a new local. If the function already has
// locals then coalesce-locals will usually merge the new local into one of
// them, so only restrict this when there are no locals at all.
if (Measurer::measure(curr) <= 2 && requests < 2 &&
getFunction()->vars.empty()) {
return false;
}
return true;
}

void visitExpression(Expression* curr) {
auto iter = requestInfos.find(curr);
if (iter == requestInfos.end()) {
Expand All @@ -603,6 +626,10 @@ struct Applier
info.validate();

if (info.requests) {
if (!worthApplying(curr, info.requests)) {
skipped.insert(curr);
return;
}
// We have requests for this value. Add a local and tee the value to
// there.
Index local = originalLocalMap[curr] =
Expand All @@ -611,7 +638,7 @@ struct Applier
Builder(*getModule()).makeLocalTee(local, curr, curr->type));
} else if (info.original) {
auto& originalInfo = requestInfos.at(info.original);
if (originalInfo.requests) {
if (originalInfo.requests && !skipped.contains(info.original)) {
// This is a valid request of an original value. Get the value from the
// local.
assert(originalLocalMap.contains(info.original));
Expand Down Expand Up @@ -663,7 +690,7 @@ struct LocalCSE : public WalkerPass<PostWalker<LocalCSE>> {
return;
}

Applier applier(requestInfos);
Applier applier(requestInfos, options);
applier.walkFunctionInModule(func, getModule());
}
};
Expand Down
66 changes: 66 additions & 0 deletions test/lit/passes/local-cse_shrink.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
;; RUN: foreach %s %t wasm-opt --local-cse --shrink-level=1 -S -o - | filecheck %s

;; Under shrink levels, a size-2 expression reused once is only worth it when
;; the new local can be merged into an existing one.

(module
(memory 1)
;; CHECK: (type $0 (func (param i32) (result i32)))

;; CHECK: (memory $0 1)

;; CHECK: (export "no-locals" (func $no-locals))

;; CHECK: (export "with-locals" (func $with-locals))

;; CHECK: (func $no-locals (param $p i32) (result i32)
;; CHECK-NEXT: (i32.add
;; CHECK-NEXT: (i32.load
;; CHECK-NEXT: (local.get $p)
;; CHECK-NEXT: )
;; CHECK-NEXT: (i32.load
;; CHECK-NEXT: (local.get $p)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $no-locals (export "no-locals") (param $p i32) (result i32)
(i32.add
(i32.load
(local.get $p)
)
(i32.load
(local.get $p)
)
)
)
;; CHECK: (func $with-locals (param $p i32) (result i32)
;; CHECK-NEXT: (local $x i32)
;; CHECK-NEXT: (local $2 i32)
;; CHECK-NEXT: (local.set $x
;; CHECK-NEXT: (i32.const 1)
;; CHECK-NEXT: )
;; CHECK-NEXT: (i32.add
;; CHECK-NEXT: (local.tee $2
;; CHECK-NEXT: (i32.load
;; CHECK-NEXT: (local.get $p)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.get $2)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $with-locals (export "with-locals") (param $p i32) (result i32)
(local $x i32)
(local.set $x
(i32.const 1)
)
(i32.add
(i32.load
(local.get $p)
)
(i32.load
(local.get $p)
)
)
)
)
20 changes: 10 additions & 10 deletions test/wasm2js/conversions-emscripten-modified.2asm.js.opt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,28 +37,28 @@ function asmFunc(imports) {

function legalstub$4($0_1) {
var $1_1 = 0, $2_1 = 0;
$2_1 = ~~$0_1 >>> 0;
$1_1 = ~~$0_1 >>> 0;
if (Math_fround(Math_abs($0_1)) >= Math_fround(1.0)) {
$1_1 = ~~($0_1 > Math_fround(0.0) ? Math_fround(Math_min(Math_fround(Math_floor(Math_fround($0_1 * Math_fround(2.3283064365386963e-10)))), Math_fround(4294967296.0))) : Math_fround(Math_ceil(Math_fround(Math_fround($0_1 - Math_fround(~~$0_1 >>> 0 >>> 0)) * Math_fround(2.3283064365386963e-10))))) >>> 0
$2_1 = ~~($0_1 > Math_fround(0.0) ? Math_fround(Math_min(Math_fround(Math_floor(Math_fround($0_1 * Math_fround(2.3283064365386963e-10)))), Math_fround(4294967296.0))) : Math_fround(Math_ceil(Math_fround(Math_fround($0_1 - Math_fround($1_1 >>> 0)) * Math_fround(2.3283064365386963e-10))))) >>> 0
} else {
$1_1 = 0
$2_1 = 0
}
i64toi32_i32$HIGH_BITS = $1_1;
i64toi32_i32$HIGH_BITS = $2_1;
setTempRet0(i64toi32_i32$HIGH_BITS | 0);
return $2_1;
return $1_1;
}

function legalstub$6($0_1) {
var $1_1 = 0, $2_1 = 0;
$2_1 = ~~$0_1 >>> 0;
$1_1 = ~~$0_1 >>> 0;
if (Math_abs($0_1) >= 1.0) {
$1_1 = ~~($0_1 > 0.0 ? Math_min(Math_floor($0_1 * 2.3283064365386963e-10), 4294967295.0) : Math_ceil(($0_1 - +(~~$0_1 >>> 0 >>> 0)) * 2.3283064365386963e-10)) >>> 0
$2_1 = ~~($0_1 > 0.0 ? Math_min(Math_floor($0_1 * 2.3283064365386963e-10), 4294967295.0) : Math_ceil(($0_1 - +($1_1 >>> 0)) * 2.3283064365386963e-10)) >>> 0
} else {
$1_1 = 0
$2_1 = 0
}
i64toi32_i32$HIGH_BITS = $1_1;
i64toi32_i32$HIGH_BITS = $2_1;
setTempRet0(i64toi32_i32$HIGH_BITS | 0);
return $2_1;
return $1_1;
}

// EMSCRIPTEN_END_FUNCS
Expand Down
20 changes: 10 additions & 10 deletions test/wasm2js/conversions-modified.2asm.js.opt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,28 +122,28 @@ function asmFunc(imports) {

function legalstub$7($0) {
var $1 = 0, $2 = 0;
$2 = ~~$0 >>> 0;
$1 = ~~$0 >>> 0;
if (Math_fround(Math_abs($0)) >= Math_fround(1.0)) {
$1 = ~~($0 > Math_fround(0.0) ? Math_fround(Math_min(Math_fround(Math_floor(Math_fround($0 * Math_fround(2.3283064365386963e-10)))), Math_fround(4294967296.0))) : Math_fround(Math_ceil(Math_fround(Math_fround($0 - Math_fround(~~$0 >>> 0 >>> 0)) * Math_fround(2.3283064365386963e-10))))) >>> 0
$2 = ~~($0 > Math_fround(0.0) ? Math_fround(Math_min(Math_fround(Math_floor(Math_fround($0 * Math_fround(2.3283064365386963e-10)))), Math_fround(4294967296.0))) : Math_fround(Math_ceil(Math_fround(Math_fround($0 - Math_fround($1 >>> 0)) * Math_fround(2.3283064365386963e-10))))) >>> 0
} else {
$1 = 0
$2 = 0
}
i64toi32_i32$HIGH_BITS = $1;
i64toi32_i32$HIGH_BITS = $2;
setTempRet0(i64toi32_i32$HIGH_BITS | 0);
return $2;
return $1;
}

function legalstub$9($0) {
var $1 = 0, $2 = 0;
$2 = ~~$0 >>> 0;
$1 = ~~$0 >>> 0;
if (Math_abs($0) >= 1.0) {
$1 = ~~($0 > 0.0 ? Math_min(Math_floor($0 * 2.3283064365386963e-10), 4294967295.0) : Math_ceil(($0 - +(~~$0 >>> 0 >>> 0)) * 2.3283064365386963e-10)) >>> 0
$2 = ~~($0 > 0.0 ? Math_min(Math_floor($0 * 2.3283064365386963e-10), 4294967295.0) : Math_ceil(($0 - +($1 >>> 0)) * 2.3283064365386963e-10)) >>> 0
} else {
$1 = 0
$2 = 0
}
i64toi32_i32$HIGH_BITS = $1;
i64toi32_i32$HIGH_BITS = $2;
setTempRet0(i64toi32_i32$HIGH_BITS | 0);
return $2;
return $1;
}

function legalstub$12($0, $1) {
Expand Down