Skip to content

LocalCSE: Do not optimize small things like global.get #6087

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

Merged
merged 1 commit into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 6 additions & 3 deletions src/passes/LocalCSE.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -334,13 +334,16 @@ struct Scanner
// 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
if (options.shrinkLevel > 0 && Measurer::measure(curr) >= 3) {
auto size = Measurer::measure(curr);
if (options.shrinkLevel > 0 && size >= 3) {
return true;
}

// If we focus on speed, any reduction in cost is beneficial, as the
// cost of a get is essentially free.
if (options.shrinkLevel == 0 && CostAnalyzer(curr).cost > 0) {
// cost of a get is essentially free. However, we need to balance that with
// the fact that the VM will also do CSE/GVN itself, so minor improvements
// are not worthwhile, so skip things of size 1 (like a global.get).
if (options.shrinkLevel == 0 && CostAnalyzer(curr).cost > 0 && size >= 2) {
return true;
}

Expand Down
68 changes: 31 additions & 37 deletions test/lit/passes/inlining-optimizing_optimize-level=3.wast
Original file line number Diff line number Diff line change
Expand Up @@ -299,13 +299,11 @@
;; CHECK-NEXT: (local $12 i32)
;; CHECK-NEXT: (local $13 i32)
;; CHECK-NEXT: (local.set $8
;; CHECK-NEXT: (local.tee $4
;; CHECK-NEXT: (global.get $STACKTOP)
;; CHECK-NEXT: )
;; CHECK-NEXT: (global.get $STACKTOP)
;; CHECK-NEXT: )
;; CHECK-NEXT: (global.set $STACKTOP
;; CHECK-NEXT: (i32.add
;; CHECK-NEXT: (local.get $4)
;; CHECK-NEXT: (global.get $STACKTOP)
;; CHECK-NEXT: (i32.const 16)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
Expand All @@ -316,11 +314,12 @@
;; CHECK-NEXT: )
;; CHECK-NEXT: (call $abort)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $6
;; CHECK-NEXT: (global.get $STACKTOP)
;; CHECK-NEXT: )
;; CHECK-NEXT: (global.set $STACKTOP
;; CHECK-NEXT: (i32.add
;; CHECK-NEXT: (local.tee $4
;; CHECK-NEXT: (global.get $STACKTOP)
;; CHECK-NEXT: )
;; CHECK-NEXT: (global.get $STACKTOP)
;; CHECK-NEXT: (i32.const 16)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
Expand All @@ -332,19 +331,20 @@
;; CHECK-NEXT: (call $abort)
;; CHECK-NEXT: )
;; CHECK-NEXT: (i32.store
;; CHECK-NEXT: (local.get $4)
;; CHECK-NEXT: (local.get $6)
;; CHECK-NEXT: (local.get $8)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $0
;; CHECK-NEXT: (i32.load
;; CHECK-NEXT: (i32.const 8)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $1
;; CHECK-NEXT: (global.get $STACKTOP)
;; CHECK-NEXT: )
;; CHECK-NEXT: (global.set $STACKTOP
;; CHECK-NEXT: (i32.add
;; CHECK-NEXT: (local.tee $1
;; CHECK-NEXT: (global.get $STACKTOP)
;; CHECK-NEXT: )
;; CHECK-NEXT: (global.get $STACKTOP)
;; CHECK-NEXT: (i32.const 224)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
Expand All @@ -361,13 +361,13 @@
;; CHECK-NEXT: (i32.const 120)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $5
;; CHECK-NEXT: (local.set $4
;; CHECK-NEXT: (i32.add
;; CHECK-NEXT: (local.get $1)
;; CHECK-NEXT: (i32.const 136)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $6
;; CHECK-NEXT: (local.set $5
;; CHECK-NEXT: (i32.add
;; CHECK-NEXT: (local.tee $3
;; CHECK-NEXT: (local.tee $7
Expand All @@ -393,14 +393,14 @@
;; CHECK-NEXT: (i32.const 4)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.get $6)
;; CHECK-NEXT: (local.get $5)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (i32.store
;; CHECK-NEXT: (local.get $2)
;; CHECK-NEXT: (i32.load
;; CHECK-NEXT: (local.get $4)
;; CHECK-NEXT: (local.get $6)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (drop
Expand Down Expand Up @@ -444,7 +444,7 @@
;; CHECK-NEXT: )
;; CHECK-NEXT: (if
;; CHECK-NEXT: (i32.load
;; CHECK-NEXT: (local.tee $6
;; CHECK-NEXT: (local.tee $5
;; CHECK-NEXT: (i32.add
;; CHECK-NEXT: (local.get $0)
;; CHECK-NEXT: (i32.const 48)
Expand Down Expand Up @@ -473,7 +473,7 @@
;; CHECK-NEXT: )
;; CHECK-NEXT: (i32.store
;; CHECK-NEXT: (local.get $9)
;; CHECK-NEXT: (local.get $5)
;; CHECK-NEXT: (local.get $4)
;; CHECK-NEXT: )
;; CHECK-NEXT: (i32.store
;; CHECK-NEXT: (local.tee $12
Expand All @@ -482,7 +482,7 @@
;; CHECK-NEXT: (i32.const 28)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.get $5)
;; CHECK-NEXT: (local.get $4)
;; CHECK-NEXT: )
;; CHECK-NEXT: (i32.store
;; CHECK-NEXT: (local.tee $11
Expand All @@ -491,10 +491,10 @@
;; CHECK-NEXT: (i32.const 20)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.get $5)
;; CHECK-NEXT: (local.get $4)
;; CHECK-NEXT: )
;; CHECK-NEXT: (i32.store
;; CHECK-NEXT: (local.get $6)
;; CHECK-NEXT: (local.get $5)
;; CHECK-NEXT: (i32.const 80)
;; CHECK-NEXT: )
;; CHECK-NEXT: (i32.store
Expand All @@ -505,7 +505,7 @@
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (i32.add
;; CHECK-NEXT: (local.get $5)
;; CHECK-NEXT: (local.get $4)
;; CHECK-NEXT: (i32.const 80)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
Expand Down Expand Up @@ -547,7 +547,7 @@
;; CHECK-NEXT: (local.get $10)
;; CHECK-NEXT: )
;; CHECK-NEXT: (i32.store
;; CHECK-NEXT: (local.get $6)
;; CHECK-NEXT: (local.get $5)
;; CHECK-NEXT: (i32.const 0)
;; CHECK-NEXT: )
;; CHECK-NEXT: (i32.store
Expand Down Expand Up @@ -586,7 +586,7 @@
;; CHECK-NEXT: (local.get $1)
;; CHECK-NEXT: )
;; CHECK-NEXT: (global.set $STACKTOP
;; CHECK-NEXT: (local.get $4)
;; CHECK-NEXT: (local.get $6)
;; CHECK-NEXT: )
;; CHECK-NEXT: (global.set $STACKTOP
;; CHECK-NEXT: (local.get $8)
Expand Down Expand Up @@ -4080,13 +4080,11 @@
;; CHECK-NEXT: (local $44 i32)
;; CHECK-NEXT: (local $45 i32)
;; CHECK-NEXT: (local.set $13
;; CHECK-NEXT: (local.tee $5
;; CHECK-NEXT: (global.get $STACKTOP)
;; CHECK-NEXT: )
;; CHECK-NEXT: (global.get $STACKTOP)
;; CHECK-NEXT: )
;; CHECK-NEXT: (global.set $STACKTOP
;; CHECK-NEXT: (i32.add
;; CHECK-NEXT: (local.get $5)
;; CHECK-NEXT: (global.get $STACKTOP)
;; CHECK-NEXT: (i32.const 624)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
Expand Down Expand Up @@ -5785,21 +5783,19 @@
;; CHECK-NEXT: (i32.const 0)
;; CHECK-NEXT: )
;; CHECK-NEXT: (f64.store
;; CHECK-NEXT: (local.tee $5
;; CHECK-NEXT: (global.get $tempDoublePtr)
;; CHECK-NEXT: )
;; CHECK-NEXT: (global.get $tempDoublePtr)
;; CHECK-NEXT: (local.get $14)
;; CHECK-NEXT: )
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (i32.load
;; CHECK-NEXT: (local.get $5)
;; CHECK-NEXT: (global.get $tempDoublePtr)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $30
;; CHECK-NEXT: (if (result i32)
;; CHECK-NEXT: (i32.lt_s
;; CHECK-NEXT: (i32.load offset=4
;; CHECK-NEXT: (local.get $5)
;; CHECK-NEXT: (global.get $tempDoublePtr)
;; CHECK-NEXT: )
;; CHECK-NEXT: (i32.const 0)
;; CHECK-NEXT: )
Expand Down Expand Up @@ -5844,22 +5840,20 @@
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (f64.store
;; CHECK-NEXT: (local.tee $5
;; CHECK-NEXT: (global.get $tempDoublePtr)
;; CHECK-NEXT: )
;; CHECK-NEXT: (global.get $tempDoublePtr)
;; CHECK-NEXT: (local.get $14)
;; CHECK-NEXT: )
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (i32.load
;; CHECK-NEXT: (local.get $5)
;; CHECK-NEXT: (global.get $tempDoublePtr)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $7
;; CHECK-NEXT: (if (result i32)
;; CHECK-NEXT: (i32.lt_u
;; CHECK-NEXT: (i32.and
;; CHECK-NEXT: (i32.load offset=4
;; CHECK-NEXT: (local.get $5)
;; CHECK-NEXT: (global.get $tempDoublePtr)
;; CHECK-NEXT: )
;; CHECK-NEXT: (i32.const 2146435072)
;; CHECK-NEXT: )
Expand Down
15 changes: 8 additions & 7 deletions test/lit/passes/local-cse.wast
Original file line number Diff line number Diff line change
Expand Up @@ -447,20 +447,17 @@
(global $other-glob (mut i32) (i32.const 1))

;; CHECK: (func $global
;; CHECK-NEXT: (local $0 i32)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (local.tee $0
;; CHECK-NEXT: (global.get $glob)
;; CHECK-NEXT: )
;; CHECK-NEXT: (global.get $glob)
;; CHECK-NEXT: )
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (local.get $0)
;; CHECK-NEXT: (global.get $glob)
;; CHECK-NEXT: )
;; CHECK-NEXT: (global.set $other-glob
;; CHECK-NEXT: (i32.const 100)
;; CHECK-NEXT: )
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (local.get $0)
;; CHECK-NEXT: (global.get $glob)
;; CHECK-NEXT: )
;; CHECK-NEXT: (global.set $glob
;; CHECK-NEXT: (i32.const 200)
Expand All @@ -470,7 +467,11 @@
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $global
;; We should optimize redundant global.get operations.
;; We should not optimize redundant global.get operations: they are of size
;; 1 (no children), and so we may end up increasing code size here for
;; unclear benefit. The benefit is unclear since VMs already do GVN/CSE
;; themselves, and so we focus on things of size 2 and above, where we
;; definitely reduce code size at least.
(drop (global.get $glob))
(drop (global.get $glob))
;; We can do it past a write to another global
Expand Down