diff --git a/src/ir/table-utils.cpp b/src/ir/table-utils.cpp index cb10aff82b9..516b12a215b 100644 --- a/src/ir/table-utils.cpp +++ b/src/ir/table-utils.cpp @@ -95,13 +95,13 @@ TableInfoMap computeTableInfo(Module& wasm, bool initialContentsImmutable) { for (auto& table : wasm.tables) { if (table->imported()) { - tables[table->name].mayBeModified = true; + tables[table->name].hasSet = true; } } for (auto& ex : wasm.exports) { if (ex->kind == ExternalKind::Table) { - tables[*ex->getInternalName()].mayBeModified = true; + tables[*ex->getInternalName()].hasSet = true; } } @@ -109,7 +109,7 @@ TableInfoMap computeTableInfo(Module& wasm, bool initialContentsImmutable) { // might learn anything new. auto hasUnmodifiableTable = false; for (auto& [_, info] : tables) { - if (!info.mayBeModified) { + if (!info.hasSet) { hasUnmodifiableTable = true; break; } @@ -118,39 +118,54 @@ TableInfoMap computeTableInfo(Module& wasm, bool initialContentsImmutable) { return tables; } - using TablesWithSet = std::unordered_set; + // Miniature form of TableInfo, without things we don't need (some of which + // cause compilation errors on the copies below). + struct MiniTableInfo { + bool hasSet = false; + bool hasGrow = false; + }; - ModuleUtils::ParallelFunctionAnalysis analysis( - wasm, [&](Function* func, TablesWithSet& tablesWithSet) { + using MiniTableInfoMap = std::unordered_map; + + ModuleUtils::ParallelFunctionAnalysis analysis( + wasm, [&](Function* func, MiniTableInfoMap& tableInfoMap) { if (func->imported()) { return; } struct Finder : public PostWalker { - TablesWithSet& tablesWithSet; + MiniTableInfoMap& tableInfoMap; - Finder(TablesWithSet& tablesWithSet) : tablesWithSet(tablesWithSet) {} + Finder(MiniTableInfoMap& tableInfoMap) : tableInfoMap(tableInfoMap) {} void visitTableSet(TableSet* curr) { - tablesWithSet.insert(curr->table); + tableInfoMap[curr->table].hasSet = true; } void visitTableFill(TableFill* curr) { - tablesWithSet.insert(curr->table); + tableInfoMap[curr->table].hasSet = true; } void visitTableCopy(TableCopy* curr) { - tablesWithSet.insert(curr->destTable); + tableInfoMap[curr->destTable].hasSet = true; } void visitTableInit(TableInit* curr) { - tablesWithSet.insert(curr->table); + tableInfoMap[curr->table].hasSet = true; + } + void visitTableGrow(TableGrow* curr) { + tableInfoMap[curr->table].hasGrow = true; } }; - Finder(tablesWithSet).walkFunction(func); + Finder(tableInfoMap).walkFunction(func); }); - for (auto& [_, names] : analysis.map) { - for (auto name : names) { - tables[name].mayBeModified = true; + for (auto& [_, tableInfoMap] : analysis.map) { + for (auto& [tableName, info] : tableInfoMap) { + if (info.hasSet) { + tables[tableName].hasSet = true; + } + if (info.hasGrow) { + tables[tableName].hasGrow = true; + } } } diff --git a/src/ir/table-utils.h b/src/ir/table-utils.h index 4e788a2685e..884f2309797 100644 --- a/src/ir/table-utils.h +++ b/src/ir/table-utils.h @@ -122,9 +122,14 @@ bool usesExpressions(ElementSegment* curr, Module* module); // Information about a table's optimizability. struct TableInfo { - // Whether the table may be modified at runtime, either because it is imported - // or exported, or table.set operations exist for it in the code. - bool mayBeModified = false; + // Whether the table has writes to it (anything but a grow, see below). The + // writes may be internal, or through imports and exports. + bool hasSet = false; + + // Whether the table may grow. Growing does modify the table, but it only + // appends, so we track this separately from mayBeModified. This allows more + // optimizations in tables that grow but have no other sets. + bool hasGrow = false; // Whether we can assume that the initial contents are immutable. That is, if // a table looks like [a, b, c] in the wasm, and we see a call to index 1, we @@ -144,6 +149,9 @@ struct TableInfo { std::unique_ptr flatTable; + // Whether the contents may change. + bool mayBeModified() const { return hasSet || hasGrow; } + // Whether we can optimize using this table's data on the entry level, that // is, individual entries in the table are known to us, so calls through the // table with known indexes can be inferred, etc. @@ -154,7 +162,10 @@ struct TableInfo { // contents, even if other things might be appended later, which we // cannot infer). // * The table is flat (so we can see what is in it, by index). - return (!mayBeModified || initialContentsImmutable) && flatTable->valid; + // + // Note that we do not check hasGrow, as we can optimize at least *some* + // entries in that case (growth only appends). + return (!hasSet || initialContentsImmutable) && flatTable->valid; } }; diff --git a/src/passes/Directize.cpp b/src/passes/Directize.cpp index c0cd9e221ab..2c0809b8dba 100644 --- a/src/passes/Directize.cpp +++ b/src/passes/Directize.cpp @@ -127,18 +127,23 @@ struct FunctionDirectizer : public WalkerPass> { // The index is out of bounds for the initial table's content. This may // trap, but it may also not trap if the table is modified later (if a // function is appended to it). - if (!table.mayBeModified) { + if (!table.mayBeModified()) { return CallUtils::Trap{}; } else { // The table may be modified, so it might be appended to. We should only - // get here in the case that the initial contents are immutable, as - // otherwise we have nothing to optimize at all. - assert(table.initialContentsImmutable); + // get here in the case that the initial contents are immutable, or the + // table can grow, as otherwise we have nothing to optimize at all. + assert(table.initialContentsImmutable || table.hasGrow); return CallUtils::Unknown{}; } } auto name = flatTable.names[index]; if (!name.is()) { + // No segment wrote to this part of the initial contents of the table. + // This must trap, as we only get here if we can optimize such cases, + // relying on the fact that the table cannot be modified, or at least the + // initial contents cannot be. + assert(!table.hasSet || table.initialContentsImmutable); return CallUtils::Trap{}; } auto* func = getModule()->getFunction(name); diff --git a/src/passes/RemoveUnusedModuleElements.cpp b/src/passes/RemoveUnusedModuleElements.cpp index 8bba990af8d..fd026490cb1 100644 --- a/src/passes/RemoveUnusedModuleElements.cpp +++ b/src/passes/RemoveUnusedModuleElements.cpp @@ -432,11 +432,13 @@ struct Analyzer { // Note a possible call of a function reference as well, if something else // might be written into the table during runtime. - // TODO: Add an option for immutable initial content like Directize? + // TODO: Add an option for immutable initial content like Directize? Can + // also check for grow without set, which leaves initial entries + // fixed. if (!tableInfoMap) { tableInfoMap = TableUtils::computeTableInfo(*module); } - if ((*tableInfoMap)[table].mayBeModified) { + if ((*tableInfoMap)[table].mayBeModified()) { useCallRefType(type); } } diff --git a/test/lit/passes/directize_all-features.wast b/test/lit/passes/directize_all-features.wast index 03ff576245d..adb064e8a96 100644 --- a/test/lit/passes/directize_all-features.wast +++ b/test/lit/passes/directize_all-features.wast @@ -1953,3 +1953,92 @@ ) ) +;; table.grow inhibits some optimizations. +(module + ;; CHECK: (type $func (func)) + ;; IMMUT: (type $func (func)) + (type $func (func)) + + ;; CHECK: (table $table 5 funcref) + ;; IMMUT: (table $table 5 funcref) + (table $table 5 funcref) + + ;; CHECK: (elem $table (i32.const 1) $target) + ;; IMMUT: (elem $table (i32.const 1) $target) + (elem $table (i32.const 1) $target) + + ;; CHECK: (elem declare func $grow) + + ;; CHECK: (export "caller" (func $caller)) + + ;; CHECK: (func $grow (type $func) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (table.grow $table + ;; CHECK-NEXT: (ref.func $grow) + ;; CHECK-NEXT: (i32.const 42) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; IMMUT: (elem declare func $grow) + + ;; IMMUT: (export "caller" (func $caller)) + + ;; IMMUT: (func $grow (type $func) + ;; IMMUT-NEXT: (drop + ;; IMMUT-NEXT: (table.grow $table + ;; IMMUT-NEXT: (ref.func $grow) + ;; IMMUT-NEXT: (i32.const 42) + ;; IMMUT-NEXT: ) + ;; IMMUT-NEXT: ) + ;; IMMUT-NEXT: ) + (func $grow + (drop + (table.grow $table + (ref.func $grow) + (i32.const 42) + ) + ) + ) + + ;; CHECK: (func $caller (type $func) + ;; CHECK-NEXT: (call $target) + ;; CHECK-NEXT: (call_indirect $table (type $func) + ;; CHECK-NEXT: (i32.const 10) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (call_indirect $table (type $func) + ;; CHECK-NEXT: (i32.const 1000) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; IMMUT: (func $caller (type $func) + ;; IMMUT-NEXT: (call $target) + ;; IMMUT-NEXT: (call_indirect $table (type $func) + ;; IMMUT-NEXT: (i32.const 10) + ;; IMMUT-NEXT: ) + ;; IMMUT-NEXT: (call_indirect $table (type $func) + ;; IMMUT-NEXT: (i32.const 1000) + ;; IMMUT-NEXT: ) + ;; IMMUT-NEXT: ) + (func $caller (export "caller") + ;; This is in the elem segment, so we can optimize it. Growth can only append. + (call_indirect (type $func) + (i32.const 1) + ) + ;; This is in the range that we grow to, if grow() is called, but we don't + ;; know if it will, so we don't optimize. + (call_indirect (type $func) + (i32.const 10) + ) + ;; This is in the range that we grow to, if grow() is called multiple times, + ;; but again we can't optimize. + (call_indirect (type $func) + (i32.const 1000) + ) + ) + + ;; CHECK: (func $target (type $func) + ;; CHECK-NEXT: ) + ;; IMMUT: (func $target (type $func) + ;; IMMUT-NEXT: ) + (func $target + ) +) diff --git a/test/lit/passes/remove-unused-module-elements-tables.wast b/test/lit/passes/remove-unused-module-elements-tables.wast index b0b6bd323a8..a556c09948f 100644 --- a/test/lit/passes/remove-unused-module-elements-tables.wast +++ b/test/lit/passes/remove-unused-module-elements-tables.wast @@ -333,7 +333,7 @@ ) ) -;; As above, but now the table has an initialistion expression +;; As above, but now the table has an initialization expression (module (rec ;; CHECK: (rec @@ -447,3 +447,116 @@ (drop (i32.const 30)) ) ) + +;; As above, but now the table has a table.grow. Like table.set, this prevents +;; optimization. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $foo (func)) + ;; OPEN_WORLD: (rec + ;; OPEN_WORLD-NEXT: (type $foo (func)) + (type $foo (func)) + ;; CHECK: (type $bar (func)) + ;; OPEN_WORLD: (type $bar (func)) + (type $bar (func)) + ) + + ;; CHECK: (type $2 (func)) + + ;; CHECK: (table $table 10 funcref) + ;; OPEN_WORLD: (type $2 (func)) + + ;; OPEN_WORLD: (table $table 10 funcref) + (table $table 10 funcref) + ;; CHECK: (elem $table (i32.const 0) $foo-in-table $bar) + ;; OPEN_WORLD: (elem $table (i32.const 0) $foo-in-table $bar) + (elem $table (i32.const 0) $foo-in-table $bar) + + ;; CHECK: (elem declare func $foo-not-in-table) + + ;; CHECK: (export "export" (func $export)) + + ;; CHECK: (func $export (type $2) + ;; CHECK-NEXT: (call_indirect $table (type $foo) + ;; CHECK-NEXT: (i32.const 5) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (table.grow $table + ;; CHECK-NEXT: (ref.func $foo-not-in-table) + ;; CHECK-NEXT: (i32.const 7) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; OPEN_WORLD: (elem declare func $foo-not-in-table) + + ;; OPEN_WORLD: (export "export" (func $export)) + + ;; OPEN_WORLD: (func $export (type $2) + ;; OPEN_WORLD-NEXT: (call_indirect $table (type $foo) + ;; OPEN_WORLD-NEXT: (i32.const 5) + ;; OPEN_WORLD-NEXT: ) + ;; OPEN_WORLD-NEXT: (drop + ;; OPEN_WORLD-NEXT: (table.grow $table + ;; OPEN_WORLD-NEXT: (ref.func $foo-not-in-table) + ;; OPEN_WORLD-NEXT: (i32.const 7) + ;; OPEN_WORLD-NEXT: ) + ;; OPEN_WORLD-NEXT: ) + ;; OPEN_WORLD-NEXT: ) + (func $export (export "export") + (call_indirect $table (type $foo) + (i32.const 5) + ) + (drop + (table.grow $table + (ref.func $foo-not-in-table) + (i32.const 7) + ) + ) + ) + + ;; CHECK: (func $foo-in-table (type $foo) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 10) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; OPEN_WORLD: (func $foo-in-table (type $foo) + ;; OPEN_WORLD-NEXT: (drop + ;; OPEN_WORLD-NEXT: (i32.const 10) + ;; OPEN_WORLD-NEXT: ) + ;; OPEN_WORLD-NEXT: ) + (func $foo-in-table (type $foo) + ;; This is in the table, and might be reached. + (drop (i32.const 10)) + ) + + ;; CHECK: (func $foo-not-in-table (type $foo) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 20) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; OPEN_WORLD: (func $foo-not-in-table (type $foo) + ;; OPEN_WORLD-NEXT: (drop + ;; OPEN_WORLD-NEXT: (i32.const 20) + ;; OPEN_WORLD-NEXT: ) + ;; OPEN_WORLD-NEXT: ) + (func $foo-not-in-table (type $foo) + ;; The reference taken of this function might be added to the table using + ;; table.grow, so we can do nothing here. + (drop (i32.const 20)) + ) + + ;; CHECK: (func $bar (type $bar) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; OPEN_WORLD: (func $bar (type $bar) + ;; OPEN_WORLD-NEXT: (drop + ;; OPEN_WORLD-NEXT: (i32.const 30) + ;; OPEN_WORLD-NEXT: ) + ;; OPEN_WORLD-NEXT: ) + (func $bar (type $bar) + ;; Not even references, so this is unreachable in closed world. + (drop (i32.const 30)) + ) +) +