Skip to content

Commit 7747683

Browse files
kripkenkripken
committed
Move if copy logic from coalesce-locals to remove-unused-brs.
If copies is the case where an if arm is a get that feeds into a set of the same local: (set_local $x (if (result i32) (..condition..) (..result) (get_local $x) ) ) We can rework this so that the if-else is only an if, which executes the code path not going to the get. This was done in coalesce-locals only because it is likely to work there as after coalescing there are more copies. However, the logic is of removing a branch, and so belongs in remove-unused-brs, and fits alongside existing logic there for handling ifs with an arm that is a br. Also refactor that code so that the two optimizations can feed into each other.
1 parent 6b6e89d commit 7747683

File tree

5 files changed

+460
-86
lines changed

5 files changed

+460
-86
lines changed

‎src/passes/CoalesceLocals.cpp‎

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -315,25 +315,6 @@ void CoalesceLocals::pickIndices(std::vector<Index>& indices) {
315315
}
316316
}
317317

318-
// Remove a copy from a set of an if, where one if arm is a get of the same set
319-
static void removeIfCopy(Expression** origin, SetLocal* set, If* iff, Expression*& copy, Expression*& other, Module* module) {
320-
// replace the origin with the if, and sink the set into the other non-copying arm
321-
bool tee = set->isTee();
322-
*origin = iff;
323-
set->value = other;
324-
set->finalize();
325-
other = set;
326-
// if this is not a tee, then we can get rid of the copy in that arm
327-
if (!tee) {
328-
// we don't need the copy at all
329-
copy = nullptr;
330-
if (!iff->ifTrue) {
331-
Builder(*module).flip(iff);
332-
}
333-
iff->finalize();
334-
}
335-
}
336-
337318
void CoalesceLocals::applyIndices(std::vector<Index>& indices, Expression* root) {
338319
assert(indices.size() == numLocals);
339320
for (auto& curr : basicBlocks) {
@@ -362,20 +343,6 @@ void CoalesceLocals::applyIndices(std::vector<Index>& indices, Expression* root)
362343
}
363344
continue;
364345
}
365-
if (auto* iff = set->value->dynCast<If>()) {
366-
if (auto* get = iff->ifTrue->dynCast<GetLocal>()) {
367-
if (get->index == set->index) {
368-
removeIfCopy(action.origin, set, iff, iff->ifTrue, iff->ifFalse, getModule());
369-
continue;
370-
}
371-
}
372-
if (auto* get = iff->ifFalse->dynCast<GetLocal>()) {
373-
if (get->index == set->index) {
374-
removeIfCopy(action.origin, set, iff, iff->ifFalse, iff->ifTrue, getModule());
375-
continue;
376-
}
377-
}
378-
}
379346
}
380347
}
381348
}

‎src/passes/RemoveUnusedBrs.cpp‎

Lines changed: 128 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -804,53 +804,150 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> {
804804
}
805805

806806
void visitSetLocal(SetLocal* curr) {
807-
// Undo an if return value into a local, if one arm is a br, as we
808-
// prefer a br_if:
809-
// (set_local $x
810-
// (if (result i32)
811-
// (..condition..)
812-
// (br $somewhere)
813-
// (..result)
814-
// )
815-
// )
816-
// =>
817-
// (br_if $somewhere
818-
// (..condition..)
819-
// )
820-
// (set_local $x
821-
// (..result)
822-
// )
823-
// TODO: handle a condition in the br? need to watch for side effects
824-
auto* iff = curr->value->dynCast<If>();
825-
if (!iff) return;
826-
if (!isConcreteType(iff->type) || !isConcreteType(iff->condition->type)) return;
807+
// Sets of an if can be optimized in various ways that remove part of
808+
// the if branching, or all of it.
809+
// The optimizations we can do here can recurse and call each
810+
// other, so pass around a pointer to the output.
811+
optimizeSetIf(getCurrentPointer());
812+
}
813+
814+
void optimizeSetIf(Expression** currp) {
815+
if (optimizeSetIfWithBrArm(currp)) return;
816+
if (optimizeSetIfWithCopyArm(currp)) return;
817+
}
818+
819+
// If one arm is a br, we prefer a br_if and the set later:
820+
// (set_local $x
821+
// (if (result i32)
822+
// (..condition..)
823+
// (br $somewhere)
824+
// (..result)
825+
// )
826+
// )
827+
// =>
828+
// (br_if $somewhere
829+
// (..condition..)
830+
// )
831+
// (set_local $x
832+
// (..result)
833+
// )
834+
// TODO: handle a condition in the br? need to watch for side effects
835+
bool optimizeSetIfWithBrArm(Expression** currp) {
836+
auto* set = (*currp)->cast<SetLocal>();
837+
auto* iff = set->value->dynCast<If>();
838+
if (!iff ||
839+
!isConcreteType(iff->type) ||
840+
!isConcreteType(iff->condition->type)) {
841+
return false;
842+
}
827843
auto tryToOptimize = [&](Expression* one, Expression* two, bool flipCondition) {
828844
if (one->type == unreachable && two->type != unreachable) {
829845
if (auto* br = one->dynCast<Break>()) {
830846
if (ExpressionAnalyzer::isSimple(br)) {
831847
// Wonderful, do it!
832848
Builder builder(*getModule());
833-
br->condition = iff->condition;
834849
if (flipCondition) {
835-
br->condition = builder.makeUnary(EqZInt32, br->condition);
850+
builder.flip(iff);
836851
}
852+
br->condition = iff->condition;
837853
br->finalize();
838-
curr->value = two;
839-
replaceCurrent(
840-
builder.makeSequence(
841-
br,
842-
curr
843-
)
844-
);
854+
set->value = two;
855+
auto* block = builder.makeSequence(br, set);
856+
*currp = block;
857+
// Recurse on the set, which now has a new value.
858+
optimizeSetIf(&block->list[1]);
845859
return true;
846860
}
847861
}
848862
}
849863
return false;
850864
};
851-
if (!tryToOptimize(iff->ifTrue, iff->ifFalse, false)) {
852-
tryToOptimize(iff->ifFalse, iff->ifTrue, true);
865+
return tryToOptimize(iff->ifTrue, iff->ifFalse, false) ||
866+
tryToOptimize(iff->ifFalse, iff->ifTrue, true);
867+
}
868+
869+
// If one arm is a get of the same outer set, it is a copy which
870+
// we can remove. If this is not a tee, then we remove the get
871+
// as well as the if-else opcode in the binary format, which is
872+
// great:
873+
// (set_local $x
874+
// (if (result i32)
875+
// (..condition..)
876+
// (..result)
877+
// (get_local $x)
878+
// )
879+
// )
880+
// =>
881+
// (if
882+
// (..condition..)
883+
// (set_local $x
884+
// (..result)
885+
// )
886+
// )
887+
// If this is a tee, then we can do the same operation but
888+
// inside a block, and keep the get:
889+
// (tee_local $x
890+
// (if (result i32)
891+
// (..condition..)
892+
// (..result)
893+
// (get_local $x)
894+
// )
895+
// )
896+
// =>
897+
// (block (result i32)
898+
// (if
899+
// (..condition..)
900+
// (set_local $x
901+
// (..result)
902+
// )
903+
// )
904+
// (get_local $x)
905+
// )
906+
// We save the if-else opcode, and add the block's opcodes.
907+
// This may be detrimental, however, often the block can be
908+
// merged or eliminated given the outside scope, and we
909+
// removed one of the if branches.
910+
bool optimizeSetIfWithCopyArm(Expression** currp) {
911+
auto* set = (*currp)->cast<SetLocal>();
912+
auto* iff = set->value->dynCast<If>();
913+
if (!iff ||
914+
!isConcreteType(iff->type) ||
915+
!isConcreteType(iff->condition->type)) {
916+
return false;
917+
}
918+
Builder builder(*getModule());
919+
GetLocal* get = iff->ifTrue->dynCast<GetLocal>();
920+
if (get && get->index == set->index) {
921+
builder.flip(iff);
922+
} else {
923+
get = iff->ifFalse->dynCast<GetLocal>();
924+
if (get && get->index != set->index) {
925+
get = nullptr;
926+
}
927+
}
928+
if (!get) return false;
929+
// We can do it!
930+
bool tee = set->isTee();
931+
assert(set->index == get->index);
932+
assert(iff->ifFalse == get);
933+
set->value = iff->ifTrue;
934+
set->finalize();
935+
iff->ifTrue = set;
936+
iff->ifFalse = nullptr;
937+
iff->finalize();
938+
Expression* replacement = iff;
939+
if (tee) {
940+
set->setTee(false);
941+
// We need a block too.
942+
replacement = builder.makeSequence(
943+
iff,
944+
get // reuse the get
945+
);
853946
}
947+
*currp = replacement;
948+
// Recurse on the set, which now has a new value.
949+
optimizeSetIf(&iff->ifTrue);
950+
return true;
854951
}
855952

856953
// (br_if)+ => br_table

‎test/passes/coalesce-locals.txt‎

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1010,11 +1010,10 @@
10101010
(local $0 i32)
10111011
(local $1 i32)
10121012
(loop $top
1013-
(if
1014-
(i32.eqz
1013+
(set_local $0
1014+
(if (result i32)
10151015
(i32.const 1)
1016-
)
1017-
(set_local $0
1016+
(get_local $0)
10181017
(get_local $1)
10191018
)
10201019
)
@@ -1031,10 +1030,11 @@
10311030
(local $0 i32)
10321031
(local $1 i32)
10331032
(loop $top
1034-
(if
1035-
(i32.const 1)
1036-
(set_local $1
1033+
(set_local $1
1034+
(if (result i32)
1035+
(i32.const 1)
10371036
(get_local $0)
1037+
(get_local $1)
10381038
)
10391039
)
10401040
(drop
@@ -1050,10 +1050,11 @@
10501050
(local $0 i32)
10511051
(local $1 i32)
10521052
(loop $top
1053-
(if
1054-
(i32.const 1)
1055-
(tee_local $0
1053+
(set_local $0
1054+
(if (result i32)
1055+
(i32.const 1)
10561056
(unreachable)
1057+
(get_local $0)
10571058
)
10581059
)
10591060
(drop
@@ -1090,10 +1091,10 @@
10901091
(local $1 i32)
10911092
(loop $top
10921093
(drop
1093-
(if (result i32)
1094-
(i32.const 1)
1095-
(get_local $0)
1096-
(tee_local $0
1094+
(tee_local $0
1095+
(if (result i32)
1096+
(i32.const 1)
1097+
(get_local $0)
10971098
(i32.const 2)
10981099
)
10991100
)
@@ -1127,10 +1128,10 @@
11271128
)
11281129
(func $tee_if_with_unreachable_else (; 50 ;) (type $8) (param $0 f64) (param $1 i32) (result i64)
11291130
(call $tee_if_with_unreachable_else
1130-
(if (result f64)
1131-
(get_local $1)
1132-
(get_local $0)
1133-
(tee_local $0
1131+
(tee_local $0
1132+
(if (result f64)
1133+
(get_local $1)
1134+
(get_local $0)
11341135
(unreachable)
11351136
)
11361137
)
@@ -1142,12 +1143,12 @@
11421143
)
11431144
(func $tee_if_with_unreachable_true (; 51 ;) (type $8) (param $0 f64) (param $1 i32) (result i64)
11441145
(call $tee_if_with_unreachable_else
1145-
(if (result f64)
1146-
(get_local $1)
1147-
(tee_local $0
1146+
(tee_local $0
1147+
(if (result f64)
1148+
(get_local $1)
11481149
(unreachable)
1150+
(get_local $0)
11491151
)
1150-
(get_local $0)
11511152
)
11521153
(f64.lt
11531154
(f64.const -128)

0 commit comments

Comments
 (0)