Skip to content

Simplify signed reminders which compared with zero #3153

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 26 commits into from
Sep 30, 2020
Merged

Simplify signed reminders which compared with zero #3153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
31a5276
Simplify signed reminders which compared with zero
MaxGraey Sep 21, 2020
25aca33
add match for unary. More tests
MaxGraey Sep 21, 2020
8654640
more test
MaxGraey Sep 21, 2020
c934b6f
Merge branch 'master' into opt-rem_s-w-cmp-zero
MaxGraey Sep 25, 2020
25c5658
remove expr == 0 matching
MaxGraey Sep 25, 2020
5601d17
refactor
MaxGraey Sep 25, 2020
0adb236
remove redundancy and refactoring
MaxGraey Sep 25, 2020
969b642
Merge branch 'master' into opt-rem_s-w-cmp-zero
MaxGraey Sep 26, 2020
9ba1540
Merge branch 'master' into opt-rem_s-w-cmp-zero
MaxGraey Sep 29, 2020
dd6099a
update fixtures
MaxGraey Sep 29, 2020
d2cee70
move eqz rule on top
MaxGraey Sep 29, 2020
99fe2a6
trigger ci
MaxGraey Sep 29, 2020
12bec6f
Merge branch 'master' into opt-rem_s-w-cmp-zero
MaxGraey Sep 29, 2020
99df9be
update fixtures after sync with master
MaxGraey Sep 29, 2020
a881c38
Update src/passes/OptimizeInstructions.cpp
MaxGraey Sep 29, 2020
2e41728
refactor
MaxGraey Sep 29, 2020
998ee26
more
MaxGraey Sep 29, 2020
cc4bc4e
more (scope)
MaxGraey Sep 29, 2020
f583965
refactor according review
MaxGraey Sep 29, 2020
4a01171
space
MaxGraey Sep 29, 2020
6bb1aa5
revert changes
MaxGraey Sep 29, 2020
1ca1bc6
refactor
MaxGraey Sep 29, 2020
e8cc01b
same for eqz rule
MaxGraey Sep 30, 2020
c9fc299
more
MaxGraey Sep 30, 2020
002906a
use c->type instead left->type which simplify expr != 0 rule
MaxGraey Sep 30, 2020
4bd7810
simplify eqz rule as well
MaxGraey Sep 30, 2020
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
38 changes: 29 additions & 9 deletions src/passes/OptimizeInstructions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,13 +242,6 @@ struct OptimizeInstructions
// block scope can be removed.
using namespace Match;
Builder builder(*getModule());
{
// X == 0 => eqz X
Expression* x;
if (matches(curr, binary(EqInt32, any(&x), i32(0)))) {
return Builder(*getModule()).makeUnary(EqZInt32, x);
}
}
{
// try to get rid of (0 - ..), that is, a zero only used to negate an
// int. an add of a subtract can be flipped in order to remove it:
Expand Down Expand Up @@ -301,6 +294,19 @@ struct OptimizeInstructions
return sub;
}
}
{
// eqz((signed)x % C_pot) => eqz(x & (C_pot - 1))
Const* c;
Binary* inner;
if (matches(curr,
unary(Abstract::EqZ,
binary(&inner, Abstract::RemS, any(), ival(&c)))) &&
IsPowerOf2((uint64_t)c->value.getInteger())) {
Copy link
Member

@kripken kripken Oct 2, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

why is the cast to uint64_t needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IsPowerOf2 allow only unsigned types but getInteger() return int64_t

Copy link
Contributor Author

@MaxGraey MaxGraey Oct 2, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Btw negative dividers like x % -4 == 0 currently rejected and not optimized which is ok but it seems it could be handle as (x & 3) == 0 as well.

Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it work without the cast, then? The function is

template<typename T> bool isPowerOf2(T v) {
  return v != 0 && (v & (v - 1)) == 0;
}

which just cares about bits anyhow. There should be no compiler warning or error here? Maybe I'm not sure what you mean by "allow only".

Copy link
Contributor Author

@MaxGraey MaxGraey Oct 2, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure signed type does make sense for isPowerOf2 due to negative values always return false. But you're right casting to uint64_t here unnecessary.

Copy link
Member

@kripken kripken Oct 2, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Logically, I agree, it's odd to ask about a negative number. But we just have bits here, really. A Literal just stores the bits, without a sign (just like wasm and LLVM IR, etc.).

Copy link
Contributor Author

@MaxGraey MaxGraey Oct 2, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I'll create PR which handle also negative dividers: x % -C_pot == 0 -> (x & (abs(C_pot) - 1)) == 0 and remove this cast as well

inner->op = Abstract::getBinary(c->type, Abstract::And);
c->value = c->value.sub(Literal::makeFromInt32(1, c->type));
return curr;
}
}
{
// try de-morgan's AND law,
// (eqz X) and (eqz Y) === eqz (X or Y)
Expand Down Expand Up @@ -1272,15 +1278,29 @@ struct OptimizeInstructions
return right;
}
// x == 0 ==> eqz x
if ((matches(curr, binary(Abstract::Eq, any(&left), ival(0))))) {
return builder.makeUnary(EqZInt64, left);
if (matches(curr, binary(Abstract::Eq, any(&left), ival(0)))) {
return builder.makeUnary(Abstract::getUnary(type, Abstract::EqZ), left);
}
// Operations on one
// (signed)x % 1 ==> 0
if (matches(curr, binary(Abstract::RemS, pure(&left), ival(1)))) {
right->value = Literal::makeSingleZero(type);
return right;
}
// (signed)x % C_pot != 0 ==> x & (C_pot - 1) != 0
{
Const* c;
Binary* inner;
if (matches(curr,
binary(Abstract::Ne,
binary(&inner, Abstract::RemS, any(), ival(&c)),
ival(0))) &&
IsPowerOf2((uint64_t)c->value.getInteger())) {
inner->op = Abstract::getBinary(c->type, Abstract::And);
c->value = c->value.sub(Literal::makeFromInt32(1, c->type));
return curr;
}
}
// bool(x) | 1 ==> 1
if (matches(curr, binary(Abstract::Or, pure(&left), ival(1))) &&
Bits::getMaxBits(left, this) == 1) {
Expand Down
77 changes: 76 additions & 1 deletion test/passes/optimize-instructions_all-features.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
(type $i32_i32_=>_none (func (param i32 i32)))
(type $i32_=>_i32 (func (param i32) (result i32)))
(type $none_=>_i32 (func (result i32)))
(type $none_=>_none (func))
(type $i32_i64_=>_none (func (param i32 i64)))
(type $none_=>_none (func))
(type $i32_i32_=>_i32 (func (param i32 i32) (result i32)))
(type $i32_=>_none (func (param i32)))
(type $none_=>_i64 (func (result i64)))
Expand Down Expand Up @@ -2744,6 +2744,81 @@
(i64.const 0)
)
)
(func $srem-by-pot-eq-ne-zero (param $x i32) (param $y i64)
(drop
(i32.eqz
(i32.and
(local.get $x)
(i32.const 3)
)
)
)
(drop
(i64.eqz
(i64.and
(local.get $y)
(i64.const 3)
)
)
)
(drop
(i32.eqz
(i32.and
(local.get $x)
(i32.const 3)
)
)
)
(drop
(i64.eqz
(i64.and
(local.get $y)
(i64.const 1)
)
)
)
(drop
(i32.ne
(i32.and
(local.get $x)
(i32.const 1)
)
(i32.const 0)
)
)
(drop
(i32.wrap_i64
(i64.and
(local.get $y)
(i64.const 1)
)
)
)
(drop
(i32.eqz
(i32.rem_s
(local.get $x)
(i32.const 3)
)
)
)
(drop
(i32.eqz
(i32.rem_s
(local.get $x)
(i32.const -4)
)
)
)
(drop
(i64.eqz
(i64.rem_s
(local.get $y)
(i64.const 3)
)
)
)
)
(func $orZero (param $0 i32) (result i32)
(local.get $0)
)
Expand Down
67 changes: 67 additions & 0 deletions test/passes/optimize-instructions_all-features.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3104,6 +3104,73 @@
(i64.const 1)
))
)
(func $srem-by-pot-eq-ne-zero (param $x i32) (param $y i64)
;; eqz((signed)x % 4)
(drop (i32.eqz
(i32.rem_s
(local.get $x)
(i32.const 4)
)
))
(drop (i64.eqz
(i64.rem_s
(local.get $y)
(i64.const 4)
)
))
;; (signed)x % 4 == 0
(drop (i32.eq
(i32.rem_s
(local.get $x)
(i32.const 4)
)
(i32.const 0)
))
(drop (i64.eq
(i64.rem_s
(local.get $y)
(i64.const 2)
)
(i64.const 0)
))
;; ;; (signed)x % 2 != 0
(drop (i32.ne
(i32.rem_s
(local.get $x)
(i32.const 2)
)
(i32.const 0)
))
(drop (i64.ne
(i64.rem_s
(local.get $y)
(i64.const 2)
)
(i64.const 0)
))
;; ;;
(drop (i32.eq
(i32.rem_s
(local.get $x)
(i32.const 3) ;; skip
)
(i32.const 0)
))
(drop (i32.eq
(i32.rem_s
(local.get $x)
(i32.const -4) ;; skip
)
(i32.const 0)
))
(drop (i64.eq
(i64.rem_s
(local.get $y)
(i64.const 3) ;; skip
)
(i64.const 0)
))
)
(func $orZero (param $0 i32) (result i32)
(i32.or
(local.get $0)
Expand Down