Skip to content

OptimizeInstructions: Eq64 of 0 => Eqz64 #2421

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

Merged
merged 3 commits into from
Nov 4, 2019
Merged

OptimizeInstructions: Eq64 of 0 => Eqz64 #2421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
195826f
wip
kripken Nov 4, 2019
5a54095
tests
kripken Nov 4, 2019
a4797e9
fix
kripken Nov 4, 2019
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
2 changes: 2 additions & 0 deletions src/passes/OptimizeInstructions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1312,6 +1312,8 @@ struct OptimizeInstructions
!EffectAnalyzer(getPassOptions(), binary->left)
.hasSideEffects()) {
return binary->right;
} else if (binary->op == EqInt64) {
return Builder(*getModule()).makeUnary(EqZInt64, binary->left);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this optimization done for i32.eqz? I would expect this optimization to be organized to be symmetrically located with the i32 version.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other one appears to be here:

binaryen/src/passes/OptimizeInstructions.cpp

Line 373 in 7fa8c78

if (binary->op == EqInt32 && c->value.geti32() == 0) {
also handling various sign extension cases, which made me shy away from a quick fix as it seems there's more to it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is not structured in a very symmetrical way atm. It grew organically + some thoughts about checking common patterns early. So i32.eqz is much more common than i64.eqz, and hence they are differently located. It's very possible that's not worth it, and this should be refactored.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, LGTM for now.

}
}
// operations on all 1s
Expand Down
13 changes: 6 additions & 7 deletions test/passes/O.bin.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@
(export "fac-opt" (func $4))
(func $0 (; 0 ;) (; has Stack IR ;) (type $0) (param $0 i64) (result i64)
(if (result i64)
(i64.eq
(i64.eqz
(local.get $0)
(i64.const 0)
)
(i64.const 1)
(i64.mul
Expand All @@ -25,9 +24,8 @@
)
(func $1 (; 1 ;) (; has Stack IR ;) (type $0) (param $0 i64) (result i64)
(if (result i64)
(i64.eq
(i64.eqz
(local.get $0)
(i64.const 0)
)
(i64.const 1)
(i64.mul
Expand All @@ -51,9 +49,10 @@
)
(loop $label$3
(if
(i64.ne
(local.get $0)
(i64.const 0)
(i32.eqz
(i64.eqz
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems i32.eqz i64.eqz could be eliminate at all for CFG operands

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by "CFG" here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant arguments for control operations like if (i64) but I forgot that if should be accept only i32 input argument, so series of eqz may be better than i64.ne + i64.const 0

(local.get $0)
)
)
(block
(local.set $1
Expand Down
16 changes: 16 additions & 0 deletions test/passes/optimize-instructions_enable-threads.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,22 @@
(i32.const 0)
)
)
(drop
(i64.eqz
(i64.const 100)
)
)
(drop
(i64.eq
(i64.const 0)
(i64.const 100)
)
)
(drop
(i64.eqz
(i64.const 0)
)
)
(if
(i32.const 123)
(nop)
Expand Down
18 changes: 18 additions & 0 deletions test/passes/optimize-instructions_enable-threads.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,24 @@
(i32.const 0)
)
)
(drop
(i64.eq
(i64.const 100)
(i64.const 0)
)
)
(drop
(i64.eq
(i64.const 0)
(i64.const 100)
)
)
(drop
(i64.eq
(i64.const 0)
(i64.const 0)
)
)
(if
(i32.eqz
(i32.eqz
Expand Down