diff --git a/src/passes/RemoveUnusedBrs.cpp b/src/passes/RemoveUnusedBrs.cpp index bc7e1e5b7c3..a2961da3233 100644 --- a/src/passes/RemoveUnusedBrs.cpp +++ b/src/passes/RemoveUnusedBrs.cpp @@ -65,9 +65,15 @@ stealSlice(Builder& builder, Block* input, Index from, Index to) { return ret; } -// to turn an if into a br-if, we must be able to reorder the -// condition and possible value, and the possible value must -// not have side effects (as they would run unconditionally) +// Check if a single expression is too costly to move from a conditional to an +// unconditional place. +static bool tooCostlyToRunUnconditionally(const PassOptions& passOptions, + Expression* curr); + +// To turn an if into a br-if, we must be able to reorder the condition and +// possible value, and the possible value must not have side effects (as they +// would run unconditionally). Also, if there is a value, it must not be too +// expensive to run unconditionally. static bool canTurnIfIntoBrIf(Expression* ifCondition, Expression* brValue, PassOptions& options, @@ -79,6 +85,9 @@ static bool canTurnIfIntoBrIf(Expression* ifCondition, if (!brValue) { return true; } + if (tooCostlyToRunUnconditionally(options, brValue)) { + return false; + } EffectAnalyzer value(options, wasm, brValue); if (value.hasSideEffects()) { return false; @@ -120,8 +129,6 @@ static bool tooCostlyToRunUnconditionally(const PassOptions& passOptions, } } -// As above, but a single expression that we are considering moving to a place -// where it executes unconditionally. static bool tooCostlyToRunUnconditionally(const PassOptions& passOptions, Expression* curr) { // If we care entirely about code size, just do it for that reason (early diff --git a/test/lit/passes/remove-unused-brs-gc.wast b/test/lit/passes/remove-unused-brs-gc.wast index 01f24852416..325deba8eee 100644 --- a/test/lit/passes/remove-unused-brs-gc.wast +++ b/test/lit/passes/remove-unused-brs-gc.wast @@ -997,4 +997,91 @@ (i32.const 0) ) ) -) + + ;; CHECK: (func $costly-br-value-no-if (type $16) (param $x i32) (result anyref) + ;; CHECK-NEXT: (block $out (result anyref) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (br $out + ;; CHECK-NEXT: (struct.new_default $struct) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (br $out + ;; CHECK-NEXT: (struct.new_default $struct) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (ref.null none) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $costly-br-value-no-if (param $x i32) (result anyref) + ;; We do not turn an if into a br_if if that makes a costly value execute + ;; unconditionally. + (block $out (result anyref) + (if + (local.get $x) + (then + (br $out + ;; An allocation is too expensive to unconditionalize. + (struct.new_default $struct) + ) + ) + ) + ;; Another if, so the entire block is not trivially optimized in another way. + (if + (local.get $x) + (then + (br $out + (struct.new_default $struct) + ) + ) + ) + (ref.null any) + ) + ) + + ;; CHECK: (func $cheap-br-value-yes-if (type $17) (param $x i32) (result i32) + ;; CHECK-NEXT: (block $out (result i32) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (br_if $out + ;; CHECK-NEXT: (i32.const 10) + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (br_if $out + ;; CHECK-NEXT: (i32.const 20) + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (i32.const 42) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $cheap-br-value-yes-if (param $x i32) (result i32) + ;; For comparison to above, if the br value is cheap, we do emit br_ifs here. + (block $out (result i32) + (if + (local.get $x) + (then + (br $out + (i32.const 10) + ) + ) + ) + ;; Another if, so the entire block is not trivially optimized in another way. + (if + (local.get $x) + (then + (br $out + (i32.const 20) + ) + ) + ) + (i32.const 42) + ) + ))