Skip to content

Commit a63a322

Browse files
authored
[NFC] Remove TypeUpdating::canHandleAsLocal (#8490)
This method originally handled non-nullable types, which early in the design of Wasm GC could not be stored in locals. Now they can be stored in locals, so this function does nothing more than check if the type is concrete. The property that any type can be stored in locals is important enough that it is almost certain that all future WebAssembly extensions will preserve it, so canHandleAsLocal will probably never have a non-trivial implementation again. Remove this utility and simplify all its former users.
1 parent 5283ba0 commit a63a322

9 files changed

Lines changed: 8 additions & 78 deletions

File tree

src/ir/type-updating.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -408,11 +408,6 @@ Type GlobalTypeRewriter::getTempTupleType(Tuple tuple) {
408408

409409
namespace TypeUpdating {
410410

411-
bool canHandleAsLocal(Type type) {
412-
// TODO: Inline this into its callers.
413-
return type.isConcrete();
414-
}
415-
416411
void handleNonDefaultableLocals(Function* func, Module& wasm) {
417412
if (!wasm.features.hasReferenceTypes()) {
418413
// No references, so no non-nullable ones at all.

src/ir/type-updating.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -556,10 +556,6 @@ class TypeMapper : public GlobalTypeRewriter {
556556

557557
namespace TypeUpdating {
558558

559-
// Checks whether a type is valid as a local, or whether
560-
// handleNonDefaultableLocals() can handle it if not.
561-
bool canHandleAsLocal(Type type);
562-
563559
// Finds non-nullable locals, which are currently not supported, and handles
564560
// them. Atm this turns them into nullable ones, and adds ref.as_non_null on
565561
// their uses (which keeps the type of the users identical).

src/passes/Heap2Local.cpp

Lines changed: 4 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1534,12 +1534,10 @@ struct Heap2Local {
15341534
// constant indexes, so they are effectively structs, and turning them into
15351535
// such allows uniform handling later.
15361536
for (auto* allocation : finder.arrayNews) {
1537-
// The point of this optimization is to replace heap allocations with
1538-
// locals, so we must be able to place the data in locals.
1539-
if (!canHandleAsLocals(allocation->type)) {
1537+
if (allocation->type == Type::unreachable) {
1538+
// Leave this for DCE.
15401539
continue;
15411540
}
1542-
15431541
EscapeAnalyzer analyzer(
15441542
localGraph, parents, branchTargets, passOptions, wasm);
15451543
if (!analyzer.escapes(allocation)) {
@@ -1554,11 +1552,10 @@ struct Heap2Local {
15541552

15551553
// Next, process all structNews.
15561554
for (auto* allocation : finder.structNews) {
1557-
// As above, we must be able to use locals for this data.
1558-
if (!canHandleAsLocals(allocation->type)) {
1555+
if (allocation->type == Type::unreachable) {
1556+
// Leave this for DCE.
15591557
continue;
15601558
}
1561-
15621559
// Check for escaping, noting relevant information as we go. If this does
15631560
// not escape, optimize it into locals.
15641561
EscapeAnalyzer analyzer(
@@ -1576,30 +1573,6 @@ struct Heap2Local {
15761573
EHUtils::handleBlockNestedPops(func, wasm);
15771574
}
15781575
}
1579-
1580-
bool canHandleAsLocal(const Field& field) {
1581-
return TypeUpdating::canHandleAsLocal(field.type);
1582-
}
1583-
1584-
bool canHandleAsLocals(Type type) {
1585-
if (type == Type::unreachable) {
1586-
return false;
1587-
}
1588-
1589-
auto heapType = type.getHeapType();
1590-
if (heapType.isStruct()) {
1591-
auto& fields = heapType.getStruct().fields;
1592-
for (auto field : fields) {
1593-
if (!canHandleAsLocal(field)) {
1594-
return false;
1595-
}
1596-
}
1597-
return true;
1598-
}
1599-
1600-
assert(heapType.isArray());
1601-
return canHandleAsLocal(heapType.getArray().element);
1602-
}
16031576
};
16041577

16051578
struct Heap2LocalPass : public WalkerPass<PostWalker<Heap2LocalPass>> {

src/passes/Inlining.cpp

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -186,17 +186,6 @@ struct FunctionInfo {
186186
}
187187
};
188188

189-
static bool canHandleParams(Function* func) {
190-
// We cannot inline a function if we cannot handle placing its params in a
191-
// locals, as all params become locals.
192-
for (auto param : func->getParams()) {
193-
if (!TypeUpdating::canHandleAsLocal(param)) {
194-
return false;
195-
}
196-
}
197-
return true;
198-
}
199-
200189
using NameInfoMap = std::unordered_map<Name, FunctionInfo>;
201190

202191
struct FunctionInfoScanner
@@ -246,10 +235,6 @@ struct FunctionInfoScanner
246235
void visitFunction(Function* curr) {
247236
auto& info = infos[curr->name];
248237

249-
if (!canHandleParams(curr)) {
250-
info.inliningMode = InliningMode::Uninlineable;
251-
}
252-
253238
info.size = Measurer::measure(curr->body);
254239

255240
// If the body is a simple instruction with roughly the same encoded size as

src/passes/LocalCSE.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -354,8 +354,7 @@ struct Scanner
354354
// them is not cheap, so leave them for later, after we know if there
355355
// actually are any requests for reuse of this value (which is rare).
356356
if (!curr->type.isConcrete() || curr->is<LocalGet>() ||
357-
curr->is<LocalSet>() || Properties::isConstantExpression(curr) ||
358-
!TypeUpdating::canHandleAsLocal(curr->type)) {
357+
curr->is<LocalSet>() || Properties::isConstantExpression(curr)) {
359358
return false;
360359
}
361360

src/passes/OptimizeInstructions.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1432,10 +1432,6 @@ struct OptimizeInstructions
14321432
// The call_ref is not reached; leave this for DCE.
14331433
return;
14341434
}
1435-
if (!TypeUpdating::canHandleAsLocal(lastOperandType)) {
1436-
// We cannot create a local, so we must give up.
1437-
return;
1438-
}
14391435
Index tempLocal = builder.addVar(
14401436
getFunction(),
14411437
TypeUpdating::getValidLocalType(lastOperandType, features));

src/passes/call-utils.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,7 @@ convertToDirectCalls(T* curr,
101101
// execute first, so we'll use locals for them all. First, see if any are
102102
// unreachable, and if so stop trying to optimize and leave this for DCE.
103103
for (auto* operand : operands) {
104-
if (operand->type == Type::unreachable ||
105-
!TypeUpdating::canHandleAsLocal(operand->type)) {
104+
if (operand->type == Type::unreachable) {
106105
return nullptr;
107106
}
108107
}

src/passes/param-utils.cpp

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -131,15 +131,6 @@ RemovalOutcome removeParameter(const std::vector<Function*>& funcs,
131131
}
132132
}
133133

134-
// The type must be valid for us to handle as a local (since we
135-
// replace the parameter with a local).
136-
// TODO: if there are no references at all, we can avoid creating a
137-
// local
138-
bool typeIsValid = TypeUpdating::canHandleAsLocal(first->getLocalType(index));
139-
if (!typeIsValid) {
140-
return Failure;
141-
}
142-
143134
// We can do it!
144135

145136
// Remove the parameter from the function. We must add a new local

src/tools/fuzzing/fuzzing.cpp

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1687,11 +1687,7 @@ Function* TranslateToFuzzReader::addFunction() {
16871687

16881688
Index numVars = upToSquared(fuzzParams->MAX_VARS);
16891689
for (Index i = 0; i < numVars; i++) {
1690-
auto type = getConcreteType();
1691-
if (!TypeUpdating::canHandleAsLocal(type)) {
1692-
type = Type::i32;
1693-
}
1694-
func->vars.push_back(type);
1690+
func->vars.push_back(getConcreteType());
16951691
}
16961692
// Generate the function creation context after we filled in locals, which it
16971693
// will scan.
@@ -3122,7 +3118,7 @@ Expression* TranslateToFuzzReader::makeLocalGet(Type type) {
31223118
// the time), or emit a local.get of a new local, or emit a local.tee of a new
31233119
// local.
31243120
auto choice = upTo(3);
3125-
if (choice == 0 || !TypeUpdating::canHandleAsLocal(type)) {
3121+
if (choice == 0) {
31263122
return makeConst(type);
31273123
}
31283124
// Otherwise, add a new local. If the type is not non-nullable then we may

0 commit comments

Comments
 (0)