diff options
| -rw-r--r-- | src/passes/OptimizeInstructions.cpp | 31 | ||||
| -rw-r--r-- | test/lit/passes/optimize-instructions.wast | 50 |
2 files changed, 79 insertions, 2 deletions
diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp index c5ffd34b3..310924327 100644 --- a/src/passes/OptimizeInstructions.cpp +++ b/src/passes/OptimizeInstructions.cpp @@ -2874,9 +2874,10 @@ private: // TODO: consider the case with more than one child. ChildIterator ifTrueChildren(curr->ifTrue); if (ifTrueChildren.children.size() == 1) { - ChildIterator ifFalseChildren(curr->ifFalse); // ifTrue and ifFalse's children will become the direct children of - // curr, and so there must be an LUB for curr to have a proper type. + // curr, and so there must be an LUB for curr to have a proper new + // type after the transformation. + // // An example where that does not happen is this: // // (if @@ -2884,10 +2885,35 @@ private: // (drop (i32.const 1)) // (drop (f64.const 2.0)) // ) + ChildIterator ifFalseChildren(curr->ifFalse); auto* ifTrueChild = *ifTrueChildren.begin(); auto* ifFalseChild = *ifFalseChildren.begin(); bool validTypes = Type::hasLeastUpperBound(ifTrueChild->type, ifFalseChild->type); + + // In addition, after we move code outside of curr then we need to + // not change unreachability - if we did, we'd need to propagate + // that further, and we leave such work to DCE and Vacuum anyhow. + // This can happen in something like this for example, where the + // outer type changes from i32 to unreachable if we move the + // returns outside: + // + // (if (result i32) + // (local.get $x) + // (return + // (local.get $y) + // ) + // (return + // (local.get $z) + // ) + // ) + assert(curr->ifTrue->type == curr->ifFalse->type); + auto newOuterType = curr->ifTrue->type; + if ((newOuterType == Type::unreachable) != + (curr->type == Type::unreachable)) { + validTypes = false; + } + // If the expression we are about to move outside has side effects, // then we cannot do so in general with a select: we'd be reducing // the amount of the effects as well as moving them. For an if, @@ -2898,6 +2924,7 @@ private: getModule()->features, curr->ifTrue) .hasSideEffects(); + if (validTypes && validEffects) { // Replace ifTrue with its child. curr->ifTrue = ifTrueChild; diff --git a/test/lit/passes/optimize-instructions.wast b/test/lit/passes/optimize-instructions.wast index 058c030dd..5b5b44b57 100644 --- a/test/lit/passes/optimize-instructions.wast +++ b/test/lit/passes/optimize-instructions.wast @@ -12188,6 +12188,33 @@ ) ) ) + ;; CHECK: (func $ternary-identical-arms-return-select (param $x i32) (param $y i32) (param $z i32) (result i32) + ;; CHECK-NEXT: (block $block + ;; CHECK-NEXT: (select + ;; CHECK-NEXT: (return + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (return + ;; CHECK-NEXT: (local.get $y) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.get $z) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $ternary-identical-arms-return-select (param $x i32) (param $y i32) (param $z i32) (result i32) + (block $block + ;; we cannot optimize a select currently as the return has side effects + (select + (return + (local.get $x) + ) + (return + (local.get $y) + ) + (local.get $z) + ) + ) + ) ;; CHECK: (func $send-i32 (param $0 i32) ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: ) @@ -12212,4 +12239,27 @@ ) ) ) + ;; CHECK: (func $if-dont-change-to-unreachable (param $x i32) (param $y i32) (param $z i32) (result i32) + ;; CHECK-NEXT: (if (result i32) + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (return + ;; CHECK-NEXT: (local.get $y) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (return + ;; CHECK-NEXT: (local.get $z) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $if-dont-change-to-unreachable (param $x i32) (param $y i32) (param $z i32) (result i32) + ;; if we move the returns outside, we'd become unreachable; avoid that. + (if (result i32) + (local.get $x) + (return + (local.get $y) + ) + (return + (local.get $z) + ) + ) + ) ) |