Skip to content
Merged
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
60 changes: 39 additions & 21 deletions src/ir/linear-execution.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,11 @@ struct LinearExecutionWalker : public PostWalker<SubType, VisitorType> {
static void scan(SubType* self, Expression** currp) {
Expression* curr = *currp;

auto handleCall = [&](bool mayThrow, bool isReturn) {
auto handleCall = [&](bool isReturn, bool refutesThrowEffect) {
bool mayThrow = !self->getModule() ||
self->getModule()->features.hasExceptionHandling();
mayThrow = mayThrow && !refutesThrowEffect;

if (!self->connectAdjacentBlocks) {
// Control is nonlinear if we return or throw. Traps don't need to be
// taken into account since they don't break control flow in a way
Expand Down Expand Up @@ -156,40 +160,54 @@ struct LinearExecutionWalker : public PostWalker<SubType, VisitorType> {
case Expression::Id::CallId: {
auto* call = curr->cast<Call>();

bool mayThrow = !self->getModule() ||
self->getModule()->features.hasExceptionHandling();
if (mayThrow && self->getModule()) {
auto* effects =
self->getModule()->getFunction(call->target)->effects.get();

if (effects && !effects->throws_) {
mayThrow = false;
bool refutesThrowEffect = false;
if (self->getModule()) {
auto* func = self->getModule()->getFunctionOrNull(call->target);
// TODO: `func` might not exist here because of #8753. Fix this
// and remove the null check.
if (func && func->effects) {
refutesThrowEffect = !func->effects->throws_;
}
}

handleCall(mayThrow, call->isReturn);
handleCall(call->isReturn, refutesThrowEffect);
break;
}
case Expression::Id::CallRefId: {
auto* callRef = curr->cast<CallRef>();

// TODO: Effect analysis for indirect calls isn't implemented yet.
// Assume any indirect call may throw for now.
bool mayThrow = !self->getModule() ||
self->getModule()->features.hasExceptionHandling();
bool refutesThrowEffect = [&]() {
if (!self->getModule()) {
return false;
}
if (!callRef->target->type.isRef()) {
Comment thread
tlively marked this conversation as resolved.
// This is an unreachable, so no throws effect.
return true;
}

handleCall(mayThrow, callRef->isReturn);
auto* effects = find_or_null(self->getModule()->indirectCallEffects,
callRef->target->type.getHeapType());
if (!effects) {
return false;
}
return !(*effects)->throws_;
}();

handleCall(callRef->isReturn, refutesThrowEffect);
break;
}
case Expression::Id::CallIndirectId: {
auto* callIndirect = curr->cast<CallIndirect>();

// TODO: Effect analysis for indirect calls isn't implemented yet.
// Assume any indirect call may throw for now.
bool mayThrow = !self->getModule() ||
self->getModule()->features.hasExceptionHandling();

handleCall(mayThrow, callIndirect->isReturn);
bool refutesThrowEffect = false;
if (self->getModule()) {
if (auto* effects = find_or_null(
self->getModule()->indirectCallEffects, callIndirect->heapType);
effects) {
refutesThrowEffect = !(*effects)->throws_;
}
}
handleCall(callIndirect->isReturn, refutesThrowEffect);
break;
}
case Expression::Id::TryId: {
Expand Down
8 changes: 8 additions & 0 deletions src/support/utilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,14 @@ template<typename... Ts> struct overloaded : Ts... {

template<typename... Ts> overloaded(Ts...) -> overloaded<Ts...>;

// Lookup a value from `map` and return a pointer to the underlying value
// or nullptr if not present. Returns a const pointer if `map` is const and
// non-const otherwise
auto* find_or_null(auto& map, const auto& key) {
Comment thread
stevenfontanella marked this conversation as resolved.
auto it = map.find(key);
return it != map.end() ? &it->second : nullptr;
}

} // namespace wasm

#endif // wasm_support_utilities_h
84 changes: 64 additions & 20 deletions test/lit/passes/simplify-locals-global-effects-eh.wast
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,11 @@
)

(module
;; CHECK: (type $throw-type (func (result f64)))

;; CHECK: (type $const-type (func (result f32)))
(type $const-type (func (result f32)))

;; CHECK: (type $throw-type (func (result f64)))
(type $throw-type (func (result f64)))

;; CHECK: (global $g (mut i32) (i32.const 0))
Expand All @@ -68,7 +69,7 @@
;; CHECK: (table $t 2 2 funcref)
(table $t 2 2 funcref)

;; CHECK: (tag $t (type $2))
;; CHECK: (tag $t (type $4))
(tag $t)

;; CHECK: (func $const (type $const-type) (result f32)
Expand All @@ -90,32 +91,48 @@
)
(elem declare $throws)

;; CHECK: (func $read-g (type $3) (param $ref (ref null $const-type)) (result i32)
;; CHECK: (func $read-g-with-nop-call-ref (type $5) (param $ref (ref null $const-type)) (result i32)
;; CHECK-NEXT: (local $x i32)
;; CHECK-NEXT: (local.set $x
;; CHECK-NEXT: (global.get $g)
;; CHECK-NEXT: )
;; CHECK-NEXT: (nop)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (call_ref $const-type
;; CHECK-NEXT: (local.get $ref)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.get $x)
;; CHECK-NEXT: (global.get $g)
;; CHECK-NEXT: )
(func $read-g (param $ref (ref null $const-type)) (result i32)
(func $read-g-with-nop-call-ref (param $ref (ref null $const-type)) (result i32)
(local $x i32)
(local.set $x (global.get $g))

;; With more precise effect analysis for indirect calls, we can determine
;; that the only possible target for this ref is $const in a closed world,
;; which wouldn't block our optimizations.
;; TODO: Add effects analysis for indirect calls.
;; With --closed-world enabled, we can tell that this can only possibly call
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 think --closed-world isn't even necessary for this, since this module has no imports or exports. There's no way for an outside function reference to sneak in.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good point, is it worth doing any indirect call analysis in the open world case? We could aggregate type effects unconditionally, and if we see a function ref that's imported or exported then invalidate the effects for that type and all subtypes. But any exported table of funcref would be enough to stop all analysis, and I know that many users use --closed-world anyway?

In practice, we only do indirect call analysis if --closed-world is enabled, so I think the comment is appropriate. It's still true that --closed-world is enough to determine that $const is the only possible callee here.

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 think we should do indirect call analysis in open world, too. It just needs to respect the public/private type classification it gets from module-utils.h. (In the long run we want everything to run in both open-world and closed-world and fully encapsulate the difference in the type visibility classfication.)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'll leave this alone since I think the comment is still accurate for now based on #8754. In the future we can change GlobalEffects to look at public/private for types instead of --closed-world at which point we can change this.

;; $const, which doesn't block our optimizations.
(drop (call_ref $const-type (local.get $ref)))

(local.get $x)
)

;; CHECK: (func $read-g-with-throw-in-between (type $4) (param $ref (ref $throw-type)) (result i32)
;; CHECK: (func $read-g-with-nop-call-indirect (type $2) (result i32)
;; CHECK-NEXT: (local $x i32)
;; CHECK-NEXT: (nop)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (call_indirect $t (type $const-type)
;; CHECK-NEXT: (i32.const 0)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (global.get $g)
;; CHECK-NEXT: )
(func $read-g-with-nop-call-indirect (result i32)
(local $x i32)
(local.set $x (global.get $g))

;; Similar to above with call_indirect instead of call_ref.
(drop (call_indirect (type $const-type) (i32.const 0)))

(local.get $x)
)

;; CHECK: (func $read-g-with-effectful-call-ref (type $3) (param $ref (ref $throw-type)) (result i32)
;; CHECK-NEXT: (local $x i32)
;; CHECK-NEXT: (local.set $x
;; CHECK-NEXT: (global.get $g)
Expand All @@ -127,7 +144,7 @@
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.get $x)
;; CHECK-NEXT: )
(func $read-g-with-throw-in-between (param $ref (ref $throw-type)) (result i32)
(func $read-g-with-effectful-call-ref (param $ref (ref $throw-type)) (result i32)
(local $x i32)
(local.set $x (global.get $g))

Expand All @@ -138,25 +155,52 @@
(local.get $x)
)

;; CHECK: (func $read-g-with-call-indirect-in-between (type $5) (result i32)
;; CHECK: (func $read-g-with-effectful-call-indirect (type $3) (param $ref (ref $throw-type)) (result i32)
;; CHECK-NEXT: (local $x i32)
;; CHECK-NEXT: (local.set $x
;; CHECK-NEXT: (global.get $g)
;; CHECK-NEXT: )
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (call_indirect $t (type $const-type)
;; CHECK-NEXT: (call_indirect $t (type $throw-type)
;; CHECK-NEXT: (i32.const 0)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.get $x)
;; CHECK-NEXT: )
(func $read-g-with-call-indirect-in-between (result i32)
(func $read-g-with-effectful-call-indirect (param $ref (ref $throw-type)) (result i32)
(local $x i32)
(local.set $x (global.get $g))

;; Similar to above with call_indirect instead of call_ref.
;; TODO: Add effects analysis for indirect calls.
(drop (call_indirect (type $const-type) (i32.const 0)))
;; Similar to above, except here we can tell that the indirect call may
;; throw so optimization is halted.
(drop (call_indirect (type $throw-type) (i32.const 0)))
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 guess SimplifyLocals could be improved to optimize here despite the throws effect, but that's unrelated to this PR.


(local.get $x)
)

;; CHECK: (func $read-g-with-unreachable-call-ref (type $2) (result i32)
;; CHECK-NEXT: (local $x i32)
;; CHECK-NEXT: (local.set $x
;; CHECK-NEXT: (global.get $g)
;; CHECK-NEXT: )
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block ;; (replaces unreachable CallRef we can't emit)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (unreachable)
;; CHECK-NEXT: )
;; CHECK-NEXT: (unreachable)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.get $x)
;; CHECK-NEXT: )
(func $read-g-with-unreachable-call-ref (result i32)
(local $x i32)
(local.set $x (global.get $g))

;; This is guaranteed to trap, and the type immediate doesn't matter.
;; TODO: we should be able to optimize this, but something is likely missing
;; in SimplifyGlobals (LinearExecutionWalker handles this case correctly).
(drop (call_ref $throw-type (unreachable)))

(local.get $x)
)
Expand Down
Loading