Skip to content
Draft
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
14 changes: 14 additions & 0 deletions src/ir/effects.h
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,12 @@ class EffectAnalyzer {
}
}
void visitCallIndirect(CallIndirect* curr) {
if (auto it = parent.module.typeEffects.find(curr->heapType);
it != parent.module.typeEffects.end()) {
parent.mergeIn(*it->second);
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.

It doesn't look like this accounts for the possible trap from a bad type check. I guess the previous code didn't need to handle it because setting calls implicitly includes trapping along with most other effects.

To make this as precise as possible, we could look at the type of the table and skip setting the trap effect if it is non-nullable and a subtype of the call_indirect expected type.

return;
}

parent.calls = true;
if (curr->isReturn) {
parent.branchesOut = true;
Expand Down Expand Up @@ -1040,6 +1046,14 @@ class EffectAnalyzer {
if (trapOnNull(curr->target)) {
return;
}

if (auto it =
parent.module.typeEffects.find(curr->target->type.getHeapType());
it != parent.module.typeEffects.end()) {
parent.mergeIn(*it->second);
return;
}

if (curr->isReturn) {
parent.branchesOut = true;
if (parent.features.hasExceptionHandling()) {
Expand Down
36 changes: 25 additions & 11 deletions src/passes/GlobalEffects.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "ir/module-utils.h"
#include "pass.h"
#include "support/strongly_connected_components.h"
#include "support/utilities.h"
#include "wasm.h"

namespace wasm {
Expand Down Expand Up @@ -210,10 +211,13 @@ void mergeMaybeEffects(std::optional<EffectAnalyzer>& dest,
// - Merge all of the effects of functions within the CC
// - Also merge the (already computed) effects of each callee CC
// - Add trap effects for potentially recursive call chains
void propagateEffects(const Module& module,
const PassOptions& passOptions,
std::map<Function*, FuncInfo>& funcInfos,
const CallGraph& callGraph) {
void propagateEffects(
const Module& module,
const PassOptions& passOptions,
std::map<Function*, FuncInfo>& funcInfos,
std::unordered_map<HeapType, std::shared_ptr<const EffectAnalyzer>>&
typeEffects,
const CallGraph& callGraph) {
// We only care about Functions that are roots, not types.
// A type would be a root if a function exists with that type, but no-one
// indirect calls the type.
Expand Down Expand Up @@ -302,12 +306,21 @@ void propagateEffects(const Module& module,
}

// Assign each function's effects to its CC effects.
for (Function* f : ccFuncs) {
if (!ccEffects) {
funcInfos.at(f).effects = UnknownEffects;
} else {
funcInfos.at(f).effects.emplace(*ccEffects);
}
for (auto node : cc) {
std::visit(overloaded{[&](HeapType type) {
if (ccEffects != UnknownEffects) {
typeEffects[type] =
std::make_shared<EffectAnalyzer>(*ccEffects);
}
},
[&](Function* f) {
if (!ccEffects) {
funcInfos.at(f).effects = UnknownEffects;
} else {
funcInfos.at(f).effects.emplace(*ccEffects);
}
}},
node);
}
}
}
Expand All @@ -331,7 +344,8 @@ struct GenerateGlobalEffects : public Pass {
auto callGraph =
buildCallGraph(*module, funcInfos, getPassOptions().closedWorld);

propagateEffects(*module, getPassOptions(), funcInfos, callGraph);
propagateEffects(
*module, getPassOptions(), funcInfos, module->typeEffects, callGraph);

copyEffectsToFunctions(funcInfos);
}
Expand Down
4 changes: 4 additions & 0 deletions src/support/utilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ class Fatal {
#define WASM_UNREACHABLE(msg) wasm::handle_unreachable()
#endif

template<class... Ts> struct overloaded : Ts... {
using Ts::operator()...;
};

} // namespace wasm

#endif // wasm_support_utilities_h
5 changes: 5 additions & 0 deletions src/wasm.h
Original file line number Diff line number Diff line change
Expand Up @@ -2682,6 +2682,11 @@ class Module {
std::unordered_map<HeapType, TypeNames> typeNames;
std::unordered_map<HeapType, Index> typeIndices;

// Potential effects for bodies of indirect calls to this type.
// TODO: make this into Type
std::unordered_map<HeapType, std::shared_ptr<const EffectAnalyzer>>
typeEffects;

MixedArena allocator;

private:
Expand Down
15 changes: 1 addition & 14 deletions test/lit/passes/global-effects-closed-world-tnh.wast
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,9 @@
)

;; CHECK: (func $calls-nop-via-nullable-ref (type $1) (param $ref (ref null $nopType))
;; CHECK-NEXT: (call_ref $nopType
;; CHECK-NEXT: (i32.const 1)
;; CHECK-NEXT: (local.get $ref)
;; CHECK-NEXT: )
;; CHECK-NEXT: (nop)
;; CHECK-NEXT: )
(func $calls-nop-via-nullable-ref (param $ref (ref null $nopType))
(call_ref $nopType (i32.const 1) (local.get $ref))
)

;; CHECK: (func $f (type $1) (param $ref (ref null $nopType))
;; CHECK-NEXT: (nop)
;; CHECK-NEXT: )
(func $f (param $ref (ref null $nopType))
;; The only possible implementation of $nopType has no effects.
;; $calls-nop-via-nullable-ref may trap from a null reference, but
;; --traps-never-happen is enabled, so we're free to optimize this out.
(call $calls-nop-via-nullable-ref (local.get $ref))
)
)
Loading
Loading