Skip to content
Open
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
26 changes: 12 additions & 14 deletions src/passes/Vacuum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -331,17 +331,6 @@ struct Vacuum : public WalkerPass<ExpressionStackWalker<Vacuum>> {
curr->condition =
Builder(*getModule()).makeUnary(EqZInt32, curr->condition);
BranchHints::flip(curr, getFunction());
} else if (curr->ifTrue->is<Drop>() && curr->ifFalse->is<Drop>()) {
// instead of dropping both sides, drop the if, if they are the same
// type
auto* left = curr->ifTrue->cast<Drop>()->value;
auto* right = curr->ifFalse->cast<Drop>()->value;
if (left->type == right->type) {
curr->ifTrue = left;
curr->ifFalse = right;
curr->finalize();
replaceCurrent(Builder(*getModule()).makeDrop(curr));
}

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.

For MVP types this still saves one byte, so I'm not sure we want to remove this completely?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Per your other comment, I asked agent to prototype special casing MVP (re-hoist all isBasic()), it didn't make a meaningful difference.

Agent made following argument while prototyping but I didn't dig to verify claims:

Why MVP expressions survive Vacuum::optimizeDrop—and why hoisting hurts downstream passes
Because all MVP basic types are defaultable (Type::isDefaultable()), Vacuum::optimize and Vacuum::optimizeDrop already eliminate (drop X) for every pure expression, unary/binary arithmetic or comparison wrapper, non-trapping load, local.tee, and poppable block. Consequently, an MVP-typed arm can remain a Drop after Vacuum only when the arm is:

  1. Call / CallRef / CallIndirect (11 instances in calcworker_wasm):
    DeadArgumentElimination::visitDrop only records info->droppedCalls[call] when Drop directly wraps Call.
    Re-hoisting drop out of (if cond (drop (call $f)) (drop (call $g))) into (drop (if (result i32) cond (call $f) (call $g))) couples $f's return type to $g's return type and hides (drop (call $f)) from dae-optimizing.
    In calcworker_wasm, re-hoisting MVP Drops (Variant B1) prevented DAE from stripping the unused i32 return value of $725 (because $725 was paired in an if-else in $2119 with $841, whose i32 return value is used elsewhere), which also prevented eliminating a caller thunk ($2772).

  2. Break (br_if carrying a value) or Block with value-carrying branches (5 instances in calcworker_wasm):
    ProblemFinder::visitExpression and MergeBlocks::visitDrop require br_if and block to be directly wrapped in Drop to strip unused branch values from enclosing blocks.

}
} else {
// This is an if without an else. If the body is empty, we do not need it.
Expand Down Expand Up @@ -427,9 +416,11 @@ struct Vacuum : public WalkerPass<ExpressionStackWalker<Vacuum>> {
}
}
}
// sink a drop into an arm of an if-else if the other arm ends in an
// unreachable, as it if is a branch, this can make that branch optimizable
// and more vacuuming possible
// Sink a drop into an arm of an if-else if the other arm ends in an
// unreachable, or sink drops into both arms if both arms are concrete.
// This allows the if expression to become void, eliminates block type
// overhead, and enables further vacuuming/dead-code elimination in each
// arm.
auto* iff = curr->value->dynCast<If>();
if (iff && iff->ifFalse && iff->type.isConcrete()) {
// reuse the drop in both cases
Expand All @@ -445,6 +436,13 @@ struct Vacuum : public WalkerPass<ExpressionStackWalker<Vacuum>> {
iff->ifTrue = curr;
iff->type = Type::none;
replaceCurrent(iff);
} else if (iff->ifTrue->type.isConcrete() &&
iff->ifFalse->type.isConcrete()) {
Builder builder(*getModule());
iff->ifTrue = builder.dropIfConcretelyTyped(iff->ifTrue);
iff->ifFalse = builder.dropIfConcretelyTyped(iff->ifFalse);

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.

These can just be makeDrop as we know they are concretely typed.

iff->type = Type::none;
replaceCurrent(iff);
}
}
}
Expand Down
24 changes: 12 additions & 12 deletions test/lit/passes/dae-optimizing.wast
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
(type $1 (func (param f64 f32 f32 f64 f32 i64 f64) (result i32)))
;; CHECK: (type $3 (func (result i32)))

;; CHECK: (type $4 (func (result f32)))
;; CHECK: (type $4 (func))

;; CHECK: (type $2 (func (param f64 f32 f32 f64 f32 i32 i32 f64) (result i32)))
(type $2 (func (param f64 f32 f32 f64 f32 i32 i32 f64) (result i32)))
Expand All @@ -17,12 +17,12 @@
;; CHECK: (func $0 (type $3) (result i32)
;; CHECK-NEXT: (local $0 i32)
;; CHECK-NEXT: (local $1 i32)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (if (result f32)
;; CHECK-NEXT: (local.tee $0
;; CHECK-NEXT: (i32.const 33554432)
;; CHECK-NEXT: )
;; CHECK-NEXT: (then
;; CHECK-NEXT: (if
;; CHECK-NEXT: (local.tee $0
;; CHECK-NEXT: (i32.const 33554432)
;; CHECK-NEXT: )
;; CHECK-NEXT: (then
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (loop $label$2 (result f32)
;; CHECK-NEXT: (if
;; CHECK-NEXT: (global.get $global$0)
Expand All @@ -44,9 +44,9 @@
;; CHECK-NEXT: (f32.const 1)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (else
;; CHECK-NEXT: (call $1)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (else
;; CHECK-NEXT: (call $1)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (i32.const -11)
Expand Down Expand Up @@ -96,8 +96,8 @@
)
(i32.const -11)
)
;; CHECK: (func $1 (type $4) (result f32)
;; CHECK-NEXT: (f32.const 0)
;; CHECK: (func $1 (type $4)
;; CHECK-NEXT: (nop)
;; CHECK-NEXT: )
(func $1 (; 1 ;) (type $0) (param $0 f32) (result f32)
(f32.const 0)
Expand Down
Loading
Loading