diff options
77 files changed, 1591 insertions, 447 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e005f6e8..04f4525ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ full changeset diff at the end of each section. Current Trunk ------------- +- Add support for non-nullable locals in wasm GC. - Add a new flag to Directize, `--pass-arg=directize-initial-contents-immutable` which indicates the initial table contents are immutable. That is the case for LLVM, for example, and it allows us to optimize more indirect calls to direct @@ -117,6 +117,25 @@ There are a few differences between Binaryen IR and the WebAssembly language: `(elem declare func $..)`. Binaryen will emit that data when necessary, but it does not represent it in IR. That is, IR can be worked on without needing to think about declaring function references. + * Binaryen IR allows non-nullable locals in the form that the wasm spec does, + (which was historically nicknamed "1a"), in which a `local.get` must be + structurally dominated by a `local.set` in order to validate (that ensures + we do not read the default value of null). Despite being aligned with the + wasm spec, there are some minor details that you may notice: + * A nameless `Block` in Binaryen IR does not interfere with validation. + Nameless blocks are never emitted into the binary format (we just emit + their contents), so we ignore them for purposes of non-nullable locals. As + a result, if you read wasm text emitted by Binaryen then you may see what + seems to be code that should not validate per the spec (and may not + validate in wasm text parsers), but that difference will not exist in the + binary format (binaries emitted by Binaryen will always work everywhere, + aside for bugs of course). + * The Binaryen pass runner will automatically fix up validation after each + pass (finding things that do not validate and fixing them up, usually by + demoting a local to be nullable). As a result you do not need to worry + much about this when writing Binaryen passes. For more details see the + `requiresNonNullableLocalFixups()` hook in `pass.h` and the + `LocalStructuralDominance` class. As a result, you might notice that round-trip conversions (wasm => Binaryen IR => wasm) change code a little in some corner cases. diff --git a/scripts/test/shared.py b/scripts/test/shared.py index 1764f56ba..2c18a053c 100644 --- a/scripts/test/shared.py +++ b/scripts/test/shared.py @@ -262,7 +262,8 @@ V8_OPTS = [ '--experimental-wasm-gc', '--experimental-wasm-typed-funcref', '--experimental-wasm-memory64', - '--experimental-wasm-extended-const' + '--experimental-wasm-extended-const', + '--experimental-wasm-nn-locals', ] # external tools diff --git a/src/ir/CMakeLists.txt b/src/ir/CMakeLists.txt index e3315565b..2dadb743f 100644 --- a/src/ir/CMakeLists.txt +++ b/src/ir/CMakeLists.txt @@ -12,6 +12,7 @@ set(ir_SOURCES possible-contents.cpp properties.cpp LocalGraph.cpp + LocalStructuralDominance.cpp ReFinalize.cpp stack-utils.cpp table-utils.cpp diff --git a/src/ir/LocalStructuralDominance.cpp b/src/ir/LocalStructuralDominance.cpp new file mode 100644 index 000000000..cfb75e006 --- /dev/null +++ b/src/ir/LocalStructuralDominance.cpp @@ -0,0 +1,230 @@ +/* + * Copyright 2022 WebAssembly Community Group participants + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "ir/iteration.h" +#include "ir/local-structural-dominance.h" +#include "support/small_vector.h" + +namespace wasm { + +LocalStructuralDominance::LocalStructuralDominance(Function* func, + Module& wasm, + Mode mode) { + if (!wasm.features.hasReferenceTypes()) { + // No references, so nothing to look at. + return; + } + + bool hasRefVar = false; + for (auto var : func->vars) { + if (var.isRef()) { + hasRefVar = true; + break; + } + } + if (!hasRefVar) { + return; + } + + if (mode == NonNullableOnly) { + bool hasNonNullableVar = false; + for (auto var : func->vars) { + // Check if we have any non-nullable vars at all. + if (var.isNonNullable()) { + hasNonNullableVar = true; + break; + } + } + if (!hasNonNullableVar) { + return; + } + } + + struct Scanner : public PostWalker<Scanner> { + std::set<Index>& nonDominatingIndices; + + // The locals that have been set, and so at the current time, they + // structurally dominate. + std::vector<bool> localsSet; + + Scanner(Function* func, Mode mode, std::set<Index>& nonDominatingIndices) + : nonDominatingIndices(nonDominatingIndices) { + localsSet.resize(func->getNumLocals()); + + // Parameters always dominate. + for (Index i = 0; i < func->getNumParams(); i++) { + localsSet[i] = true; + } + + for (Index i = func->getNumParams(); i < func->getNumLocals(); i++) { + auto type = func->getLocalType(i); + // Mark locals we don't need to care about as "set". We never do any + // work for such a local. + if (!type.isRef() || (mode == NonNullableOnly && type.isNullable())) { + localsSet[i] = true; + } + } + + // Note that we do not need to start a scope for the function body. + // Logically there is a scope there, but there is no code after it, so + // there is nothing to clean up when that scope exits, so we may as well + // not even create a scope. Just start walking the body now. + walk(func->body); + } + + using Locals = SmallVector<Index, 5>; + + // When we exit a control flow scope, we must undo the locals that it set. + std::vector<Locals> cleanupStack; + + static void doBeginScope(Scanner* self, Expression** currp) { + self->cleanupStack.emplace_back(); + } + + static void doEndScope(Scanner* self, Expression** currp) { + for (auto index : self->cleanupStack.back()) { + assert(self->localsSet[index]); + self->localsSet[index] = false; + } + self->cleanupStack.pop_back(); + } + + static void doLocalSet(Scanner* self, Expression** currp) { + auto index = (*currp)->cast<LocalSet>()->index; + if (!self->localsSet[index]) { + // This local is now set until the end of this scope. + self->localsSet[index] = true; + // If we are not in the topmost scope, note this for later cleanup. + if (!self->cleanupStack.empty()) { + self->cleanupStack.back().push_back(index); + } + } + } + + static void scan(Scanner* self, Expression** currp) { + // Use a loop to avoid recursing on the last child - we can just go + // straight into a loop iteration for it. + while (1) { + Expression* curr = *currp; + + switch (curr->_id) { + case Expression::Id::InvalidId: + WASM_UNREACHABLE("bad id"); + + // local.get can just be visited immediately, as it has no children. + case Expression::Id::LocalGetId: { + auto index = curr->cast<LocalGet>()->index; + if (!self->localsSet[index]) { + self->nonDominatingIndices.insert(index); + } + return; + } + case Expression::Id::LocalSetId: { + auto* set = curr->cast<LocalSet>(); + if (!self->localsSet[set->index]) { + self->pushTask(doLocalSet, currp); + } + // Immediately continue in the loop. + currp = &set->value; + continue; + } + + // Control flow structures. + case Expression::Id::BlockId: { + auto* block = curr->cast<Block>(); + // Blocks with no name are never emitted in the binary format, so do + // not create a scope for them. + if (block->name.is()) { + self->pushTask(Scanner::doEndScope, currp); + } + auto& list = block->list; + for (int i = int(list.size()) - 1; i >= 0; i--) { + self->pushTask(Scanner::scan, &list[i]); + } + if (block->name.is()) { + // Just call the task immediately. + doBeginScope(self, currp); + } + return; + } + case Expression::Id::IfId: { + if (curr->cast<If>()->ifFalse) { + self->pushTask(Scanner::doEndScope, currp); + self->maybePushTask(Scanner::scan, &curr->cast<If>()->ifFalse); + self->pushTask(Scanner::doBeginScope, currp); + } + self->pushTask(Scanner::doEndScope, currp); + self->pushTask(Scanner::scan, &curr->cast<If>()->ifTrue); + self->pushTask(Scanner::doBeginScope, currp); + // Immediately continue in the loop. + currp = &curr->cast<If>()->condition; + continue; + } + case Expression::Id::LoopId: { + self->pushTask(Scanner::doEndScope, currp); + // Just call the task immediately. + doBeginScope(self, currp); + // Immediately continue in the loop. + currp = &curr->cast<Loop>()->body; + continue; + } + case Expression::Id::TryId: { + auto& list = curr->cast<Try>()->catchBodies; + for (int i = int(list.size()) - 1; i >= 0; i--) { + self->pushTask(Scanner::doEndScope, currp); + self->pushTask(Scanner::scan, &list[i]); + self->pushTask(Scanner::doBeginScope, currp); + } + self->pushTask(Scanner::doEndScope, currp); + // Just call the task immediately. + doBeginScope(self, currp); + // Immediately continue in the loop. + currp = &curr->cast<Try>()->body; + continue; + } + + default: { + // Control flow structures have been handled. This is an expression, + // which we scan normally. + assert(!Properties::isControlFlowStructure(curr)); + PostWalker<Scanner>::scan(self, currp); + return; + } + } + } + } + + // Only local.set needs to be visited. + void pushTask(TaskFunc func, Expression** currp) { + // Visits to anything but a set can be ignored, so only very specific + // tasks need to actually be pushed here. In particular, we don't want to + // push tasks to call doVisit* when those callbacks do nothing. + if (func == scan || func == doLocalSet || func == doBeginScope || + func == doEndScope) { + PostWalker<Scanner>::pushTask(func, currp); + } + } + void maybePushTask(TaskFunc func, Expression** currp) { + if (*currp) { + pushTask(func, currp); + } + } + }; + + Scanner(func, mode, nonDominatingIndices); +} + +} // namespace wasm diff --git a/src/ir/eh-utils.cpp b/src/ir/eh-utils.cpp index fe33b09c2..f9cd2d940 100644 --- a/src/ir/eh-utils.cpp +++ b/src/ir/eh-utils.cpp @@ -17,7 +17,6 @@ #include "ir/eh-utils.h" #include "ir/branch-utils.h" #include "ir/find_all.h" -#include "ir/type-updating.h" namespace wasm { @@ -157,9 +156,6 @@ void handleBlockNestedPops(Function* func, Module& wasm) { for (auto* try_ : trys.list) { handleBlockNestedPop(try_, func, wasm); } - // Pops we handled can be of non-defaultable types, so we may have created - // non-nullable type locals. Fix them. - TypeUpdating::handleNonDefaultableLocals(func, wasm); } Pop* findPop(Expression* expr) { diff --git a/src/ir/iteration.h b/src/ir/iteration.h index e897c51a5..a2c9ceef6 100644 --- a/src/ir/iteration.h +++ b/src/ir/iteration.h @@ -76,6 +76,7 @@ template<class Specific> class AbstractChildIterator { public: // The vector of children in the order emitted by wasm-delegations-fields // (which is in reverse execution order). + // TODO: rename this "reverseChildren"? SmallVector<Expression**, 4> children; AbstractChildIterator(Expression* parent) { diff --git a/src/ir/linear-execution.h b/src/ir/linear-execution.h index 99b3a2a22..c88d0e444 100644 --- a/src/ir/linear-execution.h +++ b/src/ir/linear-execution.h @@ -49,7 +49,7 @@ struct LinearExecutionWalker : public PostWalker<SubType, VisitorType> { switch (curr->_id) { case Expression::Id::InvalidId: - abort(); + WASM_UNREACHABLE("bad id"); case Expression::Id::BlockId: { self->pushTask(SubType::doVisitBlock, currp); if (curr->cast<Block>()->name.is()) { diff --git a/src/ir/local-structural-dominance.h b/src/ir/local-structural-dominance.h new file mode 100644 index 000000000..e0cc2f329 --- /dev/null +++ b/src/ir/local-structural-dominance.h @@ -0,0 +1,94 @@ +/* + * Copyright 2022 WebAssembly Community Group participants + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef wasm_ir_local_structural_dominance_h +#define wasm_ir_local_structural_dominance_h + +#include "wasm.h" + +namespace wasm { + +// +// This class is useful for the handling of non-nullable locals that is in the +// wasm spec: a local.get validates if it is structurally dominated by a set. +// That dominance proves the get cannot access the default null value, and, +// nicely, it can be validated in linear time. (Historically, this was called +// "1a" during spec discussions.) +// +// Concretely, this class finds which local indexes lack the structural +// dominance property. It only looks at reference types and not anything else. +// It can look at both nullable and non-nullable references, though, as it can +// be used to validate non-nullable ones, and also to check if a nullable one +// could become non-nullable and still validate. +// +// The property of "structural dominance" means that the set dominates the gets +// using wasm's structured control flow constructs, like this: +// +// (block $A +// (local.set $x ..) +// (local.get $x) ;; use in the same scope. +// (block $B +// (local.get $x) ;; use in an inner scope. +// ) +// ) +// +// That set structurally dominates both of those gets. However, in this example +// it does not: +// +// (block $A +// (local.set $x ..) +// ) +// (local.get $x) ;; use in an outer scope. +// +// This is a little similar to breaks: (br $foo) can only go to a label $foo +// that is in scope. +// +// Note that this property must hold on the final wasm binary, while we are +// checking it on Binaryen IR. The property will carry over, however: when +// lowering to wasm we may remove some Block nodes, but removing nodes cannot +// break validation. +// +// In fact, since Blocks without names are not emitted in the binary format (we +// never need them, since nothing can branch to them, so we just emit their +// contents), we can ignore them here. That means that this will validate, which +// is identical to the last example but the block has no name: +// +// (block ;; no name here +// (local.set $x ..) +// ) +// (local.get $x) +// +// It is useful to ignore such blocks here, as various passes emit them +// temporarily. +// +struct LocalStructuralDominance { + // We always look at non-nullable locals, but can be configured to ignore + // or process nullable ones. + enum Mode { + All, + NonNullableOnly, + }; + + LocalStructuralDominance(Function* func, Module& wasm, Mode mode = All); + + // Local indexes for whom a local.get exists that is not structurally + // dominated. + std::set<Index> nonDominatingIndices; +}; + +} // namespace wasm + +#endif // wasm_ir_local_structural_dominance_h diff --git a/src/ir/type-updating.cpp b/src/ir/type-updating.cpp index 5452613bb..6d5848c83 100644 --- a/src/ir/type-updating.cpp +++ b/src/ir/type-updating.cpp @@ -16,6 +16,7 @@ #include "type-updating.h" #include "find_all.h" +#include "ir/local-structural-dominance.h" #include "ir/module-utils.h" #include "ir/utils.h" #include "wasm-type.h" @@ -268,8 +269,12 @@ bool canHandleAsLocal(Type type) { } void handleNonDefaultableLocals(Function* func, Module& wasm) { - // Check if this is an issue. if (wasm.features.hasGCNNLocals()) { + // We have nothing to fix up: all locals are allowed. + return; + } + if (!wasm.features.hasReferenceTypes()) { + // No references, so no non-nullable ones at all. return; } bool hasNonNullable = false; @@ -280,6 +285,26 @@ void handleNonDefaultableLocals(Function* func, Module& wasm) { } } if (!hasNonNullable) { + // No non-nullable types exist in practice. + return; + } + + // Non-nullable locals exist, which we may need to fix up. See if they + // validate as they are, that is, if they fall within the validation rules of + // the wasm spec. We do not need to modify such locals. + LocalStructuralDominance info( + func, wasm, LocalStructuralDominance::NonNullableOnly); + std::unordered_set<Index> badIndexes; + for (auto index : info.nonDominatingIndices) { + badIndexes.insert(index); + + // LocalStructuralDominance should have only looked at non-nullable indexes + // since we told it to ignore nullable ones. Also, params always dominate + // and should not appear here. + assert(func->getLocalType(index).isNonNullable()); + assert(!func->isParam(index)); + } + if (badIndexes.empty()) { return; } @@ -287,11 +312,9 @@ void handleNonDefaultableLocals(Function* func, Module& wasm) { Builder builder(wasm); for (auto** getp : FindAllPointers<LocalGet>(func->body).list) { auto* get = (*getp)->cast<LocalGet>(); - if (!func->isVar(get->index)) { - // We do not need to process params, which can legally be non-nullable. - continue; + if (badIndexes.count(get->index)) { + *getp = fixLocalGet(get, wasm); } - *getp = fixLocalGet(get, wasm); } // Update tees, whose type must match the local (if the wasm spec changes for @@ -307,8 +330,8 @@ void handleNonDefaultableLocals(Function* func, Module& wasm) { if (!set->isTee() || set->type == Type::unreachable) { continue; } - auto type = func->getLocalType(set->index); - if (type.isNonNullable()) { + if (badIndexes.count(set->index)) { + auto type = func->getLocalType(set->index); set->type = Type(type.getHeapType(), Nullable); *setp = builder.makeRefAs(RefAsNonNull, set); } @@ -316,12 +339,14 @@ void handleNonDefaultableLocals(Function* func, Module& wasm) { // Rewrite the types of the function's vars (which we can do now, after we // are done using them to know which local.gets etc to fix). - for (auto& type : func->vars) { - type = getValidLocalType(type, wasm.features); + for (auto index : badIndexes) { + func->vars[index - func->getNumParams()] = + getValidLocalType(func->getLocalType(index), wasm.features); } } Type getValidLocalType(Type type, FeatureSet features) { + // TODO: this should handle tuples with a non-nullable item assert(canHandleAsLocal(type)); if (type.isNonNullable() && !features.hasGCNNLocals()) { type = Type(type.getHeapType(), Nullable); diff --git a/src/ir/utils.h b/src/ir/utils.h index cad4a78aa..b7cd65e19 100644 --- a/src/ir/utils.h +++ b/src/ir/utils.h @@ -119,6 +119,10 @@ struct ReFinalize : public WalkerPass<PostWalker<ReFinalize, OverriddenVisitor<ReFinalize>>> { bool isFunctionParallel() override { return true; } + // Re-running finalize() does not change the types of locals, so validation is + // preserved. + bool requiresNonNullableLocalFixups() override { return false; } + Pass* create() override { return new ReFinalize; } ReFinalize() { name = "refinalize"; } diff --git a/src/pass.h b/src/pass.h index 924d608b9..6769c91ee 100644 --- a/src/pass.h +++ b/src/pass.h @@ -386,6 +386,15 @@ public: // for. This is used to issue a proper warning about that. virtual bool invalidatesDWARF() { return false; } + // Whether this pass modifies Binaryen IR in ways that may require fixups for + // non-nullable locals to validate according to the wasm spec. If the pass + // adds locals not in that form, or moves code around in ways that might break + // that validation, this must return true. In that case the pass runner will + // automatically run the necessary fixups afterwards. + // + // For more details see the LocalStructuralDominance class. + virtual bool requiresNonNullableLocalFixups() { return true; } + std::string name; protected: diff --git a/src/passes/CoalesceLocals.cpp b/src/passes/CoalesceLocals.cpp index ba9b87527..edf142039 100644 --- a/src/passes/CoalesceLocals.cpp +++ b/src/passes/CoalesceLocals.cpp @@ -526,7 +526,25 @@ void CoalesceLocals::applyIndices(std::vector<Index>& indices, continue; } } - // remove ineffective actions + + // Remove ineffective actions, that is, dead stores. + // + // Note that this may have downsides for non-nullable locals: + // + // x = whatever; // dead set for validation + // if (..) { + // x = value1; + // } else { + // x = value2; + // } + // + // The dead set ensures validation, at the cost of extra code size and + // slower speed in some tiers (the optimizing tier, at least, will + // remove such dead sets anyhow). In theory keeping such a dead set may + // be worthwhile, as it may save code size (by keeping the local + // non-nullable and avoiding ref.as_non_nulls later). But the tradeoff + // here isn't clear, so do the simple thing for now and remove all dead + // sets. if (!action.effective) { // value may have no side effects, further optimizations can eliminate // it diff --git a/src/passes/CodePushing.cpp b/src/passes/CodePushing.cpp index 29a3ae743..682f025c8 100644 --- a/src/passes/CodePushing.cpp +++ b/src/passes/CodePushing.cpp @@ -234,6 +234,11 @@ private: struct CodePushing : public WalkerPass<PostWalker<CodePushing>> { bool isFunctionParallel() override { return true; } + // This pass moves code forward in blocks, but a local.set would not be moved + // after a local.get with the same index (effects prevent breaking things that + // way), so validation will be preserved. + bool requiresNonNullableLocalFixups() override { return false; } + Pass* create() override { return new CodePushing; } LocalAnalyzer analyzer; diff --git a/src/passes/ConstantFieldPropagation.cpp b/src/passes/ConstantFieldPropagation.cpp index d2b08ea30..0ec889e36 100644 --- a/src/passes/ConstantFieldPropagation.cpp +++ b/src/passes/ConstantFieldPropagation.cpp @@ -52,6 +52,9 @@ using PCVFunctionStructValuesMap = struct FunctionOptimizer : public WalkerPass<PostWalker<FunctionOptimizer>> { bool isFunctionParallel() override { return true; } + // Only modifies struct.get operations. + bool requiresNonNullableLocalFixups() override { return false; } + Pass* create() override { return new FunctionOptimizer(infos); } FunctionOptimizer(PCVStructValuesMap& infos) : infos(infos) {} @@ -175,6 +178,9 @@ struct PCVScanner }; struct ConstantFieldPropagation : public Pass { + // Only modifies struct.get operations. + bool requiresNonNullableLocalFixups() override { return false; } + void run(PassRunner* runner, Module* module) override { if (!module->features.hasGC()) { return; diff --git a/src/passes/DeadCodeElimination.cpp b/src/passes/DeadCodeElimination.cpp index c30da5428..caafb755d 100644 --- a/src/passes/DeadCodeElimination.cpp +++ b/src/passes/DeadCodeElimination.cpp @@ -44,6 +44,10 @@ struct DeadCodeElimination UnifiedExpressionVisitor<DeadCodeElimination>>> { bool isFunctionParallel() override { return true; } + // This pass removes dead code, which can only help validation (a dead + // local.get might have prevented validation). + bool requiresNonNullableLocalFixups() override { return false; } + Pass* create() override { return new DeadCodeElimination; } // as we remove code, we must keep the types of other nodes valid diff --git a/src/passes/Directize.cpp b/src/passes/Directize.cpp index ba5280d65..af020aca6 100644 --- a/src/passes/Directize.cpp +++ b/src/passes/Directize.cpp @@ -34,7 +34,6 @@ #include "call-utils.h" #include "ir/table-utils.h" -#include "ir/type-updating.h" #include "ir/utils.h" #include "pass.h" #include "wasm-builder.h" @@ -107,7 +106,6 @@ struct FunctionDirectizer : public WalkerPass<PostWalker<FunctionDirectizer>> { WalkerPass<PostWalker<FunctionDirectizer>>::doWalkFunction(func); if (changedTypes) { ReFinalize().walkFunctionInModule(func, getModule()); - TypeUpdating::handleNonDefaultableLocals(func, *getModule()); } } diff --git a/src/passes/DuplicateFunctionElimination.cpp b/src/passes/DuplicateFunctionElimination.cpp index 2e8c83cdd..2ebdca4ba 100644 --- a/src/passes/DuplicateFunctionElimination.cpp +++ b/src/passes/DuplicateFunctionElimination.cpp @@ -34,6 +34,9 @@ struct DuplicateFunctionElimination : public Pass { // FIXME Merge DWARF info bool invalidatesDWARF() override { return true; } + // This pass merges functions but does not alter their contents. + bool requiresNonNullableLocalFixups() override { return false; } + void run(PassRunner* runner, Module* module) override { // Multiple iterations may be necessary: A and B may be identical only after // we see the functions C1 and C2 that they call are in fact identical. diff --git a/src/passes/DuplicateImportElimination.cpp b/src/passes/DuplicateImportElimination.cpp index faf2bb776..1a0319e3a 100644 --- a/src/passes/DuplicateImportElimination.cpp +++ b/src/passes/DuplicateImportElimination.cpp @@ -28,6 +28,9 @@ namespace wasm { struct DuplicateImportElimination : public Pass { + // This pass does not alter function contents. + bool requiresNonNullableLocalFixups() override { return false; } + void run(PassRunner* runner, Module* module) override { ImportInfo imports(*module); std::map<Name, Name> replacements; diff --git a/src/passes/Flatten.cpp b/src/passes/Flatten.cpp index be43e57f2..771183d2c 100644 --- a/src/passes/Flatten.cpp +++ b/src/passes/Flatten.cpp @@ -44,7 +44,6 @@ #include <ir/eh-utils.h> #include <ir/flat.h> #include <ir/properties.h> -#include <ir/type-updating.h> #include <ir/utils.h> #include <pass.h> #include <wasm-builder.h> @@ -368,17 +367,6 @@ struct Flatten } // the body may have preludes curr->body = getPreludesWithExpression(originalBody, curr->body); - // New locals we added may be non-nullable. - TypeUpdating::handleNonDefaultableLocals(curr, *getModule()); - // We cannot handle non-nullable tuples currently, see the comment at the - // top of the file. - for (auto type : curr->vars) { - if (!type.isDefaultable()) { - Fatal() << "Flatten was forced to add a local of a type it cannot " - "handle yet: " - << type; - } - } // Flatten can generate blocks within 'catch', making pops invalid. Fix them // up. diff --git a/src/passes/GlobalRefining.cpp b/src/passes/GlobalRefining.cpp index 6f2f19842..626fbaeb5 100644 --- a/src/passes/GlobalRefining.cpp +++ b/src/passes/GlobalRefining.cpp @@ -31,6 +31,9 @@ namespace wasm { namespace { struct GlobalRefining : public Pass { + // Only modifies globals and global.get operations. + bool requiresNonNullableLocalFixups() override { return false; } + void run(PassRunner* runner, Module* module) override { if (!module->features.hasGC()) { return; @@ -102,6 +105,9 @@ struct GlobalRefining : public Pass { struct GetUpdater : public WalkerPass<PostWalker<GetUpdater>> { bool isFunctionParallel() override { return true; } + // Only modifies global.get operations. + bool requiresNonNullableLocalFixups() override { return false; } + GlobalRefining& parent; Module& wasm; diff --git a/src/passes/GlobalStructInference.cpp b/src/passes/GlobalStructInference.cpp index b2717b850..e2b3fa0c9 100644 --- a/src/passes/GlobalStructInference.cpp +++ b/src/passes/GlobalStructInference.cpp @@ -57,6 +57,9 @@ namespace wasm { namespace { struct GlobalStructInference : public Pass { + // Only modifies struct.get operations. + bool requiresNonNullableLocalFixups() override { return false; } + // Maps optimizable struct types to the globals whose init is a struct.new of // them. If a global is not present here, it cannot be optimized. std::unordered_map<HeapType, std::vector<Name>> typeGlobals; diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 2c0799874..dad54d116 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -366,7 +366,6 @@ struct GlobalTypeOptimization : public Pass { block->list.push_back(curr); block->finalize(curr->type); replaceCurrent(block); - addedLocals = true; } // Remove the unneeded operands. @@ -405,7 +404,6 @@ struct GlobalTypeOptimization : public Pass { getPassOptions()); replaceCurrent( builder.makeDrop(builder.makeRefAs(RefAsNonNull, flipped))); - addedLocals = true; } } @@ -420,15 +418,7 @@ struct GlobalTypeOptimization : public Pass { curr->index = newIndex; } - void visitFunction(Function* curr) { - if (addedLocals) { - TypeUpdating::handleNonDefaultableLocals(curr, *getModule()); - } - } - private: - bool addedLocals = false; - Index getNewIndex(HeapType type, Index index) { auto iter = parent.indexesAfterRemovals.find(type); if (iter == parent.indexesAfterRemovals.end()) { diff --git a/src/passes/Heap2Local.cpp b/src/passes/Heap2Local.cpp index 86235b001..104a5bbcd 100644 --- a/src/passes/Heap2Local.cpp +++ b/src/passes/Heap2Local.cpp @@ -181,8 +181,6 @@ struct Heap2LocalOptimizer { Parents parents; BranchUtils::BranchTargets branchTargets; - bool optimized = false; - Heap2LocalOptimizer(Function* func, Module* module, const PassOptions& passOptions) @@ -203,9 +201,7 @@ struct Heap2LocalOptimizer { continue; } - if (convertToLocals(allocation)) { - optimized = true; - } + convertToLocals(allocation); } } @@ -473,7 +469,7 @@ struct Heap2LocalOptimizer { // Analyze an allocation to see if we can convert it from a heap allocation to // locals. - bool convertToLocals(StructNew* allocation) { + void convertToLocals(StructNew* allocation) { Rewriter rewriter(allocation, func, module); // A queue of flows from children to parents. When something is in the queue @@ -502,13 +498,13 @@ struct Heap2LocalOptimizer { // different call to this function and use a different queue (any overlap // between calls would prove non-exclusivity). if (!seen.emplace(parent).second) { - return false; + return; } switch (getParentChildInteraction(parent, child)) { case ParentChildInteraction::Escapes: { // If the parent may let us escape then we are done. - return false; + return; } case ParentChildInteraction::FullyConsumes: { // If the parent consumes us without letting us escape then all is @@ -524,7 +520,7 @@ struct Heap2LocalOptimizer { case ParentChildInteraction::Mixes: { // Our allocation is not used exclusively via the parent, as other // values are mixed with it. Give up. - return false; + return; } } @@ -558,13 +554,11 @@ struct Heap2LocalOptimizer { // We finished the loop over the flows. Do the final checks. if (!getsAreExclusiveToSets(rewriter.sets)) { - return false; + return; } // We can do it, hurray! rewriter.applyOptimization(); - - return true; } ParentChildInteraction getParentChildInteraction(Expression* parent, @@ -750,9 +744,7 @@ struct Heap2Local : public WalkerPass<PostWalker<Heap2Local>> { // vacuum, in particular, to optimize such nested allocations. // TODO Consider running multiple iterations here, and running vacuum in // between them. - if (Heap2LocalOptimizer(func, getModule(), getPassOptions()).optimized) { - TypeUpdating::handleNonDefaultableLocals(func, *getModule()); - } + Heap2LocalOptimizer(func, getModule(), getPassOptions()); } }; diff --git a/src/passes/LocalCSE.cpp b/src/passes/LocalCSE.cpp index 4728f189a..e777241a4 100644 --- a/src/passes/LocalCSE.cpp +++ b/src/passes/LocalCSE.cpp @@ -549,8 +549,6 @@ struct LocalCSE : public WalkerPass<PostWalker<LocalCSE>> { Applier applier(requestInfos); applier.walkFunctionInModule(func, getModule()); - - TypeUpdating::handleNonDefaultableLocals(func, *getModule()); } }; diff --git a/src/passes/LocalSubtyping.cpp b/src/passes/LocalSubtyping.cpp index b6e94b771..31e689075 100644 --- a/src/passes/LocalSubtyping.cpp +++ b/src/passes/LocalSubtyping.cpp @@ -25,6 +25,7 @@ #include <ir/find_all.h> #include <ir/linear-execution.h> #include <ir/local-graph.h> +#include <ir/local-structural-dominance.h> #include <ir/lubs.h> #include <ir/utils.h> #include <pass.h> @@ -36,6 +37,10 @@ namespace wasm { struct LocalSubtyping : public WalkerPass<PostWalker<LocalSubtyping>> { bool isFunctionParallel() override { return true; } + // This pass carefully avoids breaking validation by only refining a local's + // type to be non-nullable if it would validate. + bool requiresNonNullableLocalFixups() override { return false; } + Pass* create() override { return new LocalSubtyping(); } void doWalkFunction(Function* func) { @@ -68,24 +73,30 @@ struct LocalSubtyping : public WalkerPass<PostWalker<LocalSubtyping>> { } } - // Find which vars use the default value, if we allow non-nullable locals. - // - // If that feature is not enabled, then we can safely assume that the - // default is never used - the default would be a null value, and the type - // of the null does not really matter as all nulls compare equally, so we do - // not need to worry. - std::unordered_set<Index> usesDefault; + // Find which vars can be non-nullable. + std::unordered_set<Index> cannotBeNonNullable; if (getModule()->features.hasGCNNLocals()) { + // If the feature is enabled then the only constraint is being able to + // read the default value - if it is readable, the local cannot become + // non-nullable. for (auto& [get, sets] : localGraph.getSetses) { auto index = get->index; if (func->isVar(index) && std::any_of(sets.begin(), sets.end(), [&](LocalSet* set) { return set == nullptr; })) { - usesDefault.insert(index); + cannotBeNonNullable.insert(index); } } + } else { + // Without GCNNLocals, validation rules follow the spec rules: all gets + // must be dominated structurally by sets, for the local to be non- + // nullable. + LocalStructuralDominance info(func, *getModule()); + for (auto index : info.nonDominatingIndices) { + cannotBeNonNullable.insert(index); + } } auto varBase = func->getVarIndexBase(); @@ -136,10 +147,7 @@ struct LocalSubtyping : public WalkerPass<PostWalker<LocalSubtyping>> { // Remove non-nullability if we disallow that in locals. if (newType.isNonNullable()) { - // As mentioned earlier, even if we allow non-nullability, there may - // be a problem if the default value - a null - is used. In that case, - // remove non-nullability as well. - if (!getModule()->features.hasGCNNLocals() || usesDefault.count(i)) { + if (cannotBeNonNullable.count(i)) { newType = Type(newType.getHeapType(), Nullable); } } else if (!newType.isDefaultable()) { diff --git a/src/passes/MemoryPacking.cpp b/src/passes/MemoryPacking.cpp index cd4a6698d..0f2cf6f14 100644 --- a/src/passes/MemoryPacking.cpp +++ b/src/passes/MemoryPacking.cpp @@ -95,6 +95,10 @@ makeGtShiftedMemorySize(Builder& builder, Module& module, MemoryInit* curr) { } // anonymous namespace struct MemoryPacking : public Pass { + // This pass operates on linear memory, and does not affect reference locals. + // TODO: don't run at all if the module has no memories + bool requiresNonNullableLocalFixups() override { return false; } + void run(PassRunner* runner, Module* module) override; bool canOptimize(std::vector<std::unique_ptr<Memory>>& memories, std::vector<std::unique_ptr<DataSegment>>& dataSegments, @@ -377,6 +381,10 @@ void MemoryPacking::calculateRanges(const std::unique_ptr<DataSegment>& segment, void MemoryPacking::optimizeBulkMemoryOps(PassRunner* runner, Module* module) { struct Optimizer : WalkerPass<PostWalker<Optimizer>> { bool isFunctionParallel() override { return true; } + + // This operates on linear memory, and does not affect reference locals. + bool requiresNonNullableLocalFixups() override { return false; } + Pass* create() override { return new Optimizer; } bool needsRefinalizing; @@ -777,6 +785,9 @@ void MemoryPacking::replaceBulkMemoryOps(PassRunner* runner, struct Replacer : WalkerPass<PostWalker<Replacer>> { bool isFunctionParallel() override { return true; } + // This operates on linear memory, and does not affect reference locals. + bool requiresNonNullableLocalFixups() override { return false; } + Replacements& replacements; Replacer(Replacements& replacements) : replacements(replacements){}; diff --git a/src/passes/MinifyImportsAndExports.cpp b/src/passes/MinifyImportsAndExports.cpp index 02675e49b..3e919c799 100644 --- a/src/passes/MinifyImportsAndExports.cpp +++ b/src/passes/MinifyImportsAndExports.cpp @@ -46,6 +46,9 @@ namespace wasm { struct MinifyImportsAndExports : public Pass { + // This operates on import/export names only. + bool requiresNonNullableLocalFixups() override { return false; } + bool minifyExports, minifyModules; public: diff --git a/src/passes/NameTypes.cpp b/src/passes/NameTypes.cpp index b855e4c0d..3ad35fa1d 100644 --- a/src/passes/NameTypes.cpp +++ b/src/passes/NameTypes.cpp @@ -28,6 +28,8 @@ namespace wasm { static const size_t NameLenLimit = 20; struct NameTypes : public Pass { + bool requiresNonNullableLocalFixups() override { return false; } + void run(PassRunner* runner, Module* module) override { // Find all the types. std::vector<HeapType> types = ModuleUtils::collectHeapTypes(*module); diff --git a/src/passes/OptimizeAddedConstants.cpp b/src/passes/OptimizeAddedConstants.cpp index 0af23c1cc..8256169c1 100644 --- a/src/passes/OptimizeAddedConstants.cpp +++ b/src/passes/OptimizeAddedConstants.cpp @@ -242,6 +242,9 @@ struct OptimizeAddedConstants UnifiedExpressionVisitor<OptimizeAddedConstants>>> { bool isFunctionParallel() override { return true; } + // This pass operates on linear memory, and does not affect reference locals. + bool requiresNonNullableLocalFixups() override { return false; } + bool propagate; OptimizeAddedConstants(bool propagate) : propagate(propagate) {} diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp index 9379d8588..42d92751f 100644 --- a/src/passes/OptimizeInstructions.cpp +++ b/src/passes/OptimizeInstructions.cpp @@ -238,9 +238,6 @@ struct OptimizeInstructions optimizer.walkFunction(func); } - // Some patterns create locals (like when we use getResultOfFirst), which we - // may need to fix up. - TypeUpdating::handleNonDefaultableLocals(func, *getModule()); // Some patterns create blocks that can interfere 'catch' and 'pop', nesting // the 'pop' into a block making it invalid. EHUtils::handleBlockNestedPops(func, *getModule()); diff --git a/src/passes/RemoveUnusedModuleElements.cpp b/src/passes/RemoveUnusedModuleElements.cpp index 1b1174745..139f6f655 100644 --- a/src/passes/RemoveUnusedModuleElements.cpp +++ b/src/passes/RemoveUnusedModuleElements.cpp @@ -230,6 +230,10 @@ struct ReachabilityAnalyzer : public PostWalker<ReachabilityAnalyzer> { }; struct RemoveUnusedModuleElements : public Pass { + // This pass only removes module elements, it never modifies function + // contents. + bool requiresNonNullableLocalFixups() override { return false; } + bool rootAllFunctions; RemoveUnusedModuleElements(bool rootAllFunctions) diff --git a/src/passes/RemoveUnusedNames.cpp b/src/passes/RemoveUnusedNames.cpp index a08cb5eda..4c74245f0 100644 --- a/src/passes/RemoveUnusedNames.cpp +++ b/src/passes/RemoveUnusedNames.cpp @@ -31,6 +31,10 @@ struct RemoveUnusedNames UnifiedExpressionVisitor<RemoveUnusedNames>>> { bool isFunctionParallel() override { return true; } + // This pass only removes names, which can only help validation (as blocks + // without names are ignored, see the README section on non-nullable locals). + bool requiresNonNullableLocalFixups() override { return false; } + Pass* create() override { return new RemoveUnusedNames; } // We maintain a list of branches that we saw in children, then when we reach diff --git a/src/passes/ReorderFunctions.cpp b/src/passes/ReorderFunctions.cpp index 7d2e7fb53..6f87e74a6 100644 --- a/src/passes/ReorderFunctions.cpp +++ b/src/passes/ReorderFunctions.cpp @@ -57,6 +57,9 @@ private: }; struct ReorderFunctions : public Pass { + // Only reorders functions, does not change their contents. + bool requiresNonNullableLocalFixups() override { return false; } + void run(PassRunner* runner, Module* module) override { NameCountMap counts; // fill in info, as we operate on it in parallel (each function to its own diff --git a/src/passes/ReorderLocals.cpp b/src/passes/ReorderLocals.cpp index 7b2de0d70..ce06afd68 100644 --- a/src/passes/ReorderLocals.cpp +++ b/src/passes/ReorderLocals.cpp @@ -32,6 +32,9 @@ namespace wasm { struct ReorderLocals : public WalkerPass<PostWalker<ReorderLocals>> { bool isFunctionParallel() override { return true; } + // Sorting and removing unused locals cannot affect validation. + bool requiresNonNullableLocalFixups() override { return false; } + Pass* create() override { return new ReorderLocals; } // local index => times it is used diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index 07867b32d..089def301 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -53,7 +53,6 @@ #include "ir/find_all.h" #include "ir/literal-utils.h" #include "ir/local-graph.h" -#include "ir/type-updating.h" #include "pass.h" #include "support/permutations.h" #include "wasm-builder.h" @@ -99,8 +98,6 @@ struct SSAify : public Pass { computeGetsAndPhis(graph); // add prepends to function addPrepends(); - // Handle non-nullability in new locals we added. - TypeUpdating::handleNonDefaultableLocals(func, *module); } void createNewIndexes(LocalGraph& graph) { @@ -140,10 +137,14 @@ struct SSAify : public Pass { // no set, assign param or zero if (func->isParam(get->index)) { // leave it, it's fine - } else { + } else if (LiteralUtils::canMakeZero(get->type)) { // zero it out (*graph.locations[get]) = LiteralUtils::makeZero(get->type, *module); + } else { + // No zero exists here, so this is a nondefaultable type. The + // default won't be used anyhow, so this value does not really + // matter and we have nothing to do. } } continue; diff --git a/src/passes/SetGlobals.cpp b/src/passes/SetGlobals.cpp index 8a147665c..4109eb69b 100644 --- a/src/passes/SetGlobals.cpp +++ b/src/passes/SetGlobals.cpp @@ -25,6 +25,9 @@ namespace wasm { struct SetGlobals : public Pass { + // Only modifies globals. + bool requiresNonNullableLocalFixups() override { return false; } + void run(PassRunner* runner, Module* module) override { Name input = runner->options.getArgument( "set-globals", diff --git a/src/passes/SignatureRefining.cpp b/src/passes/SignatureRefining.cpp index 11df83f4d..d00626fca 100644 --- a/src/passes/SignatureRefining.cpp +++ b/src/passes/SignatureRefining.cpp @@ -41,6 +41,9 @@ namespace wasm { namespace { struct SignatureRefining : public Pass { + // Only changes heap types and parameter types (but not locals). + bool requiresNonNullableLocalFixups() override { return false; } + // Maps each heap type to the possible refinement of the types in their // signatures. We will fill this during analysis and then use it while doing // an update of the types. If a type has no improvement that we can find, it @@ -241,6 +244,10 @@ struct SignatureRefining : public Pass { struct CodeUpdater : public WalkerPass<PostWalker<CodeUpdater>> { bool isFunctionParallel() override { return true; } + // Updating parameter types cannot affect validation (only updating var + // types types might). + bool requiresNonNullableLocalFixups() override { return false; } + SignatureRefining& parent; Module& wasm; diff --git a/src/passes/SimplifyLocals.cpp b/src/passes/SimplifyLocals.cpp index ee9e43b0b..6250b1321 100644 --- a/src/passes/SimplifyLocals.cpp +++ b/src/passes/SimplifyLocals.cpp @@ -331,50 +331,6 @@ struct SimplifyLocals invalidated.push_back(index); continue; } - - // Non-nullable local.sets cannot be moved into a try, as that may - // change dominance from the perspective of the spec - // - // (local.set $x X) - // (try - // .. - // (Y - // (local.get $x)) - // (catch - // (Z - // (local.get $x))) - // - // => - // - // (try - // .. - // (Y - // (local.tee $x X)) - // (catch - // (Z - // (local.get $x))) - // - // After sinking the set, the tee does not dominate the get in the - // catch, at least not in the simple way the spec defines it, see - // https://github.com/WebAssembly/function-references/issues/44#issuecomment-1083146887 - // We have more refined information about control flow and dominance - // than the spec, and so we would see if ".." can throw or not (only if - // it can throw is there a branch to the catch, which can change - // dominance). To stay compliant with the spec, however, we must not - // move code regardless of whether ".." can throw - we must simply keep - // the set outside of the try. - // - // The problem described can also occur on the *value* and not the set - // itself. For example, |X| above could be a local.set of a non-nullable - // local. For that reason we must scan it all. - if (self->getModule()->features.hasGCNNLocals()) { - for (auto* set : FindAll<LocalSet>(*info.item).list) { - if (self->getFunction()->getLocalType(set->index).isNonNullable()) { - invalidated.push_back(index); - break; - } - } - } } for (auto index : invalidated) { self->sinkables.erase(index); @@ -804,7 +760,48 @@ struct SimplifyLocals if (sinkables.empty()) { return; } + + // Check if the type makes sense. A non-nullable local might be dangerous + // here, as creating new local.gets for such locals is risky: + // + // (func $silly + // (local $x (ref $T)) + // (if + // (condition) + // (local.set $x ..) + // ) + // ) + // + // That local is silly as the write is never read. If we optimize it and add + // a local.get, however, then we'd no longer validate (as no set would + // dominate that new get in the if's else arm). Fixups would add a + // ref.as_non_null around the local.get, which will then trap at runtime: + // + // (func $silly + // (local $x (ref null $T)) + // (local.set $x + // (if + // (condition) + // (..) + // (ref.as_non_null + // (local.get $x) + // ) + // ) + // ) + // ) + // + // In other words, local.get is not necessarily free of effects if the local + // is non-nullable - it must have been set already. We could check that + // here, but running that linear-time check may not be worth it as this + // optimization is fairly minor, so just skip the non-nullable case. + // + // TODO investigate more Index goodIndex = sinkables.begin()->first; + auto localType = this->getFunction()->getLocalType(goodIndex); + if (localType.isNonNullable()) { + return; + } + // Ensure we have a place to write the return values for, if not, we // need another cycle. auto* ifTrueBlock = iff->ifTrue->dynCast<Block>(); @@ -813,6 +810,9 @@ struct SimplifyLocals ifsToEnlarge.push_back(iff); return; } + + // We can optimize! + // Update the ifTrue side. Builder builder(*this->getModule()); auto** item = sinkables.at(goodIndex).item; @@ -822,8 +822,7 @@ struct SimplifyLocals ifTrueBlock->finalize(); assert(ifTrueBlock->type != Type::none); // Update the ifFalse side. - iff->ifFalse = builder.makeLocalGet( - set->index, this->getFunction()->getLocalType(set->index)); + iff->ifFalse = builder.makeLocalGet(set->index, localType); iff->finalize(); // update type // Update the get count. getCounter.num[set->index]++; diff --git a/src/passes/StackCheck.cpp b/src/passes/StackCheck.cpp index fd7ed6c0f..11ab1fe4e 100644 --- a/src/passes/StackCheck.cpp +++ b/src/passes/StackCheck.cpp @@ -67,6 +67,9 @@ struct EnforceStackLimits : public WalkerPass<PostWalker<EnforceStackLimits>> { bool isFunctionParallel() override { return true; } + // Only affects linear memory operations. + bool requiresNonNullableLocalFixups() override { return false; } + Pass* create() override { return new EnforceStackLimits( stackPointer, stackBase, stackLimit, builder, handler); diff --git a/src/passes/StackIR.cpp b/src/passes/StackIR.cpp index 84e19428d..e896af0f5 100644 --- a/src/passes/StackIR.cpp +++ b/src/passes/StackIR.cpp @@ -69,41 +69,7 @@ public: if (passOptions.optimizeLevel >= 3 || passOptions.shrinkLevel >= 1) { local2Stack(); } - // Removing unneeded blocks is dangerous with GC, as if we do this: - // - // (call - // (struct.new) - // (block - // (nop) - // (i32) - // ) - // ) - // === remove inner block ==> - // (call - // (struct.new) - // (nop) - // (i32) - // ) - // - // Then we end up with a nop that forces us to emit this during load: - // - // (call - // (block - // (local.set - // (struct.new) - // ) - // (nop) - // (local.get) - // ) - // (i32) - // ) - // - // However, that is not valid as an non-nullable reference cannot be set to - // a local. TODO: double check that this is still true now that we don't - // have RTTs. - if (!features.hasGC()) { - removeUnneededBlocks(); - } + removeUnneededBlocks(); dce(); } diff --git a/src/passes/Strip.cpp b/src/passes/Strip.cpp index 6377d2367..7c9f61061 100644 --- a/src/passes/Strip.cpp +++ b/src/passes/Strip.cpp @@ -28,6 +28,8 @@ namespace wasm { struct Strip : public Pass { + bool requiresNonNullableLocalFixups() override { return false; } + // A function that returns true if the method should be removed. typedef std::function<bool(UserSection&)> Decider; Decider decider; diff --git a/src/passes/StripTargetFeatures.cpp b/src/passes/StripTargetFeatures.cpp index cb5e51f08..878004a98 100644 --- a/src/passes/StripTargetFeatures.cpp +++ b/src/passes/StripTargetFeatures.cpp @@ -19,6 +19,8 @@ namespace wasm { struct StripTargetFeatures : public Pass { + bool requiresNonNullableLocalFixups() override { return false; } + bool isStripped = false; StripTargetFeatures(bool isStripped) : isStripped(isStripped) {} void run(PassRunner* runner, Module* module) override { diff --git a/src/passes/TypeRefining.cpp b/src/passes/TypeRefining.cpp index ac95d01fe..3d3f981db 100644 --- a/src/passes/TypeRefining.cpp +++ b/src/passes/TypeRefining.cpp @@ -93,6 +93,9 @@ struct FieldInfoScanner }; struct TypeRefining : public Pass { + // Only affects GC type declarations and struct.gets. + bool requiresNonNullableLocalFixups() override { return false; } + StructUtils::StructValuesMap<FieldInfo> finalInfos; void run(PassRunner* runner, Module* module) override { @@ -235,6 +238,9 @@ struct TypeRefining : public Pass { struct ReadUpdater : public WalkerPass<PostWalker<ReadUpdater>> { bool isFunctionParallel() override { return true; } + // Only affects struct.gets. + bool requiresNonNullableLocalFixups() override { return false; } + TypeRefining& parent; ReadUpdater(TypeRefining& parent) : parent(parent) {} diff --git a/src/passes/pass.cpp b/src/passes/pass.cpp index 570cd8610..f55f5018e 100644 --- a/src/passes/pass.cpp +++ b/src/passes/pass.cpp @@ -23,6 +23,7 @@ #include "ir/hashed.h" #include "ir/module-utils.h" +#include "ir/type-updating.h" #include "pass.h" #include "passes/passes.h" #include "support/colors.h" @@ -936,16 +937,29 @@ void PassRunner::runPassOnFunction(Pass* pass, Function* func) { } void PassRunner::handleAfterEffects(Pass* pass, Function* func) { - if (pass->modifiesBinaryenIR()) { - // If Binaryen IR is modified, Stack IR must be cleared - it would - // be out of sync in a potentially dangerous way. - if (func) { - func->stackIR.reset(nullptr); - } else { - for (auto& func : wasm->functions) { - func->stackIR.reset(nullptr); - } + if (!pass->modifiesBinaryenIR()) { + return; + } + + // Binaryen IR is modified, so we may have work here. + + if (!func) { + // If no function is provided, then this is not a function-parallel pass, + // and it may have operated on any of the functions in theory, so run on + // them all. + assert(!pass->isFunctionParallel()); + for (auto& func : wasm->functions) { + handleAfterEffects(pass, func.get()); } + return; + } + + // If Binaryen IR is modified, Stack IR must be cleared - it would + // be out of sync in a potentially dangerous way. + func->stackIR.reset(nullptr); + + if (pass->requiresNonNullableLocalFixups()) { + TypeUpdating::handleNonDefaultableLocals(func, *wasm); } } diff --git a/src/tools/fuzzing.h b/src/tools/fuzzing.h index 47d55cfef..aff2a0aa3 100644 --- a/src/tools/fuzzing.h +++ b/src/tools/fuzzing.h @@ -76,8 +76,9 @@ public: void build(); -private: Module& wasm; + +private: Builder builder; Random random; diff --git a/src/tools/fuzzing/fuzzing.cpp b/src/tools/fuzzing/fuzzing.cpp index 828f80693..0f59bb635 100644 --- a/src/tools/fuzzing/fuzzing.cpp +++ b/src/tools/fuzzing/fuzzing.cpp @@ -15,6 +15,7 @@ */ #include "tools/fuzzing.h" +#include "ir/type-updating.h" #include "tools/fuzzing/heap-types.h" #include "tools/fuzzing/parameters.h" @@ -461,6 +462,9 @@ TranslateToFuzzReader::FunctionCreationContext::~FunctionCreationContext() { assert(breakableStack.empty()); assert(hangStack.empty()); parent.funcContext = nullptr; + + // We must ensure non-nullable locals validate. + TypeUpdating::handleNonDefaultableLocals(func, parent.wasm); } Expression* TranslateToFuzzReader::makeHangLimitCheck() { diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index 75cbcf53a..c1a8eea7f 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -24,6 +24,7 @@ #include "ir/global-utils.h" #include "ir/intrinsics.h" #include "ir/local-graph.h" +#include "ir/local-structural-dominance.h" #include "ir/module-utils.h" #include "ir/stack-utils.h" #include "ir/utils.h" @@ -2761,8 +2762,6 @@ void FunctionValidator::visitFunction(Function* curr) { } for (const auto& var : curr->vars) { features |= var.getFeatures(); - bool valid = getModule()->features.hasGCNNLocals() || var.isDefaultable(); - shouldBeTrue(valid, var, "vars must be defaultable"); } shouldBeTrue(features <= getModule()->features, curr->name, @@ -2796,32 +2795,43 @@ void FunctionValidator::visitFunction(Function* curr) { shouldBeTrue(seen.insert(name).second, name, "local names must be unique"); } - if (getModule()->features.hasGCNNLocals()) { - // If we have non-nullable locals, verify that no local.get can read a null - // default value. - // TODO: this can be fairly slow due to the LocalGraph. writing more code to - // do a more specific analysis (we don't need to know all sets, just - // if there is a set of a null default value that is read) could be a - // lot faster. - bool hasNNLocals = false; - for (const auto& var : curr->vars) { - if (var.isNonNullable()) { - hasNNLocals = true; - break; + if (getModule()->features.hasGC()) { + // If we have non-nullable locals, verify that local.get are valid. + if (!getModule()->features.hasGCNNLocals()) { + // Without the special GCNNLocals feature, we implement the spec rules, + // that is, a set allows gets until the end of the block. + LocalStructuralDominance info(curr, *getModule()); + for (auto index : info.nonDominatingIndices) { + shouldBeTrue(!curr->getLocalType(index).isNonNullable(), + index, + "non-nullable local's sets must dominate gets"); } - } - if (hasNNLocals) { - LocalGraph graph(curr); - for (auto& [get, sets] : graph.getSetses) { - auto index = get->index; - // It is always ok to read nullable locals, and it is always ok to read - // params even if they are non-nullable. - if (!curr->getLocalType(index).isNonNullable() || - curr->isParam(index)) { - continue; + } else { + // With the special GCNNLocals feature, we allow gets anywhere, so long as + // we can prove they cannot read the null value. (TODO: remove this once + // the spec is stable). + // + // This is slow, so only do it if we find such locals exist at all. + bool hasNNLocals = false; + for (const auto& var : curr->vars) { + if (!var.isDefaultable()) { + hasNNLocals = true; + break; } - for (auto* set : sets) { - shouldBeTrue(!!set, index, "non-nullable local must not read null"); + } + if (hasNNLocals) { + LocalGraph graph(curr); + for (auto& [get, sets] : graph.getSetses) { + auto index = get->index; + // It is always ok to read nullable locals, and it is always ok to + // read params even if they are non-nullable. + if (curr->getLocalType(index).isDefaultable() || + curr->isParam(index)) { + continue; + } + for (auto* set : sets) { + shouldBeTrue(!!set, index, "non-nullable local must not read null"); + } } } } diff --git a/test/ctor-eval/gc.wast b/test/ctor-eval/gc.wast index 0449b6140..965f6f38a 100644 --- a/test/ctor-eval/gc.wast +++ b/test/ctor-eval/gc.wast @@ -22,8 +22,8 @@ (global $global2 (mut (ref null $struct)) (ref.null $struct)) (func "test1" - ;; Leave the first local as null, which we should handle properly (we will - ;; end up emitting nothing and still using the default null value). + ;; The locals will be optimized into a single non-nullable one by the + ;; optimizer. (local $temp1 (ref null $struct)) (local $temp2 (ref null $struct)) diff --git a/test/ctor-eval/gc.wast.out b/test/ctor-eval/gc.wast.out index b926b5ad4..e1520f12b 100644 --- a/test/ctor-eval/gc.wast.out +++ b/test/ctor-eval/gc.wast.out @@ -27,7 +27,7 @@ ) ) (func $0_0 - (local $0 (ref null $struct)) + (local $0 (ref $struct)) (local.set $0 (global.get $ctor-eval$global_0) ) diff --git a/test/lit/exec/read-nn-null.wast b/test/lit/exec/read-nn-null.wast new file mode 100644 index 000000000..d72688969 --- /dev/null +++ b/test/lit/exec/read-nn-null.wast @@ -0,0 +1,74 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py --all-items --output=fuzz-exec and should not be edited. + +;; RUN: wasm-opt %s -all --coalesce-locals --optimize-instructions --fuzz-exec -q -o /dev/null 2>&1 | filecheck %s --check-prefix=NORMAL +;; RUN: wasm-opt %s -tnh -all --coalesce-locals --optimize-instructions --fuzz-exec -q -o /dev/null 2>&1 | filecheck %s --check-prefix=TNH + +;; The sequence of passes here will do the following: +;; +;; * coalesce-locals will remove the local.set. That does not reach any +;; local.get due to the unreachable, so it is a dead store that we can +;; remove. +;; * optimize-instructions will reorder the select's arms (to get rid of the +;; i32.eqz). It looks ok to reorder them because the local.get on one arm has +;; no side effects at all. +;; +;; After those, if we did not fix up non-nullable locals in between, then we'd +;; end up executing a local.get of a non-nullable local that has no local.set, +;; which means we are reading a null by a non-nullable local - an internal +;; error. We avoid this situation by fixing up non-nullable locals in between +;; these passes, which adds a ref.as_non_null on the local.get, and that +;; possibly-trapping instruction will prevent dangerous reordering. In +;; particular, --fuzz-exec result should be identical before and after: always +;; log 42 and then trap on that unreachable. +;; +;; This also tests traps-never-happen mode. Atm that mode changes nothing here, +;; but that may change in the future as tnh starts to optimize more things. In +;; particular, tnh can in principle remove the ref.as_non_null that is added on +;; the local.get, which would then let optimize-instructions reorder - but that +;; will still not affect observable behavior, so it is fine. + +(module + (import "fuzzing-support" "log-i32" (func $log (param i32))) + + (func $foo (export "foo") (param $i i32) (result funcref) + (local $ref (ref func)) + (local.set $ref + (ref.func $foo) + ) + (select (result funcref) + (block $trap (result funcref) + (call $log + (i32.const 42) + ) + ;; We never reach the br, but its existence makes the block's type none + ;; instead of unreachable (optimization passes may ignore such + ;; obviously-unreachable code, so we make it less obvious this way). + (unreachable) + (br $trap + (ref.func $foo) + ) + ) + (local.get $ref) + (i32.eqz + (local.get $i) + ) + ) + ) +) +;; NORMAL: [fuzz-exec] calling foo +;; NORMAL-NEXT: [LoggingExternalInterface logging 42] +;; NORMAL-NEXT: [trap unreachable] + +;; NORMAL: [fuzz-exec] calling foo +;; NORMAL-NEXT: [LoggingExternalInterface logging 42] +;; NORMAL-NEXT: [trap unreachable] +;; NORMAL-NEXT: [fuzz-exec] comparing foo + +;; TNH: [fuzz-exec] calling foo +;; TNH-NEXT: [LoggingExternalInterface logging 42] +;; TNH-NEXT: [trap unreachable] + +;; TNH: [fuzz-exec] calling foo +;; TNH-NEXT: [LoggingExternalInterface logging 42] +;; TNH-NEXT: [trap unreachable] +;; TNH-NEXT: [fuzz-exec] comparing foo diff --git a/test/lit/non-nullable-locals.wast b/test/lit/non-nullable-locals.wast new file mode 100644 index 000000000..2a469b9e1 --- /dev/null +++ b/test/lit/non-nullable-locals.wast @@ -0,0 +1,176 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. + +;; RUN: foreach %s %t wasm-opt -all -S -o - | filecheck %s + +;; Tests for validation of non-nullable locals. + +(module + ;; CHECK: (type $none_=>_none (func)) + + ;; CHECK: (type $ref|func|_=>_none (func (param (ref func)))) + + ;; CHECK: (type $funcref_=>_i32 (func (param funcref) (result i32))) + + ;; CHECK: (elem declare func $helper) + + ;; CHECK: (func $no-uses + ;; CHECK-NEXT: (local $x (ref func)) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: ) + (func $no-uses + ;; A local with no uses validates. + (local $x (ref func)) + ) + + ;; CHECK: (func $func-scope + ;; CHECK-NEXT: (local $x (ref func)) + ;; CHECK-NEXT: (local.set $x + ;; CHECK-NEXT: (ref.func $helper) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $func-scope + ;; a set in the func scope helps a get validate there. + (local $x (ref func)) + (local.set $x + (ref.func $helper) + ) + (drop + (local.get $x) + ) + ) + + ;; CHECK: (func $inner-scope + ;; CHECK-NEXT: (local $x (ref func)) + ;; CHECK-NEXT: (block $b + ;; CHECK-NEXT: (local.set $x + ;; CHECK-NEXT: (ref.func $helper) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $inner-scope + ;; a set in an inner scope helps a get validate there. + (local $x (ref func)) + (block $b + (local.set $x + (ref.func $helper) + ) + (drop + (local.get $x) + ) + ) + ) + + ;; CHECK: (func $func-to-inner + ;; CHECK-NEXT: (local $x (ref func)) + ;; CHECK-NEXT: (local.set $x + ;; CHECK-NEXT: (ref.func $helper) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (block $b + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $func-to-inner + ;; a set in an outer scope helps a get validate. + (local $x (ref func)) + (local.set $x + (ref.func $helper) + ) + (block $b + (drop + (local.get $x) + ) + ) + ) + + ;; CHECK: (func $inner-to-func + ;; CHECK-NEXT: (local $x funcref) + ;; CHECK-NEXT: (block $b + ;; CHECK-NEXT: (local.set $x + ;; CHECK-NEXT: (ref.func $helper) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $inner-to-func + ;; a set in an inner scope does *not* help a get validate, but the type is + ;; nullable so that's ok. + (local $x (ref null func)) + (block $b + (local.set $x + (ref.func $helper) + ) + ) + (drop + (local.get $x) + ) + ) + + ;; CHECK: (func $if-condition + ;; CHECK-NEXT: (local $x (ref func)) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (call $helper2 + ;; CHECK-NEXT: (local.tee $x + ;; CHECK-NEXT: (ref.func $helper) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $if-condition + (local $x (ref func)) + (if + (call $helper2 + ;; Tee in the condition is good enough for the arms. + (local.tee $x + (ref.func $helper) + ) + ) + (drop + (local.get $x) + ) + (drop + (local.get $x) + ) + ) + ) + + ;; CHECK: (func $get-without-set-but-param (param $x (ref func)) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $get-without-set-but-param + ;; As a parameter, this is ok to get without a set. + (param $x (ref func)) + (drop + (local.get $x) + ) + ) + + ;; CHECK: (func $helper + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: ) + (func $helper) + + ;; CHECK: (func $helper2 (param $0 funcref) (result i32) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $helper2 (param funcref) (result i32) + (unreachable) + ) +) diff --git a/test/lit/passes/catch-pop-fixup-eh.wast b/test/lit/passes/catch-pop-fixup-eh.wast index 51a703945..813bb6899 100644 --- a/test/lit/passes/catch-pop-fixup-eh.wast +++ b/test/lit/passes/catch-pop-fixup-eh.wast @@ -355,7 +355,7 @@ ) ;; CHECK: (func $pop-non-defaultable-type-within-block - ;; CHECK-NEXT: (local $0 (ref null $struct.A)) + ;; CHECK-NEXT: (local $0 (ref $struct.A)) ;; CHECK-NEXT: (try $try ;; CHECK-NEXT: (do ;; CHECK-NEXT: (nop) @@ -366,9 +366,7 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: (throw $e-struct.A ;; CHECK-NEXT: (block (result (ref $struct.A)) - ;; CHECK-NEXT: (ref.as_non_null - ;; CHECK-NEXT: (local.get $0) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.get $0) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) diff --git a/test/lit/passes/coalesce-locals-gc.wast b/test/lit/passes/coalesce-locals-gc.wast index 074c9caf0..0113af23c 100644 --- a/test/lit/passes/coalesce-locals-gc.wast +++ b/test/lit/passes/coalesce-locals-gc.wast @@ -1,7 +1,12 @@ ;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited. -;; RUN: wasm-opt %s --coalesce-locals -all -S -o - \ +;; RUN: wasm-opt %s --remove-unused-names --coalesce-locals -all -S -o - \ ;; RUN: | filecheck %s +;; --remove-unused-names is run to avoid adding names to blocks. Block names +;; can prevent non-nullable local validation (we emit named blocks in the binary +;; format, if we need them, but never emit unnamed ones), which affects some +;; testcases. + (module ;; CHECK: (type $array (array (mut i8))) (type $array (array (mut i8))) @@ -61,4 +66,74 @@ (local.get $1) ) ) + + ;; CHECK: (func $nn-dead + ;; CHECK-NEXT: (local $0 funcref) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.func $nn-dead) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (block $inner + ;; CHECK-NEXT: (local.set $0 + ;; CHECK-NEXT: (ref.func $nn-dead) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (br_if $inner + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.as_non_null + ;; CHECK-NEXT: (local.get $0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $nn-dead + (local $x (ref func)) + (local.set $x + (ref.func $nn-dead) ;; this will be removed, as it is not needed. + ) + (block $inner + (local.set $x + (ref.func $nn-dead) ;; this is not enough for validation of the get, so we + ;; will end up making the local nullable. + ) + ;; refer to $inner to keep the name alive (see the next testcase) + (br_if $inner + (i32.const 1) + ) + ) + (drop + (local.get $x) + ) + ) + + ;; CHECK: (func $nn-dead-nameless + ;; CHECK-NEXT: (local $0 (ref func)) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.func $nn-dead) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (block + ;; CHECK-NEXT: (local.set $0 + ;; CHECK-NEXT: (ref.func $nn-dead) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $nn-dead-nameless + (local $x (ref func)) + (local.set $x + (ref.func $nn-dead) + ) + ;; As above, but now the block has no name. Nameless blocks do not interfere + ;; with validation, so we can keep the local non-nullable. + (block + (local.set $x + (ref.func $nn-dead) + ) + ) + (drop + (local.get $x) + ) + ) ) diff --git a/test/lit/passes/dae-gc.wast b/test/lit/passes/dae-gc.wast index 5ec272ea4..e6c83f0d0 100644 --- a/test/lit/passes/dae-gc.wast +++ b/test/lit/passes/dae-gc.wast @@ -66,13 +66,13 @@ ) ) ;; A function that gets a non-nullable reference that is never used. We can - ;; still create a nullable local for that parameter. + ;; still create a non-nullable local for that parameter. ;; CHECK: (func $get-nonnull - ;; CHECK-NEXT: (local $0 (ref null ${})) + ;; CHECK-NEXT: (local $0 (ref ${})) ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: ) ;; NOMNL: (func $get-nonnull (type $none_=>_none) - ;; NOMNL-NEXT: (local $0 (ref null ${})) + ;; NOMNL-NEXT: (local $0 (ref ${})) ;; NOMNL-NEXT: (nop) ;; NOMNL-NEXT: ) (func $get-nonnull (param $0 (ref ${})) @@ -94,15 +94,13 @@ ;; Test ref.func and ref.null optimization of constant parameter values. (module ;; CHECK: (func $foo (param $0 (ref $none_=>_none)) - ;; CHECK-NEXT: (local $1 (ref null $none_=>_none)) + ;; CHECK-NEXT: (local $1 (ref $none_=>_none)) ;; CHECK-NEXT: (local.set $1 ;; CHECK-NEXT: (ref.func $a) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (block ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (ref.as_non_null - ;; CHECK-NEXT: (local.get $1) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.get $1) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (local.get $0) @@ -110,15 +108,13 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; NOMNL: (func $foo (type $ref|none_->_none|_=>_none) (param $0 (ref $none_=>_none)) - ;; NOMNL-NEXT: (local $1 (ref null $none_=>_none)) + ;; NOMNL-NEXT: (local $1 (ref $none_=>_none)) ;; NOMNL-NEXT: (local.set $1 ;; NOMNL-NEXT: (ref.func $a) ;; NOMNL-NEXT: ) ;; NOMNL-NEXT: (block ;; NOMNL-NEXT: (drop - ;; NOMNL-NEXT: (ref.as_non_null - ;; NOMNL-NEXT: (local.get $1) - ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: (local.get $1) ;; NOMNL-NEXT: ) ;; NOMNL-NEXT: (drop ;; NOMNL-NEXT: (local.get $0) diff --git a/test/lit/passes/denan.wast b/test/lit/passes/denan.wast index 47912bb1f..f92570f43 100644 --- a/test/lit/passes/denan.wast +++ b/test/lit/passes/denan.wast @@ -248,6 +248,7 @@ ) ) + ;; CHECK: (func $deNan32_0 (param $0 f32) (result f32) ;; CHECK-NEXT: (if (result f32) ;; CHECK-NEXT: (f32.eq diff --git a/test/lit/passes/directize_all-features.wast b/test/lit/passes/directize_all-features.wast index 5e42e271e..491a7e064 100644 --- a/test/lit/passes/directize_all-features.wast +++ b/test/lit/passes/directize_all-features.wast @@ -1166,46 +1166,39 @@ ) ;; CHECK: (func $select-non-nullable (param $x i32) - ;; CHECK-NEXT: (local $1 (ref null $i32_=>_none)) + ;; CHECK-NEXT: (local $1 (ref $i32_=>_none)) ;; CHECK-NEXT: (local.set $1 ;; CHECK-NEXT: (ref.func $select-non-nullable) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (if ;; CHECK-NEXT: (local.get $x) ;; CHECK-NEXT: (call $foo-ref - ;; CHECK-NEXT: (ref.as_non_null - ;; CHECK-NEXT: (local.get $1) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.get $1) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (call $foo-ref - ;; CHECK-NEXT: (ref.as_non_null - ;; CHECK-NEXT: (local.get $1) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.get $1) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; IMMUT: (func $select-non-nullable (param $x i32) - ;; IMMUT-NEXT: (local $1 (ref null $i32_=>_none)) + ;; IMMUT-NEXT: (local $1 (ref $i32_=>_none)) ;; IMMUT-NEXT: (local.set $1 ;; IMMUT-NEXT: (ref.func $select-non-nullable) ;; IMMUT-NEXT: ) ;; IMMUT-NEXT: (if ;; IMMUT-NEXT: (local.get $x) ;; IMMUT-NEXT: (call $foo-ref - ;; IMMUT-NEXT: (ref.as_non_null - ;; IMMUT-NEXT: (local.get $1) - ;; IMMUT-NEXT: ) + ;; IMMUT-NEXT: (local.get $1) ;; IMMUT-NEXT: ) ;; IMMUT-NEXT: (call $foo-ref - ;; IMMUT-NEXT: (ref.as_non_null - ;; IMMUT-NEXT: (local.get $1) - ;; IMMUT-NEXT: ) + ;; IMMUT-NEXT: (local.get $1) ;; IMMUT-NEXT: ) ;; IMMUT-NEXT: ) ;; IMMUT-NEXT: ) (func $select-non-nullable (param $x i32) ;; Test we can handle a non-nullable value when optimizing a select, during - ;; which we place values in locals. + ;; which we place values in locals. The local can remain non-nullable since it + ;; dominates the uses. (call_indirect (type $F) (ref.func $select-non-nullable) (select diff --git a/test/lit/passes/flatten.wast b/test/lit/passes/flatten.wast index e07c8fafb..0446f3ce1 100644 --- a/test/lit/passes/flatten.wast +++ b/test/lit/passes/flatten.wast @@ -5,14 +5,12 @@ ;; CHECK: (type $simplefunc (func)) (type $simplefunc (func)) ;; CHECK: (func $0 (param $0 (ref $simplefunc)) (result (ref $simplefunc)) - ;; CHECK-NEXT: (local $1 (ref null $simplefunc)) + ;; CHECK-NEXT: (local $1 (ref $simplefunc)) ;; CHECK-NEXT: (local.set $1 ;; CHECK-NEXT: (local.get $0) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (return - ;; CHECK-NEXT: (ref.as_non_null - ;; CHECK-NEXT: (local.get $1) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.get $1) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $0 (param $0 (ref $simplefunc)) (result (ref $simplefunc)) diff --git a/test/lit/passes/flatten_all-features.wast b/test/lit/passes/flatten_all-features.wast index c6512deeb..934706419 100644 --- a/test/lit/passes/flatten_all-features.wast +++ b/test/lit/passes/flatten_all-features.wast @@ -3518,16 +3518,14 @@ ;; CHECK: (type $none_=>_funcref (func (result funcref))) ;; CHECK: (func $0 (result funcref) - ;; CHECK-NEXT: (local $0 (ref null $none_=>_none)) + ;; CHECK-NEXT: (local $0 (ref $none_=>_none)) ;; CHECK-NEXT: (local.set $0 ;; CHECK-NEXT: (ref.as_non_null ;; CHECK-NEXT: (ref.null $none_=>_none) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (return - ;; CHECK-NEXT: (ref.as_non_null - ;; CHECK-NEXT: (local.get $0) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.get $0) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $0 (result funcref) diff --git a/test/lit/passes/gto-removals.wast b/test/lit/passes/gto-removals.wast index 73756fc7f..f75eb7483 100644 --- a/test/lit/passes/gto-removals.wast +++ b/test/lit/passes/gto-removals.wast @@ -438,7 +438,7 @@ ;; CHECK: (func $new-side-effect (type $none_=>_none) ;; CHECK-NEXT: (local $0 i32) ;; CHECK-NEXT: (local $1 f64) - ;; CHECK-NEXT: (local $2 anyref) + ;; CHECK-NEXT: (local $2 (ref any)) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (block (result (ref $struct)) ;; CHECK-NEXT: (local.set $0 @@ -478,7 +478,7 @@ ;; CHECK: (func $new-side-effect-global-imm (type $none_=>_none) ;; CHECK-NEXT: (local $0 f64) - ;; CHECK-NEXT: (local $1 anyref) + ;; CHECK-NEXT: (local $1 (ref any)) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (block (result (ref $struct)) ;; CHECK-NEXT: (local.set $0 @@ -514,7 +514,7 @@ ;; CHECK: (func $new-side-effect-global-mut (type $none_=>_none) ;; CHECK-NEXT: (local $0 i32) ;; CHECK-NEXT: (local $1 f64) - ;; CHECK-NEXT: (local $2 anyref) + ;; CHECK-NEXT: (local $2 (ref any)) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (block (result (ref $struct)) ;; CHECK-NEXT: (local.set $0 @@ -837,7 +837,7 @@ ) ;; CHECK: (func $unreachable-set-3 (type $none_=>_none) - ;; CHECK-NEXT: (local $0 (ref null ${mut:i8})) + ;; CHECK-NEXT: (local $0 (ref ${mut:i8})) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (ref.as_non_null ;; CHECK-NEXT: (block (result (ref ${mut:i8})) @@ -847,9 +847,7 @@ ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (call $helper-i32) ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (ref.as_non_null - ;; CHECK-NEXT: (local.get $0) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.get $0) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) diff --git a/test/lit/passes/heap2local.wast b/test/lit/passes/heap2local.wast index b7ed9bd0f..6fd744278 100644 --- a/test/lit/passes/heap2local.wast +++ b/test/lit/passes/heap2local.wast @@ -392,8 +392,8 @@ ) ;; CHECK: (func $nondefaultable - ;; CHECK-NEXT: (local $0 (ref null $struct.A)) - ;; CHECK-NEXT: (local $1 (ref null $struct.A)) + ;; CHECK-NEXT: (local $0 (ref $struct.A)) + ;; CHECK-NEXT: (local $1 (ref $struct.A)) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (block (result (ref $struct.A)) ;; CHECK-NEXT: (drop @@ -402,22 +402,18 @@ ;; CHECK-NEXT: (struct.new_default $struct.A) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (local.set $0 - ;; CHECK-NEXT: (ref.as_non_null - ;; CHECK-NEXT: (local.get $1) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.get $1) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (ref.null $struct.nondefaultable) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (ref.as_non_null - ;; CHECK-NEXT: (local.get $0) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.get $0) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; NOMNL: (func $nondefaultable (type $none_=>_none) - ;; NOMNL-NEXT: (local $0 (ref null $struct.A)) - ;; NOMNL-NEXT: (local $1 (ref null $struct.A)) + ;; NOMNL-NEXT: (local $0 (ref $struct.A)) + ;; NOMNL-NEXT: (local $1 (ref $struct.A)) ;; NOMNL-NEXT: (drop ;; NOMNL-NEXT: (block (result (ref $struct.A)) ;; NOMNL-NEXT: (drop @@ -426,23 +422,17 @@ ;; NOMNL-NEXT: (struct.new_default $struct.A) ;; NOMNL-NEXT: ) ;; NOMNL-NEXT: (local.set $0 - ;; NOMNL-NEXT: (ref.as_non_null - ;; NOMNL-NEXT: (local.get $1) - ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: (local.get $1) ;; NOMNL-NEXT: ) ;; NOMNL-NEXT: (ref.null $struct.nondefaultable) ;; NOMNL-NEXT: ) ;; NOMNL-NEXT: ) - ;; NOMNL-NEXT: (ref.as_non_null - ;; NOMNL-NEXT: (local.get $0) - ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: (local.get $0) ;; NOMNL-NEXT: ) ;; NOMNL-NEXT: ) ;; NOMNL-NEXT: ) (func $nondefaultable - ;; We do not optimize structs with nondefaultable types that we cannot - ;; handle. - ;; TODO: We should be able to handle this after #4824 is resolved. + ;; The non-nullable types here can fit in locals. (drop (struct.get $struct.nondefaultable 0 (struct.new $struct.nondefaultable @@ -1459,8 +1449,8 @@ ) ;; CHECK: (func $non-nullable (param $a (ref $struct.A)) - ;; CHECK-NEXT: (local $1 (ref null $struct.A)) - ;; CHECK-NEXT: (local $2 (ref null $struct.A)) + ;; CHECK-NEXT: (local $1 (ref $struct.A)) + ;; CHECK-NEXT: (local $2 (ref $struct.A)) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (block (result (ref $struct.A)) ;; CHECK-NEXT: (drop @@ -1469,22 +1459,18 @@ ;; CHECK-NEXT: (local.get $a) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (local.set $1 - ;; CHECK-NEXT: (ref.as_non_null - ;; CHECK-NEXT: (local.get $2) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.get $2) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (ref.null $struct.nondefaultable) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (ref.as_non_null - ;; CHECK-NEXT: (local.get $1) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.get $1) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; NOMNL: (func $non-nullable (type $ref|$struct.A|_=>_none) (param $a (ref $struct.A)) - ;; NOMNL-NEXT: (local $1 (ref null $struct.A)) - ;; NOMNL-NEXT: (local $2 (ref null $struct.A)) + ;; NOMNL-NEXT: (local $1 (ref $struct.A)) + ;; NOMNL-NEXT: (local $2 (ref $struct.A)) ;; NOMNL-NEXT: (drop ;; NOMNL-NEXT: (block (result (ref $struct.A)) ;; NOMNL-NEXT: (drop @@ -1493,16 +1479,12 @@ ;; NOMNL-NEXT: (local.get $a) ;; NOMNL-NEXT: ) ;; NOMNL-NEXT: (local.set $1 - ;; NOMNL-NEXT: (ref.as_non_null - ;; NOMNL-NEXT: (local.get $2) - ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: (local.get $2) ;; NOMNL-NEXT: ) ;; NOMNL-NEXT: (ref.null $struct.nonnullable) ;; NOMNL-NEXT: ) ;; NOMNL-NEXT: ) - ;; NOMNL-NEXT: (ref.as_non_null - ;; NOMNL-NEXT: (local.get $1) - ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: (local.get $1) ;; NOMNL-NEXT: ) ;; NOMNL-NEXT: ) ;; NOMNL-NEXT: ) @@ -2826,4 +2808,59 @@ ) ) ) + + ;; CHECK: (func $non-nullable-local (result anyref) + ;; CHECK-NEXT: (local $0 (ref null $struct.A)) + ;; CHECK-NEXT: (local $1 i32) + ;; CHECK-NEXT: (local $2 f64) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (block (result (ref null $struct.A)) + ;; CHECK-NEXT: (local.set $1 + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.set $2 + ;; CHECK-NEXT: (f64.const 0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (ref.null $struct.A) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: (ref.as_non_null + ;; CHECK-NEXT: (local.get $0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; NOMNL: (func $non-nullable-local (type $none_=>_anyref) (result anyref) + ;; NOMNL-NEXT: (local $0 (ref null $struct.A)) + ;; NOMNL-NEXT: (local $1 i32) + ;; NOMNL-NEXT: (local $2 f64) + ;; NOMNL-NEXT: (drop + ;; NOMNL-NEXT: (block (result (ref null $struct.A)) + ;; NOMNL-NEXT: (local.set $1 + ;; NOMNL-NEXT: (i32.const 0) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: (local.set $2 + ;; NOMNL-NEXT: (f64.const 0) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: (ref.null $struct.A) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: (unreachable) + ;; NOMNL-NEXT: (ref.as_non_null + ;; NOMNL-NEXT: (local.get $0) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: ) + (func $non-nullable-local (result anyref) + (local $0 (ref $struct.A)) + ;; The local.get here is in unreachable code, which means we won't do + ;; anything to it. But when we remove the local.set during optimization (we + ;; can replace it with new locals for the fields of $struct.A), we must make + ;; sure that validation still passes, that is, since the local.get is + ;; around we must have a local.set for it, or it must become nullable (which + ;; is what the fixup will do). + (local.set $0 + (struct.new_default $struct.A) + ) + (unreachable) + (local.get $0) + ) ) diff --git a/test/lit/passes/inlining_all-features.wast b/test/lit/passes/inlining_all-features.wast index 3b2f3a458..f820d1826 100644 --- a/test/lit/passes/inlining_all-features.wast +++ b/test/lit/passes/inlining_all-features.wast @@ -177,14 +177,12 @@ ;; CHECK: (elem declare func $1) ;; CHECK: (func $1 (result (ref func)) - ;; CHECK-NEXT: (local $0 funcref) + ;; CHECK-NEXT: (local $0 (ref func)) ;; CHECK-NEXT: (block $__inlined_func$0 (result (ref func)) ;; CHECK-NEXT: (local.set $0 ;; CHECK-NEXT: (ref.func $1) ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (ref.as_non_null - ;; CHECK-NEXT: (local.get $0) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.get $0) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; NOMNL: (type $none_=>_ref|func| (func_subtype (result (ref func)) func)) @@ -192,14 +190,12 @@ ;; NOMNL: (elem declare func $1) ;; NOMNL: (func $1 (type $none_=>_ref|func|) (result (ref func)) - ;; NOMNL-NEXT: (local $0 funcref) + ;; NOMNL-NEXT: (local $0 (ref func)) ;; NOMNL-NEXT: (block $__inlined_func$0 (result (ref func)) ;; NOMNL-NEXT: (local.set $0 ;; NOMNL-NEXT: (ref.func $1) ;; NOMNL-NEXT: ) - ;; NOMNL-NEXT: (ref.as_non_null - ;; NOMNL-NEXT: (local.get $0) - ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: (local.get $0) ;; NOMNL-NEXT: ) ;; NOMNL-NEXT: ) (func $1 (result (ref func)) diff --git a/test/lit/passes/inlining_splitting.wast b/test/lit/passes/inlining_splitting.wast index 76062f0c0..aca20f8e8 100644 --- a/test/lit/passes/inlining_splitting.wast +++ b/test/lit/passes/inlining_splitting.wast @@ -213,7 +213,7 @@ ;; CHECK: (func $call-nondefaultable-param ;; CHECK-NEXT: (local $0 i32) - ;; CHECK-NEXT: (local $1 (ref null $struct)) + ;; CHECK-NEXT: (local $1 (ref $struct)) ;; CHECK-NEXT: (block $__inlined_func$nondefaultable-param ;; CHECK-NEXT: (local.set $0 ;; CHECK-NEXT: (i32.const 0) diff --git a/test/lit/passes/local-cse_all-features.wast b/test/lit/passes/local-cse_all-features.wast index 5d9184e8e..fd1af725b 100644 --- a/test/lit/passes/local-cse_all-features.wast +++ b/test/lit/passes/local-cse_all-features.wast @@ -148,22 +148,18 @@ ) ;; CHECK: (func $non-nullable-value (param $ref (ref $A)) - ;; CHECK-NEXT: (local $1 (ref null $A)) + ;; CHECK-NEXT: (local $1 (ref $A)) ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (ref.as_non_null - ;; CHECK-NEXT: (local.tee $1 - ;; CHECK-NEXT: (select (result (ref $A)) - ;; CHECK-NEXT: (local.get $ref) - ;; CHECK-NEXT: (local.get $ref) - ;; CHECK-NEXT: (i32.const 1) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.tee $1 + ;; CHECK-NEXT: (select (result (ref $A)) + ;; CHECK-NEXT: (local.get $ref) + ;; CHECK-NEXT: (local.get $ref) + ;; CHECK-NEXT: (i32.const 1) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (ref.as_non_null - ;; CHECK-NEXT: (local.get $1) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.get $1) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $non-nullable-value (param $ref (ref $A)) diff --git a/test/lit/passes/local-subtyping.wast b/test/lit/passes/local-subtyping.wast index 37990ad24..a2a5780da 100644 --- a/test/lit/passes/local-subtyping.wast +++ b/test/lit/passes/local-subtyping.wast @@ -1,7 +1,12 @@ ;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited. -;; RUN: wasm-opt %s --local-subtyping -all -S -o - \ +;; RUN: wasm-opt %s --remove-unused-names --local-subtyping -all -S -o - \ ;; RUN: | filecheck %s +;; --remove-unused-names is run to avoid adding names to blocks. Block names +;; can prevent non-nullable local validation (we emit named blocks in the binary +;; format, if we need them, but never emit unnamed ones), which affects some +;; testcases. + (module ;; CHECK: (type ${} (struct )) (type ${} (struct_subtype data)) @@ -65,7 +70,7 @@ ;; more specific type. A similar thing with a parameter, however, is not a ;; thing we can optimize. Also, ignore a local with zero assignments. ;; CHECK: (func $simple-local-but-not-param (param $x funcref) - ;; CHECK-NEXT: (local $y (ref null $none_=>_i32)) + ;; CHECK-NEXT: (local $y (ref $none_=>_i32)) ;; CHECK-NEXT: (local $unused funcref) ;; CHECK-NEXT: (local.set $x ;; CHECK-NEXT: (ref.func $i32) @@ -87,9 +92,9 @@ ;; CHECK: (func $locals-with-multiple-assignments (param $data dataref) ;; CHECK-NEXT: (local $x eqref) - ;; CHECK-NEXT: (local $y i31ref) + ;; CHECK-NEXT: (local $y (ref i31)) ;; CHECK-NEXT: (local $z dataref) - ;; CHECK-NEXT: (local $w funcref) + ;; CHECK-NEXT: (local $w (ref func)) ;; CHECK-NEXT: (local.set $x ;; CHECK-NEXT: (i31.new ;; CHECK-NEXT: (i32.const 0) @@ -146,8 +151,9 @@ (local.set $z (local.get $data) ) - ;; w is assigned two different types *without* a new LUB possible, as it - ;; already had the optimal LUB + ;; w is assigned two different types *without* a new LUB heap type possible, + ;; as it already had the optimal LUB heap type (but it can become non- + ;; nullable). (local.set $w (ref.func $i32) ) @@ -159,9 +165,9 @@ ;; In some cases multiple iterations are necessary, as one inferred new type ;; applies to a get which then allows another inference. ;; CHECK: (func $multiple-iterations - ;; CHECK-NEXT: (local $x (ref null $none_=>_i32)) - ;; CHECK-NEXT: (local $y (ref null $none_=>_i32)) - ;; CHECK-NEXT: (local $z (ref null $none_=>_i32)) + ;; CHECK-NEXT: (local $x (ref $none_=>_i32)) + ;; CHECK-NEXT: (local $y (ref $none_=>_i32)) + ;; CHECK-NEXT: (local $z (ref $none_=>_i32)) ;; CHECK-NEXT: (local.set $x ;; CHECK-NEXT: (ref.func $i32) ;; CHECK-NEXT: ) @@ -189,9 +195,9 @@ ;; Sometimes a refinalize is necessary in between the iterations. ;; CHECK: (func $multiple-iterations-refinalize (param $i i32) - ;; CHECK-NEXT: (local $x (ref null $none_=>_i32)) - ;; CHECK-NEXT: (local $y (ref null $none_=>_i64)) - ;; CHECK-NEXT: (local $z funcref) + ;; CHECK-NEXT: (local $x (ref $none_=>_i32)) + ;; CHECK-NEXT: (local $y (ref $none_=>_i64)) + ;; CHECK-NEXT: (local $z (ref func)) ;; CHECK-NEXT: (local.set $x ;; CHECK-NEXT: (ref.func $i32) ;; CHECK-NEXT: ) @@ -199,7 +205,7 @@ ;; CHECK-NEXT: (ref.func $i64) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (local.set $z - ;; CHECK-NEXT: (select (result funcref) + ;; CHECK-NEXT: (select (result (ref func)) ;; CHECK-NEXT: (local.get $x) ;; CHECK-NEXT: (local.get $y) ;; CHECK-NEXT: (local.get $i) @@ -273,13 +279,13 @@ ) ;; CHECK: (func $unreachables (result funcref) - ;; CHECK-NEXT: (local $temp (ref null $none_=>_funcref)) + ;; CHECK-NEXT: (local $temp (ref $none_=>_funcref)) ;; CHECK-NEXT: (local.set $temp ;; CHECK-NEXT: (ref.func $unreachables) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (unreachable) ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (block (result (ref null $none_=>_funcref)) + ;; CHECK-NEXT: (block (result (ref $none_=>_funcref)) ;; CHECK-NEXT: (local.tee $temp ;; CHECK-NEXT: (ref.func $unreachables) ;; CHECK-NEXT: ) @@ -308,7 +314,7 @@ ) ;; CHECK: (func $incompatible-sets (result i32) - ;; CHECK-NEXT: (local $temp (ref null $none_=>_i32)) + ;; CHECK-NEXT: (local $temp (ref $none_=>_i32)) ;; CHECK-NEXT: (local.set $temp ;; CHECK-NEXT: (ref.func $incompatible-sets) ;; CHECK-NEXT: ) @@ -398,4 +404,127 @@ ;; as there is no point to making something less specific in type. (local.set $x (ref.null ${i32})) ) + + ;; CHECK: (func $become-non-nullable + ;; CHECK-NEXT: (local $x (ref $none_=>_none)) + ;; CHECK-NEXT: (local.set $x + ;; CHECK-NEXT: (ref.func $become-non-nullable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $become-non-nullable + (local $x (ref null func)) + (local.set $x + (ref.func $become-non-nullable) + ) + (drop + (local.get $x) + ) + ) + + ;; CHECK: (func $already-non-nullable + ;; CHECK-NEXT: (local $x (ref $none_=>_none)) + ;; CHECK-NEXT: (local.set $x + ;; CHECK-NEXT: (ref.func $already-non-nullable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $already-non-nullable + (local $x (ref func)) + (local.set $x + (ref.func $already-non-nullable) + ) + (drop + (local.get $x) + ) + ) + + ;; CHECK: (func $cannot-become-non-nullable + ;; CHECK-NEXT: (local $x (ref null $none_=>_none)) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: (local.set $x + ;; CHECK-NEXT: (ref.func $become-non-nullable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $cannot-become-non-nullable + (local $x (ref null func)) + ;; The set is in a nested scope, so we should not make the local non- + ;; nullable, as it would not validate. (We can refine the heap type, + ;; though.) + (if + (i32.const 1) + (local.set $x + (ref.func $become-non-nullable) + ) + ) + (drop + (local.get $x) + ) + ) + + ;; CHECK: (func $cannot-become-non-nullable-block + ;; CHECK-NEXT: (local $x (ref null $none_=>_none)) + ;; CHECK-NEXT: (block $name + ;; CHECK-NEXT: (br_if $name + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.set $x + ;; CHECK-NEXT: (ref.func $become-non-nullable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $cannot-become-non-nullable-block + (local $x (ref null func)) + ;; A named block prevents us from optimizing here, the same as above. + (block $name + ;; Add a br_if to avoid the name being removed. + (br_if $name + (i32.const 1) + ) + (local.set $x + (ref.func $become-non-nullable) + ) + ) + (drop + (local.get $x) + ) + ) + + ;; CHECK: (func $become-non-nullable-block-unnamed + ;; CHECK-NEXT: (local $x (ref $none_=>_none)) + ;; CHECK-NEXT: (block + ;; CHECK-NEXT: (local.set $x + ;; CHECK-NEXT: (ref.func $become-non-nullable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $become-non-nullable-block-unnamed + (local $x (ref null func)) + ;; An named block does *not* prevent us from optimizing here. Unlike above, + ;; an unnamed block is never emitted in the binary format, so it does not + ;; prevent validation. + (block + (local.set $x + (ref.func $become-non-nullable) + ) + ) + (drop + (local.get $x) + ) + ) ) diff --git a/test/lit/passes/opt_flatten.wast b/test/lit/passes/opt_flatten.wast index 048ccd60d..0e7ab9a41 100644 --- a/test/lit/passes/opt_flatten.wast +++ b/test/lit/passes/opt_flatten.wast @@ -9,8 +9,8 @@ (export "foo" (func $foo)) ;; CHECK: (func $foo (result funcref) ;; CHECK-NEXT: (local $0 funcref) - ;; CHECK-NEXT: (local $1 (ref null $none_=>_funcref)) - ;; CHECK-NEXT: (local $2 (ref null $none_=>_funcref)) + ;; CHECK-NEXT: (local $1 (ref $none_=>_funcref)) + ;; CHECK-NEXT: (local $2 (ref $none_=>_funcref)) ;; CHECK-NEXT: (local $3 i32) ;; CHECK-NEXT: (block ;; CHECK-NEXT: (local.set $0 @@ -23,14 +23,10 @@ ;; CHECK-NEXT: (ref.func $foo) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (local.set $2 - ;; CHECK-NEXT: (ref.as_non_null - ;; CHECK-NEXT: (local.get $1) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.get $1) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (return - ;; CHECK-NEXT: (ref.as_non_null - ;; CHECK-NEXT: (local.get $2) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.get $2) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) diff --git a/test/lit/passes/precompute-gc.wast b/test/lit/passes/precompute-gc.wast index 161b1fe07..dd864eee1 100644 --- a/test/lit/passes/precompute-gc.wast +++ b/test/lit/passes/precompute-gc.wast @@ -1355,4 +1355,62 @@ ) ) ) + + ;; CHECK: (func $remove-set (result (ref func)) + ;; CHECK-NEXT: (local $nn funcref) + ;; CHECK-NEXT: (local $i i32) + ;; CHECK-NEXT: (loop $loop + ;; CHECK-NEXT: (local.set $i + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (br $loop) + ;; CHECK-NEXT: (return + ;; CHECK-NEXT: (ref.as_non_null + ;; CHECK-NEXT: (local.get $nn) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; NOMNL: (func $remove-set (type $none_=>_ref|func|) (result (ref func)) + ;; NOMNL-NEXT: (local $nn funcref) + ;; NOMNL-NEXT: (local $i i32) + ;; NOMNL-NEXT: (loop $loop + ;; NOMNL-NEXT: (local.set $i + ;; NOMNL-NEXT: (i32.const 0) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: (br $loop) + ;; NOMNL-NEXT: (return + ;; NOMNL-NEXT: (ref.as_non_null + ;; NOMNL-NEXT: (local.get $nn) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: ) + (func $remove-set (result (ref func)) + (local $nn (ref func)) + (local $i i32) + (loop $loop + ;; Add a local.set here in the loop, just so the entire loop is not optimized + ;; out. + (local.set $i + (i32.const 0) + ) + ;; This entire block can be precomputed into an unconditional br. That + ;; removes the local.set, which means the local no longer validates since + ;; there is a get without a set (the get is never reached, but the validator + ;; does not take that into account). Fixups will turn the local nullable to + ;; avoid that problem. + (block + (br_if $loop + (i32.const 1) + ) + (local.set $nn + (ref.func $remove-set) + ) + ) + (return + (local.get $nn) + ) + ) + ) ) diff --git a/test/lit/passes/roundtrip-gc.wast b/test/lit/passes/roundtrip-gc.wast index e0b59fe41..2692b7b99 100644 --- a/test/lit/passes/roundtrip-gc.wast +++ b/test/lit/passes/roundtrip-gc.wast @@ -8,33 +8,38 @@ ;; NOMNL: (export "export" (func $test)) (export "export" (func $test)) ;; CHECK: (func $test + ;; CHECK-NEXT: (local $0 (ref $\7bi32\7d)) ;; CHECK-NEXT: (call $help - ;; CHECK-NEXT: (struct.new_default $\7bi32\7d) - ;; CHECK-NEXT: (block $label$1 (result i32) + ;; CHECK-NEXT: (block (result (ref $\7bi32\7d)) + ;; CHECK-NEXT: (local.set $0 + ;; CHECK-NEXT: (struct.new_default $\7bi32\7d) + ;; CHECK-NEXT: ) ;; CHECK-NEXT: (nop) - ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: (local.get $0) ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (i32.const 1) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; NOMNL: (func $test (type $none_=>_none) + ;; NOMNL-NEXT: (local $0 (ref $\7bi32\7d)) ;; NOMNL-NEXT: (call $help - ;; NOMNL-NEXT: (struct.new_default $\7bi32\7d) - ;; NOMNL-NEXT: (block $label$1 (result i32) + ;; NOMNL-NEXT: (block (result (ref $\7bi32\7d)) + ;; NOMNL-NEXT: (local.set $0 + ;; NOMNL-NEXT: (struct.new_default $\7bi32\7d) + ;; NOMNL-NEXT: ) ;; NOMNL-NEXT: (nop) - ;; NOMNL-NEXT: (i32.const 1) + ;; NOMNL-NEXT: (local.get $0) ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: (i32.const 1) ;; NOMNL-NEXT: ) ;; NOMNL-NEXT: ) (func $test (call $help (struct.new_default ${i32}) ;; Stack IR optimizations can remove this block, leaving a nop in an odd - ;; "stacky" location. On load, we would normally use a local to work around - ;; that, creating a block to contain the non-nullable reference before us and - ;; the nop, and then returning the local. But we can't use a local for a - ;; non-nullable reference, so we should not optimize this sort of thing in - ;; stack IR. - ;; TODO: This shouldn't be true after #4824 is resolved. + ;; "stacky" location. On load, we will use a local to work around that. It + ;; is fine for the local to be non-nullable since the get is later in that + ;; same block. (block $block (result i32) (nop) (i32.const 1) diff --git a/test/lit/passes/simplify-locals-gc-nn.wast b/test/lit/passes/simplify-locals-gc-nn.wast index 814350fc0..e5941cc0e 100644 --- a/test/lit/passes/simplify-locals-gc-nn.wast +++ b/test/lit/passes/simplify-locals-gc-nn.wast @@ -1,33 +1,34 @@ ;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited. -;; RUN: wasm-opt %s --simplify-locals -all --enable-gc-nn-locals -S -o - | filecheck %s +;; RUN: wasm-opt %s --simplify-locals -all -S -o - | filecheck %s (module ;; CHECK: (func $test-nn - ;; CHECK-NEXT: (local $nn (ref any)) - ;; CHECK-NEXT: (local.set $nn - ;; CHECK-NEXT: (ref.as_non_null - ;; CHECK-NEXT: (ref.null any) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local $nn anyref) + ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: (try $try ;; CHECK-NEXT: (do - ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (local.get $nn) + ;; CHECK-NEXT: (local.set $nn + ;; CHECK-NEXT: (ref.as_non_null + ;; CHECK-NEXT: (ref.null any) + ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (catch_all ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (local.get $nn) + ;; CHECK-NEXT: (ref.as_non_null + ;; CHECK-NEXT: (local.get $nn) + ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $test-nn (local $nn (ref any)) - ;; We should not sink this set into the try, as the spec does not allow it - ;; even though we are not changing dominance (we are not changing it, - ;; because there is nothing that can throw in the try's body that can reach - ;; the catch_all before the local.set that we move there). See + ;; We can sink this set into the try, but the spec does not allow it to + ;; remain non-nullable. Even though we are not changing dominance (we are + ;; not changing it, because there is nothing that can throw in the try's + ;; body that can reach the catch_all before the local.set that we move + ;; there). See ;; https://github.com/WebAssembly/function-references/issues/44#issuecomment-1083146887 (local.set $nn (ref.as_non_null @@ -67,8 +68,8 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $test-nullable - ;; As above, but now the local is nullable. Here we *can* optimize the set - ;; into the try. + ;; As above, but now the local is nullable. Here we can optimize the set + ;; into the try, with no other necessary changes. (local $nullable (ref null any)) (local.set $nullable (ref.as_non_null @@ -88,70 +89,4 @@ ) ) ) - - ;; CHECK: (func $test-nested-nn - ;; CHECK-NEXT: (local $nullable anyref) - ;; CHECK-NEXT: (local $nn (ref any)) - ;; CHECK-NEXT: (local.set $nullable - ;; CHECK-NEXT: (block (result (ref any)) - ;; CHECK-NEXT: (local.set $nn - ;; CHECK-NEXT: (ref.as_non_null - ;; CHECK-NEXT: (ref.null any) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (ref.as_non_null - ;; CHECK-NEXT: (ref.null any) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (try $try - ;; CHECK-NEXT: (do - ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (local.get $nullable) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (catch_all - ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (local.get $nullable) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (local.get $nn) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - (func $test-nested-nn - ;; As above, the set we want to move is nullable, but now it has a nested - ;; value that is a non-nullable set. That should also prevent us from - ;; moving it. - (local $nullable (ref null any)) - (local $nn (ref any)) - (local.set $nullable - (block (result (ref any)) - (local.set $nn - (ref.as_non_null - (ref.null any) - ) - ) - (ref.as_non_null - (ref.null any) - ) - ) - ) - (try - (do - (drop - (local.get $nullable) - ) - ) - (catch_all - (drop - (local.get $nullable) - ) - (drop - (local.get $nn) - ) - ) - ) - ) ) diff --git a/test/lit/passes/simplify-locals-gc-validation.wast b/test/lit/passes/simplify-locals-gc-validation.wast new file mode 100644 index 000000000..bda59882b --- /dev/null +++ b/test/lit/passes/simplify-locals-gc-validation.wast @@ -0,0 +1,48 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited. +;; RUN: wasm-opt %s --simplify-locals -all -S -o - | filecheck %s + +;; Tests for validation of non-nullable locals. In this form a local.set allows +;; a local.get until the end of the current block. + +(module + ;; CHECK: (func $test-nn (param $x (ref any)) + ;; CHECK-NEXT: (local $nn anyref) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (block $inner + ;; CHECK-NEXT: (call $test-nn + ;; CHECK-NEXT: (ref.as_non_null + ;; CHECK-NEXT: (local.tee $nn + ;; CHECK-NEXT: (ref.as_non_null + ;; CHECK-NEXT: (ref.null any) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (call $test-nn + ;; CHECK-NEXT: (ref.as_non_null + ;; CHECK-NEXT: (local.get $nn) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test-nn (param $x (ref any)) + (local $nn (ref any)) + ;; We can sink this set into the block, but we should then update things so + ;; that we still validate, as then the final local.get is not structurally + ;; dominated. (Note that we end up with several ref.as_non_nulls here, but + ;; later passes could remove them.) + (local.set $nn + (ref.as_non_null + (ref.null any) + ) + ) + (block $inner + (call $test-nn + (local.get $nn) + ) + ) + (call $test-nn + (local.get $nn) + ) + ) +) diff --git a/test/lit/passes/simplify-locals-gc.wast b/test/lit/passes/simplify-locals-gc.wast index 821f0a862..8d15c213e 100644 --- a/test/lit/passes/simplify-locals-gc.wast +++ b/test/lit/passes/simplify-locals-gc.wast @@ -226,6 +226,145 @@ ) ) + ;; CHECK: (func $if-nnl + ;; CHECK-NEXT: (local $x (ref func)) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: (local.set $x + ;; CHECK-NEXT: (ref.func $if-nnl) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (call $helper + ;; CHECK-NEXT: (local.tee $x + ;; CHECK-NEXT: (ref.func $if-nnl) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (call $helper + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; NOMNL: (func $if-nnl (type $none_=>_none) + ;; NOMNL-NEXT: (local $x (ref func)) + ;; NOMNL-NEXT: (if + ;; NOMNL-NEXT: (i32.const 1) + ;; NOMNL-NEXT: (local.set $x + ;; NOMNL-NEXT: (ref.func $if-nnl) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: (call $helper + ;; NOMNL-NEXT: (local.tee $x + ;; NOMNL-NEXT: (ref.func $if-nnl) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: (call $helper + ;; NOMNL-NEXT: (local.get $x) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: ) + (func $if-nnl + (local $x (ref func)) + ;; We want to turn this if into an if-else with a set on the outside: + ;; + ;; (local.set $x + ;; (if + ;; (i32.const 1) + ;; (ref.func $if-nnl) + ;; (local.get $x))) + ;; + ;; That will not validate, however (no set dominates the get), so we'll get + ;; fixed up by adding a ref.as_non_null. But that may be dangerous - if no + ;; set exists before us, then that new instruction will trap, in fact. So we + ;; do not optimize here. + (if + (i32.const 1) + (local.set $x + (ref.func $if-nnl) + ) + ) + ;; An exta set + gets, just to avoid other optimizations kicking in + ;; (without them, the function only has a set and nothing else, and will + ;; remove the set entirely). Nothing should change here. + (call $helper + (local.tee $x + (ref.func $if-nnl) + ) + ) + (call $helper + (local.get $x) + ) + ) + + ;; CHECK: (func $if-nnl-previous-set + ;; CHECK-NEXT: (local $x (ref func)) + ;; CHECK-NEXT: (local.set $x + ;; CHECK-NEXT: (ref.func $if-nnl) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: (local.set $x + ;; CHECK-NEXT: (ref.func $if-nnl) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (call $helper + ;; CHECK-NEXT: (local.tee $x + ;; CHECK-NEXT: (ref.func $if-nnl) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (call $helper + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; NOMNL: (func $if-nnl-previous-set (type $none_=>_none) + ;; NOMNL-NEXT: (local $x (ref func)) + ;; NOMNL-NEXT: (local.set $x + ;; NOMNL-NEXT: (ref.func $if-nnl) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: (if + ;; NOMNL-NEXT: (i32.const 1) + ;; NOMNL-NEXT: (local.set $x + ;; NOMNL-NEXT: (ref.func $if-nnl) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: (call $helper + ;; NOMNL-NEXT: (local.tee $x + ;; NOMNL-NEXT: (ref.func $if-nnl) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: (call $helper + ;; NOMNL-NEXT: (local.get $x) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: ) + (func $if-nnl-previous-set + (local $x (ref func)) + ;; As the above testcase, but now there is a set before the if. We could + ;; optimize in this case, but don't atm. TODO + (local.set $x + (ref.func $if-nnl) + ) + (if + (i32.const 1) + (local.set $x + (ref.func $if-nnl) + ) + ) + (call $helper + (local.tee $x + (ref.func $if-nnl) + ) + ) + (call $helper + (local.get $x) + ) + ) + + ;; CHECK: (func $helper (param $ref (ref func)) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: ) + ;; NOMNL: (func $helper (type $ref|func|_=>_none) (param $ref (ref func)) + ;; NOMNL-NEXT: (nop) + ;; NOMNL-NEXT: ) + (func $helper (param $ref (ref func)) + ) + ;; CHECK: (func $needs-refinalize (param $b (ref $B)) (result anyref) ;; CHECK-NEXT: (local $a (ref null $A)) ;; CHECK-NEXT: (nop) diff --git a/test/lit/passes/ssa.wast b/test/lit/passes/ssa.wast index 30640a803..420761e40 100644 --- a/test/lit/passes/ssa.wast +++ b/test/lit/passes/ssa.wast @@ -8,23 +8,19 @@ (func $foo) ;; CHECK: (func $bar (param $x (ref func)) - ;; CHECK-NEXT: (local $1 funcref) - ;; CHECK-NEXT: (local $2 funcref) + ;; CHECK-NEXT: (local $1 (ref func)) + ;; CHECK-NEXT: (local $2 (ref func)) ;; CHECK-NEXT: (local.set $1 ;; CHECK-NEXT: (ref.func $foo) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (ref.as_non_null - ;; CHECK-NEXT: (local.get $1) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.get $1) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (local.set $2 ;; CHECK-NEXT: (ref.func $bar) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (ref.as_non_null - ;; CHECK-NEXT: (local.get $2) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.get $2) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $bar (param $x (ref func)) diff --git a/test/lit/validation/bad-non-nullable-locals.wast b/test/lit/validation/bad-non-nullable-locals.wast new file mode 100644 index 000000000..f22771591 --- /dev/null +++ b/test/lit/validation/bad-non-nullable-locals.wast @@ -0,0 +1,61 @@ +;; RUN: foreach %s %t not wasm-opt -all 2>&1 | filecheck %s + +;; CHECK: non-nullable local's sets must dominate gets +(module + (func $inner-to-func + ;; a set in an inner scope does *not* help a get validate. + (local $x (ref func)) + (block $b + (local.set $x + (ref.func $helper) + ) + ) + (drop + (local.get $x) + ) + ) + + (func $helper) +) + +;; CHECK: non-nullable local's sets must dominate gets +(module + (func $get-without-set + (local $x (ref func)) + (drop + (local.get $x) + ) + ) + + (func $helper) +) + +;; CHECK: non-nullable local's sets must dominate gets +(module + (func $get-before-set + (local $x (ref func)) + (local.set $x + (local.get $x) + ) + ) + + (func $helper) +) + +;; CHECK: non-nullable local's sets must dominate gets +(module + (func $if-arms + (local $x (ref func)) + (if + (i32.const 1) + ;; Superficially the order is right, but not really. + (local.set $x + (ref.func $helper) + ) + (local.get $x) + ) + ) + + (func $helper) +) + diff --git a/test/lit/validation/nn-tuples.wast b/test/lit/validation/nn-tuples.wast index 452a6c77e..5ecf55fbd 100644 --- a/test/lit/validation/nn-tuples.wast +++ b/test/lit/validation/nn-tuples.wast @@ -1,16 +1,15 @@ -;; Test for non-nullable types in tuples - -;; RUN: not wasm-opt -all %s 2>&1 | filecheck %s --check-prefix NO-NN-LOCALS -;; RUN: wasm-opt -all %s --enable-gc-nn-locals -o - -S | filecheck %s --check-prefix NN-LOCALS +;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited. -;; NO-NN-LOCALS: vars must be defaultable +;; RUN: wasm-opt %s -all -S -o - \ +;; RUN: | filecheck %s -;; NN-LOCALS: (module -;; NN-LOCALS: (local $tuple ((ref any) (ref any))) -;; NN-LOCALS: (nop) -;; NN-LOCALS: ) +;; Test for non-nullable types in tuples (module + ;; CHECK: (func $foo + ;; CHECK-NEXT: (local $tuple ((ref any) (ref any))) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: ) (func $foo (local $tuple ((ref any) (ref any))) ) diff --git a/test/lit/wat-kitchen-sink.wast b/test/lit/wat-kitchen-sink.wast index a8a9ea42d..09c983c5d 100644 --- a/test/lit/wat-kitchen-sink.wast +++ b/test/lit/wat-kitchen-sink.wast @@ -9,7 +9,7 @@ ;; CHECK: (type $i32_=>_none (func_subtype (param i32) func)) - ;; CHECK: (rec + ;; CHECK: (rec ;; CHECK-NEXT: (type $s0 (struct_subtype data)) (type $s0 (sub (struct))) ;; CHECK: (type $s1 (struct_subtype data)) diff --git a/test/passes/Oz_fuzz-exec_all-features.txt b/test/passes/Oz_fuzz-exec_all-features.txt index 981e8f8d2..b36e6c63e 100644 --- a/test/passes/Oz_fuzz-exec_all-features.txt +++ b/test/passes/Oz_fuzz-exec_all-features.txt @@ -121,7 +121,7 @@ ) ) (func $1 (; has Stack IR ;) - (local $0 (ref null $bytes)) + (local $0 (ref $bytes)) (call $log (array.len $bytes (local.tee $0 @@ -168,7 +168,7 @@ ) ) (func $3 (; has Stack IR ;) - (local $0 (ref null $struct)) + (local $0 (ref $struct)) (local.set $0 (struct.new_default $struct) ) @@ -262,8 +262,8 @@ ) ) (func $23 (; has Stack IR ;) - (local $0 (ref null $bytes)) - (local $1 (ref null $bytes)) + (local $0 (ref $bytes)) + (local $1 (ref $bytes)) (array.set $bytes (local.tee $1 (array.new_default $bytes @@ -317,7 +317,7 @@ ) ) (func $24 (; has Stack IR ;) - (local $0 (ref null $bytes)) + (local $0 (ref $bytes)) (call $log (array.len $bytes (local.tee $0 diff --git a/test/spec/tuples.wast b/test/spec/tuples.wast deleted file mode 100644 index 83a30aa35..000000000 --- a/test/spec/tuples.wast +++ /dev/null @@ -1,9 +0,0 @@ -(assert_invalid - (module - (func $foo - (local $temp ((ref func) i32)) - ) - ) - "var must be defaultable" -) - |