Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 113 additions & 0 deletions src/passes/RemoveUnusedModuleElements.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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<Name> redirected;
if (!module->features.hasGC()) {
std::unordered_set<Name> exportedTables;
for (auto& ex : module->exports) {
if (ex->kind == ExternalKind::Table) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was hoping tableInfoMap would make all these checks trivial - it has hasSet to check for table.sets both inside and outside the module. However, we would need to add hasGet there, for this optimization. I'm not sure if that would be useful enough, and in particular it means more scanning work.

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) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm worried about this extra scanning work, given the benefit here may be very small.

Maybe it would be good to measure the benefit of this before doing any more work here. Do you have some codebase in mind where you were expecting to see a benefit, or did you just see the open issue?

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<RefFunc>();
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
Expand Down Expand Up @@ -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<Name> stillReferenced;
auto scanExpr = [&](Expression* expr) {
if (!expr) {
return;
}
for (auto* refFunc : FindAll<RefFunc>(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
Expand Down
Original file line number Diff line number Diff line change
@@ -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))
)
)
198 changes: 198 additions & 0 deletions test/lit/passes/remove-unused-module-elements-table-stub.wast
Original file line number Diff line number Diff line change
@@ -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))
)
)
Loading