Skip to content

Commit fbce98c

Browse files
authored
Remove redundant instructions in Flatten (#2524)
When the expression type is none, it does not seem to be necessary to make it a prelude and insert a nop. This also results in unnecessary blocks that contains an expression with a nop, which can be reduced to just the expression. This also adds some newlines to improve readability.
1 parent 16c6b44 commit fbce98c

17 files changed

+3061
-4803
lines changed

src/passes/Flatten.cpp

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,17 @@ struct Flatten
6161
std::vector<Expression*> ourPreludes;
6262
Builder builder(*getModule());
6363

64+
if (curr->is<Const>() || curr->is<Nop>() || curr->is<Unreachable>()) {
65+
return;
66+
}
67+
6468
if (Flat::isControlFlowStructure(curr)) {
6569
// handle control flow explicitly. our children do not have control flow,
6670
// but they do have preludes which we need to set up in the right place
6771

6872
// no one should have given us preludes, they are on the children
6973
assert(preludes.find(curr) == preludes.end());
74+
7075
if (auto* block = curr->dynCast<Block>()) {
7176
// make a new list, where each item's preludes are added before it
7277
ExpressionList newList(getModule()->allocator);
@@ -106,6 +111,7 @@ struct Flatten
106111
}
107112
// the block now has no return value, and may have become unreachable
108113
block->finalize(none);
114+
109115
} else if (auto* iff = curr->dynCast<If>()) {
110116
// condition preludes go before the entire if
111117
auto* rep = getPreludesWithExpression(iff->condition, iff);
@@ -138,6 +144,7 @@ struct Flatten
138144
ourPreludes.push_back(prelude);
139145
}
140146
replaceCurrent(rep);
147+
141148
} else if (auto* loop = curr->dynCast<Loop>()) {
142149
// remove a loop value
143150
Expression* rep = loop;
@@ -155,15 +162,18 @@ struct Flatten
155162
loop->body = getPreludesWithExpression(originalBody, loop->body);
156163
loop->finalize();
157164
replaceCurrent(rep);
165+
158166
} else {
159167
WASM_UNREACHABLE("unexpected expr type");
160168
}
169+
161170
} else {
162171
// for anything else, there may be existing preludes
163172
auto iter = preludes.find(curr);
164173
if (iter != preludes.end()) {
165174
ourPreludes.swap(iter->second);
166175
}
176+
167177
// special handling
168178
if (auto* set = curr->dynCast<LocalSet>()) {
169179
if (set->isTee()) {
@@ -178,6 +188,7 @@ struct Flatten
178188
replaceCurrent(builder.makeLocalGet(set->index, localType));
179189
}
180190
}
191+
181192
} else if (auto* br = curr->dynCast<Break>()) {
182193
if (br->value) {
183194
auto type = br->value->type;
@@ -203,6 +214,7 @@ struct Flatten
203214
replaceCurrent(br->value);
204215
}
205216
}
217+
206218
} else if (auto* sw = curr->dynCast<Switch>()) {
207219
if (sw->value) {
208220
auto type = sw->value->type;
@@ -227,28 +239,22 @@ struct Flatten
227239
}
228240
}
229241
}
242+
230243
// continue for general handling of everything, control flow or otherwise
231244
curr = getCurrent(); // we may have replaced it
232245
// we have changed children
233246
ReFinalizeNode().visit(curr);
234-
// move everything to the prelude, if we need to: anything but constants
235-
if (!curr->is<Const>()) {
236-
if (curr->type == unreachable) {
237-
ourPreludes.push_back(curr);
238-
replaceCurrent(builder.makeUnreachable());
239-
} else if (curr->type == none) {
240-
if (!curr->is<Nop>()) {
241-
ourPreludes.push_back(curr);
242-
replaceCurrent(builder.makeNop());
243-
}
244-
} else {
245-
// use a local
246-
auto type = curr->type;
247-
Index temp = builder.addVar(getFunction(), type);
248-
ourPreludes.push_back(builder.makeLocalSet(temp, curr));
249-
replaceCurrent(builder.makeLocalGet(temp, type));
250-
}
247+
if (curr->type == unreachable) {
248+
ourPreludes.push_back(curr);
249+
replaceCurrent(builder.makeUnreachable());
250+
} else if (curr->type.isConcrete()) {
251+
// use a local
252+
auto type = curr->type;
253+
Index temp = builder.addVar(getFunction(), type);
254+
ourPreludes.push_back(builder.makeLocalSet(temp, curr));
255+
replaceCurrent(builder.makeLocalGet(temp, type));
251256
}
257+
252258
// next, finish up: migrate our preludes if we can
253259
if (!ourPreludes.empty()) {
254260
auto* parent = getParent();

0 commit comments

Comments
 (0)