diff --git a/src/passes/RemoveUnusedModuleElements.cpp b/src/passes/RemoveUnusedModuleElements.cpp index f35e750d612..1743863fe0a 100644 --- a/src/passes/RemoveUnusedModuleElements.cpp +++ b/src/passes/RemoveUnusedModuleElements.cpp @@ -43,6 +43,7 @@ #include "ir/find_all.h" #include "ir/intrinsics.h" #include "ir/module-utils.h" +#include "ir/names.h" #include "ir/struct-utils.h" #include "ir/subtypes.h" #include "ir/table-utils.h" @@ -949,6 +950,78 @@ struct RemoveUnusedModuleElements : public Pass { auto& options = getPassOptions(); Analyzer analyzer(module, options, roots); + // Functions in element segments that are referenced but not used can + // never be called successfully: any call_indirect reaching their slot + // traps on the signature check. Point all such entries at one shared + // stub function whose body is unreachable, so a matching call traps in + // the stub instead - a trap either way, which is an equivalence we + // already rely on. The redirected functions may then become entirely + // unreferenced, letting us remove their bodies rather than keeping one + // emptied-out function per signature. See issue #3029. + // + // We only do this when: + // * The entry's target is not the reason the segment is kept (the + // function is referenced, not used). + // * The segment's table is neither imported nor exported: an outside + // view of the table could observe the changed entry identity. + // * GC is not enabled: with GC, a table.get plus ref.test could + // observe the changed type of the entry. (Emptying a function in + // place preserves its type; redirecting does not.) + std::unordered_set redirected; + if (!module->features.hasGC()) { + std::unordered_set exportedTables; + for (auto& ex : module->exports) { + if (ex->kind == ExternalKind::Table) { + if (auto* name = ex->getInternalName()) { + exportedTables.insert(*name); + } + } + } + auto stubHeapType = HeapType(Signature(Type::none, Type::none)); + auto stubRefType = Type(stubHeapType, NonNullable); + Name stubName; + for (auto& segment : module->elementSegments) { + auto segmentElement = + ModuleElement{ModuleElementKind::ElementSegment, segment->name}; + bool kept = analyzer.used.contains(segmentElement) || + analyzer.referenced.contains(segmentElement); + if (!kept || segment->isPassive() || + !Type::isSubType(stubRefType, segment->type)) { + continue; + } + auto* table = module->getTable(segment->table); + if (table->imported() || exportedTables.count(table->name)) { + continue; + } + for (auto*& item : segment->data) { + auto* refFunc = item->dynCast(); + if (!refFunc) { + continue; + } + auto target = + ModuleElement{ModuleElementKind::Function, refFunc->func}; + if (analyzer.used.contains(target) || + !analyzer.referenced.contains(target)) { + continue; + } + if (!stubName) { + // Create the shared stub lazily, with the simplest signature. + stubName = Names::getValidFunctionName(*module, "trap-stub"); + module->addFunction(Builder(*module).makeFunction( + stubName, + Signature(Type::none, Type::none), + {}, + Builder(*module).makeUnreachable())); + analyzer.used.insert({ModuleElementKind::Function, stubName}); + } + redirected.insert(refFunc->func); + // The one-argument makeRefFunc computes the proper (exact) type + // from the stub function we added to the module above. + item = Builder(*module).makeRefFunc(stubName); + } + } + } + // Remove unneeded elements. auto needed = [&](ModuleElement element) { // We need to emit something in the output if it has either a reference or @@ -995,6 +1068,46 @@ struct RemoveUnusedModuleElements : public Pass { module->removeElementSegments([&](ElementSegment* curr) { return !needed({ModuleElementKind::ElementSegment, curr->name}); }); + + // Redirecting element segment entries above may have removed the last + // reference to a function (removeFunctions kept it, and emptied it out, + // based on the analysis from before the redirect). Find which of the + // redirected functions are still referred to from the remaining module + // contents, and remove the others entirely. + if (!redirected.empty()) { + std::unordered_set stillReferenced; + auto scanExpr = [&](Expression* expr) { + if (!expr) { + return; + } + for (auto* refFunc : FindAll(expr).list) { + stillReferenced.insert(refFunc->func); + } + }; + for (auto& func : module->functions) { + if (!func->imported()) { + scanExpr(func->body); + } + } + for (auto& global : module->globals) { + if (!global->imported()) { + scanExpr(global->init); + } + } + for (auto& segment : module->elementSegments) { + scanExpr(segment->offset); + for (auto* item : segment->data) { + scanExpr(item); + } + } + for (auto& table : module->tables) { + scanExpr(table->init); + } + module->removeFunctions([&](Function* curr) { + return redirected.count(curr->name) && + !stillReferenced.count(curr->name); + }); + } // TODO: After removing elements, we may be able to remove more things, and // should continue to work. (For example, after removing a reference // to a function from an element segment, we may be able to remove diff --git a/test/lit/passes/remove-unused-module-elements-table-stub-gc.wast b/test/lit/passes/remove-unused-module-elements-table-stub-gc.wast new file mode 100644 index 00000000000..9986daea5e8 --- /dev/null +++ b/test/lit/passes/remove-unused-module-elements-table-stub-gc.wast @@ -0,0 +1,51 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. + +;; RUN: foreach %s %t wasm-opt --remove-unused-module-elements --closed-world -all -S -o - | filecheck %s + +(module + ;; With GC enabled we do not redirect never-called entries to a shared trap + ;; stub: a table.get plus ref.test could observe the changed type of an + ;; entry. The never-called functions are kept, with emptied bodies. + (table 3 3 funcref) + (elem (i32.const 0) $called $dead1 $dead2) + ;; CHECK: (type $0 (func (result i32))) + + ;; CHECK: (type $1 (func (param i32))) + + ;; CHECK: (type $2 (func (result f64))) + + ;; CHECK: (type $3 (func (param i32) (result i32))) + + ;; CHECK: (table $0 3 3 funcref) + + ;; CHECK: (elem $0 (i32.const 0) $called $dead1 $dead2) + + ;; CHECK: (export "main" (func $main)) + + ;; CHECK: (func $called (type $0) (result i32) + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + (func $called (result i32) + (i32.const 1) + ) + ;; CHECK: (func $dead1 (type $1) (param $0 i32) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $dead1 (param i32) + (unreachable) + ) + ;; CHECK: (func $dead2 (type $2) (result f64) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $dead2 (result f64) + (f64.const 3.14) + ) + ;; CHECK: (func $main (type $3) (param $0 i32) (result i32) + ;; CHECK-NEXT: (call_indirect $0 (type $0) + ;; CHECK-NEXT: (local.get $0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $main (export "main") (param i32) (result i32) + (call_indirect (result i32) (local.get 0)) + ) +) diff --git a/test/lit/passes/remove-unused-module-elements-table-stub.wast b/test/lit/passes/remove-unused-module-elements-table-stub.wast new file mode 100644 index 00000000000..321c8de2d65 --- /dev/null +++ b/test/lit/passes/remove-unused-module-elements-table-stub.wast @@ -0,0 +1,198 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. + +;; RUN: foreach %s %t wasm-opt --remove-unused-module-elements --closed-world --enable-reference-types -S -o - | filecheck %s + +(module + ;; $dead1 and $dead2 can never be called: no call_indirect of a matching + ;; signature exists. Their element entries are redirected to a single shared + ;; trap stub (any call reaching those slots traps either way, on the + ;; signature check or in the stub), and the functions themselves are then + ;; removed entirely. + (table 3 3 funcref) + (elem (i32.const 0) $called $dead1 $dead2) + ;; CHECK: (type $0 (func (result i32))) + + ;; CHECK: (type $1 (func (param i32) (result i32))) + + ;; CHECK: (type $2 (func)) + + ;; CHECK: (table $0 3 3 funcref) + + ;; CHECK: (elem $0 (i32.const 0) $called $trap-stub $trap-stub) + + ;; CHECK: (export "main" (func $main)) + + ;; CHECK: (func $called (result i32) + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + (func $called (result i32) + (i32.const 1) + ) + (func $dead1 (param i32) + (unreachable) + ) + (func $dead2 (result f64) + (f64.const 3.14) + ) + ;; CHECK: (func $main (param $0 i32) (result i32) + ;; CHECK-NEXT: (call_indirect $0 (type $0) + ;; CHECK-NEXT: (local.get $0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $main (export "main") (param i32) (result i32) + (call_indirect (result i32) (local.get 0)) + ) +) + +;; CHECK: (func $trap-stub +;; CHECK-NEXT: (unreachable) +;; CHECK-NEXT: ) +(module + ;; As above, but $dead1 is also referenced by a ref.func in live code, so + ;; while its element entry is still redirected to the stub, the function + ;; itself must be kept (with an emptied body). + (table 3 3 funcref) + (elem (i32.const 0) $called $dead1 $dead2) + ;; CHECK: (type $0 (func (result i32))) + + ;; CHECK: (type $1 (func (param i32))) + + ;; CHECK: (type $2 (func (param i32) (result i32))) + + ;; CHECK: (type $3 (func)) + + ;; CHECK: (table $0 3 3 funcref) + + ;; CHECK: (elem $0 (i32.const 0) $called $trap-stub $trap-stub) + + ;; CHECK: (elem declare func $dead1) + + ;; CHECK: (export "main" (func $main)) + + ;; CHECK: (func $called (result i32) + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + (func $called (result i32) + (i32.const 1) + ) + ;; CHECK: (func $dead1 (param $0 i32) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $dead1 (param i32) + (unreachable) + ) + (func $dead2 (result f64) + (f64.const 3.14) + ) + ;; CHECK: (func $main (param $0 i32) (result i32) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.func $dead1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (call_indirect $0 (type $0) + ;; CHECK-NEXT: (local.get $0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $main (export "main") (param i32) (result i32) + (drop + (ref.func $dead1) + ) + (call_indirect (result i32) (local.get 0)) + ) +) + +;; CHECK: (func $trap-stub +;; CHECK-NEXT: (unreachable) +;; CHECK-NEXT: ) +(module + ;; An exported table: the outside can observe the entries, so we do not + ;; redirect them. The never-called functions are kept, with emptied bodies. + (table (export "t") 3 3 funcref) + (elem (i32.const 0) $called $dead1 $dead2) + ;; CHECK: (type $0 (func (result i32))) + + ;; CHECK: (type $1 (func (param i32))) + + ;; CHECK: (type $2 (func (result f64))) + + ;; CHECK: (type $3 (func (param i32) (result i32))) + + ;; CHECK: (table $0 3 3 funcref) + + ;; CHECK: (elem $0 (i32.const 0) $called $dead1 $dead2) + + ;; CHECK: (export "t" (table $0)) + + ;; CHECK: (export "main" (func $main)) + + ;; CHECK: (func $called (result i32) + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + (func $called (result i32) + (i32.const 1) + ) + ;; CHECK: (func $dead1 (param $0 i32) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $dead1 (param i32) + (unreachable) + ) + ;; CHECK: (func $dead2 (result f64) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $dead2 (result f64) + (f64.const 3.14) + ) + ;; CHECK: (func $main (param $0 i32) (result i32) + ;; CHECK-NEXT: (call_indirect $0 (type $0) + ;; CHECK-NEXT: (local.get $0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $main (export "main") (param i32) (result i32) + (call_indirect (result i32) (local.get 0)) + ) +) + +(module + ;; An imported table: same as exported, we do not redirect. + ;; CHECK: (type $0 (func (result i32))) + + ;; CHECK: (type $1 (func (param i32))) + + ;; CHECK: (type $2 (func (result f64))) + + ;; CHECK: (type $3 (func (param i32) (result i32))) + + ;; CHECK: (import "env" "t" (table $t 3 3 funcref)) + (import "env" "t" (table $t 3 3 funcref)) + (elem (table $t) (i32.const 0) func $called $dead1 $dead2) + ;; CHECK: (elem $0 (i32.const 0) $called $dead1 $dead2) + + ;; CHECK: (export "main" (func $main)) + + ;; CHECK: (func $called (result i32) + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + (func $called (result i32) + (i32.const 1) + ) + ;; CHECK: (func $dead1 (param $0 i32) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $dead1 (param i32) + (unreachable) + ) + ;; CHECK: (func $dead2 (result f64) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $dead2 (result f64) + (f64.const 3.14) + ) + ;; CHECK: (func $main (param $0 i32) (result i32) + ;; CHECK-NEXT: (call_indirect $t (type $0) + ;; CHECK-NEXT: (local.get $0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $main (export "main") (param i32) (result i32) + (call_indirect $t (result i32) (local.get 0)) + ) +)