diff --git a/src/passes/DeadArgumentElimination2.cpp b/src/passes/DeadArgumentElimination2.cpp index 6fd7adfd893..a82401ce02e 100644 --- a/src/passes/DeadArgumentElimination2.cpp +++ b/src/passes/DeadArgumentElimination2.cpp @@ -32,10 +32,9 @@ // functions replacement types to make sure they are not incorrectly updated by // the global type rewriting. // -// As a POC, only do the backward analysis to find unused parameters. To match -// and exceed the power of DAE, we will need to extend this backward analysis to -// find unused results as well, and also add a forward analysis that propagates -// constants and types through parameters and results. +// As a POC, only do the backward analysis to find unused parameters and +// results. To match and exceed the power of DAE, we will need to add a forward +// analysis that propagates constants and types through parameters and results. #include #include @@ -50,6 +49,7 @@ #include "ir/local-graph.h" #include "ir/module-utils.h" #include "ir/type-updating.h" +#include "ir/utils.h" #include "pass.h" #include "support/index.h" #include "support/mixed_arena.h" @@ -204,6 +204,11 @@ struct FunctionInfo { // Root function types that this function indirectly tail-calls. std::vector tailCalleeTypes; + + // Replacing a removed expression with its kept children can change its type + // from none to unreachable, requiring enclosing expressions to be + // refinalized. + bool refinalize = false; }; // Analysis results and call graph information for a tree of related function @@ -992,10 +997,10 @@ struct Optimizer // types to match. Function types are updated after this pass runs. bool requiresNonNullableLocalFixups() override { return false; } - const DAE2& parent; + DAE2& parent; // The info for the function we are running on. - const FunctionInfo* funcInfo = nullptr; + FunctionInfo* funcInfo = nullptr; // Map old local indices to new local indices for the function we are // currently optimizing. Kept parameters and locals may need to have their @@ -1015,7 +1020,16 @@ struct Optimizer // We will need to generate fresh labels for trampoline blocks. std::optional labels; - Optimizer(const DAE2& parent) : parent(parent) {} + Optimizer(DAE2& parent) : parent(parent) {} + + void replaceCurrent(Expression* rep) { + // If we introduce any new unreachability, we will have to refinalize. + if (rep->type == Type::unreachable && + getCurrent()->type != Type::unreachable) { + funcInfo->refinalize = true; + } + Super::replaceCurrent(rep); + } std::unique_ptr create() override { return std::make_unique(parent); @@ -1573,37 +1587,63 @@ Expression* Optimizer::getReplacement(Expression* curr) { return collector.collectExprsOrNull(); } +struct Refinalizer : public ReFinalize { + const DAE2& parent; + + Refinalizer(const DAE2& parent) : parent(parent) {} + + std::unique_ptr create() override { + return std::make_unique(parent); + } + + void runOnFunction(Module* wasm, Function* func) override { + if (parent.funcInfos[parent.funcIndices.at(func->name)].refinalize) { + ReFinalize::runOnFunction(wasm, func); + } + } +}; + void DAE2::optimize() { + TIME(Timer timer); + Optimizer optimizer(*this); if (!optimizeReferencedFuncs) { // Cannot globally rewrite types or optimize referenced functions, so just // run the optimizer to optimize unreferenced functions. optimizer.run(getPassRunner(), wasm); - return; - } - TIME(Timer timer); + TIME(std::cerr << " optimize: " << timer.lastElapsed() << "\n"); + } else { + // Global type rewriting + DAETypeUpdater typeUpdater(*this); + auto [oldTypes, newTypes] = typeUpdater.rebuildTypes(); - // Global type rewriting - DAETypeUpdater typeUpdater(*this); - auto [oldTypes, newTypes] = typeUpdater.rebuildTypes(); + TIME(std::cerr << " rebuild types: " << timer.lastElapsed() << "\n"); - TIME(std::cerr << " rebuild types: " << timer.lastElapsed() << "\n"); + makeUnreferencedFunctionTypes(oldTypes, newTypes); - makeUnreferencedFunctionTypes(oldTypes, newTypes); + TIME(std::cerr << " make replacements: " << timer.lastElapsed() << "\n"); - TIME(std::cerr << " make replacements: " << timer.lastElapsed() << "\n"); + optimizer.run(getPassRunner(), wasm); + + TIME(std::cerr << " optimize: " << timer.lastElapsed() << "\n"); - optimizer.run(getPassRunner(), wasm); + // Update the types for referenced functions and all the locations that + // might hold them. The types of non-referenced functions have already been + // updated separately. + typeUpdater.mapTypes(newTypes); - TIME(std::cerr << " optimize: " << timer.lastElapsed() << "\n"); + TIME(std::cerr << " update types: " << timer.lastElapsed() << "\n"); + } - // Update the types for referenced functions and all the locations that might - // hold them. The types of non-referenced functions have already been updated - // separately. - typeUpdater.mapTypes(newTypes); + // Refinalize after global type rewriting rather than in + // Optimizer::runOnFunction so that indirect calls whose results were removed + // do not have their types reset to the original signature results, and so + // that ref.func instructions see the final rewritten function types rather + // than temporary scratch types (or race on concurrent function type updates). + Refinalizer(*this).run(getPassRunner(), wasm); - TIME(std::cerr << " update types: " << timer.lastElapsed() << "\n"); + TIME(std::cerr << " refinalize: " << timer.lastElapsed() << "\n"); } // Generate new (possibly optimized) types for each unreferenced function so diff --git a/test/lit/passes/dae2-results-control-flow.wast b/test/lit/passes/dae2-results-control-flow.wast index 32baff3f72e..586971a01e0 100644 --- a/test/lit/passes/dae2-results-control-flow.wast +++ b/test/lit/passes/dae2-results-control-flow.wast @@ -279,3 +279,168 @@ (global.set $g (call $test)) ) ) + +(module + ;; CHECK: (func $callee (type $0) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $callee (result f32) + (unreachable) + ) + ;; CHECK: (func $caller (type $2) (param $x i32) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (block + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: (call $callee) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $caller (param $x i32) + ;; Removing the unused result of the inner block makes it unreachable, which + ;; in turn makes the enclosing if unreachable. Ensure we refinalize the + ;; function so enclosing expressions have updated types. + (drop + (block (result i32) + (if + (local.get $x) + (then + (drop + (block (result f32) + (unreachable) + (call $callee) + ) + ) + ) + (else + (unreachable) + ) + ) + (unreachable) + ) + ) + ) +) + +(module + ;; CHECK: (type $sig (func)) + (type $sig (func (result f32))) + ;; CHECK: (func $callee (type $0) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $callee (result f32) + (unreachable) + ) + ;; CHECK: (func $caller (type $2) (param $x i32) (param $ref (ref $sig)) + ;; CHECK-NEXT: (call_ref $sig + ;; CHECK-NEXT: (local.get $ref) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (block + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: (call $callee) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $caller (param $x i32) (param $ref (ref $sig)) + ;; Refinalizing must happen after global type rewriting so that the call_ref + ;; whose result was removed does not have its type reset to the old concrete + ;; result type. That would cause a validation failure because the `drop` + ;; consuming that concrete type will have been removed. + (drop + (call_ref $sig + (local.get $ref) + ) + ) + (drop + (block (result i32) + (if + (local.get $x) + (then + (drop + (block (result f32) + (unreachable) + (call $callee) + ) + ) + ) + (else + (unreachable) + ) + ) + (unreachable) + ) + ) + ) +) + +(module + ;; CHECK: (rec + ;; CHECK-NEXT: (type $super (sub (func))) + (type $super (sub (func (result f32)))) + ;; CHECK: (type $sub (sub $super (func))) + (type $sub (sub $super (func (result f32)))) + ;; CHECK: (func $f (type $sub) + ;; CHECK-NEXT: (local $x (ref null $super)) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (block + ;; CHECK-NEXT: (local.set $x + ;; CHECK-NEXT: (ref.func $f) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: (call $f) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $f (type $sub) (result f32) + ;; Refinalizing must happen after global type rewriting so that the ref.func + ;; gets the final rewritten subtype rather than the temporary type assigned + ;; in Optimizer::runOnFunction. If it were to be refinalized with the + ;; temporary type, there would be a validation failure where we assign the + ;; `ref.func` to local `$x` because the temporary type is not a subtype of + ;; $super. + (local $x (ref null $super)) + (local.set $x (ref.func $f)) + (if + (i32.const 0) + (then + (drop + (block (result f32) + (unreachable) + (call $f) + ) + ) + ) + (else + (unreachable) + ) + ) + (unreachable) + ) +)