Skip to content

Commit 7bd37d4

Browse files
kripkenkripken
authored
[NFC] Refactor ChildLocalizer to handle unreachable code better (#6394)
This is NFC in the current users, but is necessary functionality for a later PR. ChildLocalizer moves children into locals as needed. It used to stop when it saw the first unreachable. After this change we move such unreachable children out of the parent as well, making this more uniform: all interacting effects are moved out, and all that is left nested in the parent can be moved around and removed as desired. Also add a getReplacement helper that makes using this easier. This cannot be tested comprehensively with the current user as that user will not call this code path on an unreachable parent at all, so this just adds what can be tested. The later PR will have tests for all corner cases.
1 parent 08e47de commit 7bd37d4

3 files changed

Lines changed: 133 additions & 24 deletions

File tree

‎src/ir/localize.h‎

Lines changed: 85 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -46,28 +46,50 @@ struct Localizer {
4646

4747
// Replaces all children with gets of locals, if they have any effects that
4848
// interact with any of the others, or if they have side effects which cannot be
49-
// removed.
49+
// removed. Also replace unreachable things with an unreachable, leaving in
50+
// place only things without interacting effects. For example:
5051
//
51-
// After this, the original input has only local.gets as inputs, or other things
52-
// that have no interacting effects, and so those children can be reordered
53-
// and/or removed as needed.
52+
// (parent
53+
// (call $foo)
54+
// (br $out)
55+
// (i32.const)
56+
// )
5457
//
55-
// The sets of the locals are emitted on a |sets| property on the class. Those
56-
// must be emitted right before the input.
58+
// =>
5759
//
58-
// This stops at the first unreachable child, as there is no code executing
59-
// after that point anyhow.
60+
// (local.set $temp.foo
61+
// (call $foo) ;; moved out
62+
// )
63+
// (br $out) ;; moved out
64+
// (parent
65+
// (local.get $temp.foo) ;; value saved to a local
66+
// (unreachable) ;; complex effect replaced by unreachable
67+
// (i32.const)
68+
// )
69+
//
70+
// After this it is safe to reorder and remove things from the parent: all
71+
// interesting interactions happen before the parent.
72+
//
73+
// Typical usage is to call getReplacement() will produces the entire output
74+
// just shown (i.e., possible initial local.sets and other stuff that was pulled
75+
// out, followed by the parent, as relevant). Note that getReplacement() may
76+
// omit the parent, if it had an unreachable child. That is useful behavior in
77+
// that it removes unneeded code (& otherwise some users of this code would need
78+
// to write their own removal logic). However, that does imply that it is valid
79+
// to remove the parent in such cases, which is not so for e.g. br when it is
80+
// the last thing keeping a block reachable. Calling this with something like a
81+
// struct.new or a call (the current intended users) is valid; if we want to
82+
// generalize this fully then we need to make changes here.
6083
//
6184
// TODO: use in more places
6285
struct ChildLocalizer {
63-
std::vector<LocalSet*> sets;
64-
65-
ChildLocalizer(Expression* input,
86+
ChildLocalizer(Expression* parent,
6687
Function* func,
67-
Module* wasm,
68-
const PassOptions& options) {
69-
Builder builder(*wasm);
70-
ChildIterator iterator(input);
88+
Module& wasm,
89+
const PassOptions& options)
90+
: parent(parent), wasm(wasm) {
91+
Builder builder(wasm);
92+
ChildIterator iterator(parent);
7193
auto& children = iterator.children;
7294
auto num = children.size();
7395

@@ -77,15 +99,31 @@ struct ChildLocalizer {
7799
// The children are in reverse order in ChildIterator, but we want to
78100
// process them in the normal order.
79101
auto* child = *children[num - 1 - i];
80-
effects.emplace_back(options, *wasm, child);
102+
effects.emplace_back(options, wasm, child);
81103
}
82104

83105
// Go through the children and move to locals those that we need to.
84106
for (Index i = 0; i < num; i++) {
85107
auto** childp = children[num - 1 - i];
86108
auto* child = *childp;
87109
if (child->type == Type::unreachable) {
88-
break;
110+
// Move the child out, and put an unreachable in its place (note that we
111+
// don't need an actual set here, as there is no value to set to a
112+
// local).
113+
sets.push_back(child);
114+
*childp = builder.makeUnreachable();
115+
hasUnreachableChild = true;
116+
continue;
117+
}
118+
119+
if (hasUnreachableChild) {
120+
// Once we pass one unreachable, we only need to copy the children over.
121+
// (The only reason we still need them is that they may be needed for
122+
// validation, e.g. if one contains a break to a block that is the only
123+
// reason the block has type none.)
124+
sets.push_back(builder.makeDrop(child));
125+
*childp = builder.makeUnreachable();
126+
continue;
89127
}
90128

91129
// Use a local if we need to. That is the case either if this has side
@@ -106,6 +144,36 @@ struct ChildLocalizer {
106144
}
107145
}
108146
}
147+
148+
// Helper that gets a replacement for the parent: a block containing the
149+
// sets + the parent. This will not contain the parent if we don't need it
150+
// (if it was never reached).
151+
Expression* getReplacement() {
152+
if (sets.empty()) {
153+
// Nothing to add.
154+
return parent;
155+
}
156+
157+
auto* block = Builder(wasm).makeBlock();
158+
block->list.set(sets);
159+
if (hasUnreachableChild) {
160+
// If there is an unreachable child then we do not need the parent at all,
161+
// and we know the type is unreachable.
162+
block->type = Type::unreachable;
163+
} else {
164+
// Otherwise, add the parent and finalize.
165+
block->list.push_back(parent);
166+
block->finalize();
167+
}
168+
return block;
169+
}
170+
171+
private:
172+
Expression* parent;
173+
Module& wasm;
174+
175+
std::vector<Expression*> sets;
176+
bool hasUnreachableChild = false;
109177
};
110178

111179
} // namespace wasm

‎src/passes/GlobalTypeOptimization.cpp‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -365,13 +365,8 @@ struct GlobalTypeOptimization : public Pass {
365365
if (!func) {
366366
Fatal() << "TODO: side effects in removed fields in globals\n";
367367
}
368-
auto* block = Builder(*getModule()).makeBlock();
369-
auto sets =
370-
ChildLocalizer(curr, func, getModule(), getPassOptions()).sets;
371-
block->list.set(sets);
372-
block->list.push_back(curr);
373-
block->finalize(curr->type);
374-
replaceCurrent(block);
368+
ChildLocalizer localizer(curr, func, *getModule(), getPassOptions());
369+
replaceCurrent(localizer.getReplacement());
375370
}
376371

377372
// Remove the unneeded operands.

‎test/lit/passes/gto-removals.wast‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -925,3 +925,49 @@
925925
(unreachable)
926926
)
927927
)
928+
929+
(module
930+
;; CHECK: (rec
931+
;; CHECK-NEXT: (type $struct (sub (struct )))
932+
(type $struct (sub (struct (field anyref) (field i32) (field f32) (field f64))))
933+
934+
;; CHECK: (type $1 (func (result (ref $struct))))
935+
936+
;; CHECK: (func $func (type $1) (result (ref $struct))
937+
;; CHECK-NEXT: (local $0 (ref $struct))
938+
;; CHECK-NEXT: (local $1 f64)
939+
;; CHECK-NEXT: (local.set $0
940+
;; CHECK-NEXT: (call $func)
941+
;; CHECK-NEXT: )
942+
;; CHECK-NEXT: (local.set $1
943+
;; CHECK-NEXT: (block (result f64)
944+
;; CHECK-NEXT: (if
945+
;; CHECK-NEXT: (i32.const 0)
946+
;; CHECK-NEXT: (then
947+
;; CHECK-NEXT: (unreachable)
948+
;; CHECK-NEXT: )
949+
;; CHECK-NEXT: )
950+
;; CHECK-NEXT: (f64.const 30)
951+
;; CHECK-NEXT: )
952+
;; CHECK-NEXT: )
953+
;; CHECK-NEXT: (struct.new_default $struct)
954+
;; CHECK-NEXT: )
955+
(func $func (result (ref $struct))
956+
;; The fields can be removed here, but the effects must be preserved before
957+
;; the struct.new. The consts in the middle can vanish entirely.
958+
(struct.new $struct
959+
(call $func)
960+
(i32.const 10)
961+
(f32.const 20)
962+
(block (result f64)
963+
(if
964+
(i32.const 0)
965+
(then
966+
(unreachable)
967+
)
968+
)
969+
(f64.const 30)
970+
)
971+
)
972+
)
973+
)

0 commit comments

Comments
 (0)