From 95300e604c3d319df7528510c2a017012f8d6b5b Mon Sep 17 00:00:00 2001 From: Roberto Lublinerman Date: Sun, 12 May 2024 08:49:54 -0700 Subject: [PATCH 1/7] Push struct.new down to make it more likely to optimize struct.sets. --- src/passes/OptimizeInstructions.cpp | 50 +++++++++++++++++-- .../passes/optimize-instructions-gc-heap.wast | 44 ++++++++-------- 2 files changed, 69 insertions(+), 25 deletions(-) diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp index 3486fe82a01..bafc3dbd47e 100644 --- a/src/passes/OptimizeInstructions.cpp +++ b/src/passes/OptimizeInstructions.cpp @@ -1882,17 +1882,33 @@ struct OptimizeInstructions // This local.set of a struct.new looks good. Find struct.sets after it // to optimize. - for (Index j = i + 1; j < list.size(); j++) { + Index localSetIndex = i; + for (Index j = localSetIndex + 1; j < list.size(); j++) { auto* structSet = list[j]->dynCast(); + // Any time the pattern no longer matches, we try to push the + // struct.new further down but if it is not possible we stop + // optimizing possible struct.sets for this struct.new. + if (!structSet) { - // Any time the pattern no longer matches, stop optimizing possible - // struct.sets for this struct.new. + if (trySwap(list, localSetIndex, j)) { + // Update the index an continue to try again. + localSetIndex = j; + continue; + } break; } + auto* localGet = structSet->ref->dynCast(); if (!localGet || localGet->index != localSet->index) { + if (trySwap(list, localSetIndex, j)) { + // Update the index an continue to try again. + localSetIndex = j; + continue; + } break; } + + // The pattern matches, try to optimize. if (!optimizeSubsequentStructSet(new_, structSet, localGet->index)) { break; } else { @@ -1904,6 +1920,34 @@ struct OptimizeInstructions } } + // Tries pushing the struct.new down so that it is closer + // to a potential struct.set. + bool trySwap(ExpressionList& list, Index i, Index j) { + if (j == list.size() - 1) { + // There is no reason to swap with the last element + // of the list as it won't match the pattern. + return false; + } + + // Check if the local is referencenced by the instruction we want to + // swap it with, + auto* localSet = list[i]->dynCast(); + auto otherEffects = effects(list[j]); + if (otherEffects.localsRead.count(localSet->index) || + otherEffects.localsWritten.count(localSet->index)) { + return false; + } + // or if the effects don't permit moving one past the other. + auto structNewEffects = effects(localSet->value); + if (otherEffects.invalidates(structNewEffects)) { + return false; + } + + list[i] = list[j]; + list[j] = localSet; + return true; + } + // Given a struct.new and a struct.set that occurs right after it, and that // applies to the same data, try to apply the set during the new. This can be // either with a nested tee: diff --git a/test/lit/passes/optimize-instructions-gc-heap.wast b/test/lit/passes/optimize-instructions-gc-heap.wast index 6db63fae325..c44e2f67735 100644 --- a/test/lit/passes/optimize-instructions-gc-heap.wast +++ b/test/lit/passes/optimize-instructions-gc-heap.wast @@ -1,5 +1,5 @@ ;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited. -;; RUN: wasm-opt %s --remove-unused-names --optimize-instructions -all -S -o - \ +;; RUN: wasm-opt %s --remove-unused-names --optimize-instructions -all -S -o - \ ;; RUN: | filecheck %s ;; ;; --remove-unused-names allows the optimizer to see that the blocks have no @@ -272,16 +272,13 @@ ;; CHECK: (func $pattern-breaker (type $1) ;; CHECK-NEXT: (local $ref (ref null $struct)) + ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: (local.set $ref ;; CHECK-NEXT: (struct.new $struct - ;; CHECK-NEXT: (i32.const 10) + ;; CHECK-NEXT: (i32.const 20) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (nop) - ;; CHECK-NEXT: (struct.set $struct 0 - ;; CHECK-NEXT: (local.get $ref) - ;; CHECK-NEXT: (i32.const 20) - ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $pattern-breaker (local $ref (ref null $struct)) @@ -611,20 +608,17 @@ ;; CHECK: (func $many-news (type $1) ;; CHECK-NEXT: (local $ref (ref null $struct3)) ;; CHECK-NEXT: (local $ref2 (ref null $struct3)) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: (local.set $ref ;; CHECK-NEXT: (struct.new $struct3 ;; CHECK-NEXT: (i32.const 40) ;; CHECK-NEXT: (i32.const 50) - ;; CHECK-NEXT: (i32.const 30) + ;; CHECK-NEXT: (i32.const 60) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (nop) - ;; CHECK-NEXT: (nop) - ;; CHECK-NEXT: (struct.set $struct3 2 - ;; CHECK-NEXT: (local.get $ref) - ;; CHECK-NEXT: (i32.const 60) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: (local.set $ref ;; CHECK-NEXT: (struct.new $struct3 ;; CHECK-NEXT: (i32.const 400) @@ -634,6 +628,17 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: (block + ;; CHECK-NEXT: (local.set $ref2 + ;; CHECK-NEXT: (struct.new $struct3 + ;; CHECK-NEXT: (i32.const 400) + ;; CHECK-NEXT: (i32.const 200) + ;; CHECK-NEXT: (i32.const 300) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (struct.set $struct3 2 + ;; CHECK-NEXT: (local.get $ref2) + ;; CHECK-NEXT: (i32.const 500) + ;; CHECK-NEXT: ) ;; CHECK-NEXT: (local.set $ref ;; CHECK-NEXT: (struct.new $struct3 ;; CHECK-NEXT: (i32.const 40) @@ -641,15 +646,10 @@ ;; CHECK-NEXT: (i32.const 30) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (local.set $ref2 - ;; CHECK-NEXT: (struct.new $struct3 - ;; CHECK-NEXT: (i32.const 400) - ;; CHECK-NEXT: (i32.const 600) - ;; CHECK-NEXT: (i32.const 500) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (struct.set $struct3 1 + ;; CHECK-NEXT: (local.get $ref2) + ;; CHECK-NEXT: (i32.const 600) ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (nop) - ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $many-news From 1b8e5f20ba8fcc6cbf06ada3e044eef57dc2b13a Mon Sep 17 00:00:00 2001 From: Roberto Lublinerman Date: Sun, 12 May 2024 08:57:20 -0700 Subject: [PATCH 2/7] More fixes. --- src/passes/OptimizeInstructions.cpp | 5 ++++ .../passes/optimize-instructions-gc-heap.wast | 25 +++++++------------ 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp index bafc3dbd47e..0efaa84b83e 100644 --- a/src/passes/OptimizeInstructions.cpp +++ b/src/passes/OptimizeInstructions.cpp @@ -1929,6 +1929,11 @@ struct OptimizeInstructions return false; } + if (list[j]->is() && + list[j]->dynCast()->value->is()) { + // Don't swap two struct.new instructions to avoid going back and forth. + return false; + } // Check if the local is referencenced by the instruction we want to // swap it with, auto* localSet = list[i]->dynCast(); diff --git a/test/lit/passes/optimize-instructions-gc-heap.wast b/test/lit/passes/optimize-instructions-gc-heap.wast index c44e2f67735..38bcc014d16 100644 --- a/test/lit/passes/optimize-instructions-gc-heap.wast +++ b/test/lit/passes/optimize-instructions-gc-heap.wast @@ -9,11 +9,10 @@ ;; CHECK: (type $struct (struct (field (mut i32)))) (type $struct (struct (field (mut i32)))) - ;; CHECK: (type $struct3 (struct (field (mut i32)) (field (mut i32)) (field (mut i32)))) - ;; CHECK: (type $struct2 (struct (field (mut i32)) (field (mut i32)))) (type $struct2 (struct (field (mut i32)) (field (mut i32)))) + ;; CHECK: (type $struct3 (struct (field (mut i32)) (field (mut i32)) (field (mut i32)))) (type $struct3 (struct (field (mut i32)) (field (mut i32)) (field (mut i32)))) ;; CHECK: (func $tee (type $1) @@ -628,17 +627,6 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: (block - ;; CHECK-NEXT: (local.set $ref2 - ;; CHECK-NEXT: (struct.new $struct3 - ;; CHECK-NEXT: (i32.const 400) - ;; CHECK-NEXT: (i32.const 200) - ;; CHECK-NEXT: (i32.const 300) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (struct.set $struct3 2 - ;; CHECK-NEXT: (local.get $ref2) - ;; CHECK-NEXT: (i32.const 500) - ;; CHECK-NEXT: ) ;; CHECK-NEXT: (local.set $ref ;; CHECK-NEXT: (struct.new $struct3 ;; CHECK-NEXT: (i32.const 40) @@ -646,10 +634,15 @@ ;; CHECK-NEXT: (i32.const 30) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (struct.set $struct3 1 - ;; CHECK-NEXT: (local.get $ref2) - ;; CHECK-NEXT: (i32.const 600) + ;; CHECK-NEXT: (local.set $ref2 + ;; CHECK-NEXT: (struct.new $struct3 + ;; CHECK-NEXT: (i32.const 400) + ;; CHECK-NEXT: (i32.const 600) + ;; CHECK-NEXT: (i32.const 500) + ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $many-news From 1ae61a5fca0c519b9f0cf5b7ad0ecb8dab539d29 Mon Sep 17 00:00:00 2001 From: Roberto Lublinerman Date: Sun, 12 May 2024 10:28:58 -0700 Subject: [PATCH 3/7] Fix .wast. --- .../passes/optimize-instructions-gc-heap.wast | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/test/lit/passes/optimize-instructions-gc-heap.wast b/test/lit/passes/optimize-instructions-gc-heap.wast index 38bcc014d16..39d7a2330dd 100644 --- a/test/lit/passes/optimize-instructions-gc-heap.wast +++ b/test/lit/passes/optimize-instructions-gc-heap.wast @@ -271,23 +271,31 @@ ;; CHECK: (func $pattern-breaker (type $1) ;; CHECK-NEXT: (local $ref (ref null $struct)) - ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (local $ref2 (ref null $struct)) ;; CHECK-NEXT: (local.set $ref ;; CHECK-NEXT: (struct.new $struct - ;; CHECK-NEXT: (i32.const 20) + ;; CHECK-NEXT: (i32.const 10) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (local.set $ref2 + ;; CHECK-NEXT: (local.get $ref) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (struct.set $struct 0 + ;; CHECK-NEXT: (local.get $ref) + ;; CHECK-NEXT: (i32.const 20) + ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $pattern-breaker (local $ref (ref null $struct)) + (local $ref2 (ref null $struct)) (local.set $ref (struct.new $struct (i32.const 10) ) ) - ;; Anything that we don't recognize breaks the pattern. - (nop) + ;; Any instruction that can not be swapped and is not + ;; the expected struct.set breaks the pattern. + (local.set $ref2 (local.get $ref)) (struct.set $struct 0 (local.get $ref) (i32.const 20) From 9487c70c0b79ade2780d3de9ceb6eeefb22029ba Mon Sep 17 00:00:00 2001 From: Roberto Lublinerman Date: Mon, 13 May 2024 20:00:53 -0700 Subject: [PATCH 4/7] Clean up some of the logic. --- src/passes/OptimizeInstructions.cpp | 55 +++++++++++++---------------- 1 file changed, 25 insertions(+), 30 deletions(-) diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp index 0efaa84b83e..713d0ca5d95 100644 --- a/src/passes/OptimizeInstructions.cpp +++ b/src/passes/OptimizeInstructions.cpp @@ -1864,11 +1864,9 @@ struct OptimizeInstructions // => // (local.set $x (struct.new X' Y Z)) // - // We also handle other struct.sets immediately after this one, but we only - // handle the case where they are all in sequence and right after the - // local.set (anything in the middle of this pattern will stop us from - // optimizing later struct.sets, which might be improved later but would - // require an analysis of effects TODO). + // We also handle other struct.sets immediately after this one. If the + // instruction following the new is not a struct.set we push the new down if + // possible. void optimizeHeapStores(ExpressionList& list) { for (Index i = 0; i < list.size(); i++) { auto* localSet = list[i]->dynCast(); @@ -1880,26 +1878,20 @@ struct OptimizeInstructions continue; } - // This local.set of a struct.new looks good. Find struct.sets after it - // to optimize. + // This local.set of a struct.new looks good. Find struct.sets after it to + // optimize. Index localSetIndex = i; for (Index j = localSetIndex + 1; j < list.size(); j++) { - auto* structSet = list[j]->dynCast(); - // Any time the pattern no longer matches, we try to push the - // struct.new further down but if it is not possible we stop - // optimizing possible struct.sets for this struct.new. - - if (!structSet) { - if (trySwap(list, localSetIndex, j)) { - // Update the index an continue to try again. - localSetIndex = j; - continue; - } - break; - } - auto* localGet = structSet->ref->dynCast(); - if (!localGet || localGet->index != localSet->index) { + // Check that the next instruction is a struct.set on the same local as + // the struct.new. + auto* structSet = list[j]->dynCast(); + auto* localGet = + structSet ? structSet->ref->dynCast() : nullptr; + if (!structSet || !localGet || localGet->index != localSet->index) { + // Any time the pattern no longer matches, we try to push the + // struct.new further down but if it is not possible we stop + // optimizing possible struct.sets for this struct.new. if (trySwap(list, localSetIndex, j)) { // Update the index an continue to try again. localSetIndex = j; @@ -1912,20 +1904,23 @@ struct OptimizeInstructions if (!optimizeSubsequentStructSet(new_, structSet, localGet->index)) { break; } else { - // Success. Replace the set with a nop, and continue to - // perhaps optimize more. + // Success. Replace the set with a nop, and continue to perhaps + // optimize more. ExpressionManipulator::nop(structSet); } } } } - // Tries pushing the struct.new down so that it is closer - // to a potential struct.set. + // Tries pushing the struct.new down so that it is closer to a potential + // struct.set. bool trySwap(ExpressionList& list, Index i, Index j) { if (j == list.size() - 1) { - // There is no reason to swap with the last element - // of the list as it won't match the pattern. + // There is no reason to swap with the last element of the list as it + // won't match the pattern because there wont be anything after. This also + // avoids swapping an instruction that does not leave anything in the + // stack by one that could leave something, and that which would be + // incorrect. return false; } @@ -1934,8 +1929,8 @@ struct OptimizeInstructions // Don't swap two struct.new instructions to avoid going back and forth. return false; } - // Check if the local is referencenced by the instruction we want to - // swap it with, + // Check if the local is referenced by the instruction we want to swap it + // with. auto* localSet = list[i]->dynCast(); auto otherEffects = effects(list[j]); if (otherEffects.localsRead.count(localSet->index) || From 02409d22de0215c13caba7c8024a3ed2c85c0400 Mon Sep 17 00:00:00 2001 From: Roberto Lublinerman Date: Tue, 14 May 2024 16:21:58 -0700 Subject: [PATCH 5/7] More fixes. --- src/passes/OptimizeInstructions.cpp | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp index 713d0ca5d95..dd6c7fcea07 100644 --- a/src/passes/OptimizeInstructions.cpp +++ b/src/passes/OptimizeInstructions.cpp @@ -1893,7 +1893,7 @@ struct OptimizeInstructions // struct.new further down but if it is not possible we stop // optimizing possible struct.sets for this struct.new. if (trySwap(list, localSetIndex, j)) { - // Update the index an continue to try again. + // Update the index and continue to try again. localSetIndex = j; continue; } @@ -1912,8 +1912,9 @@ struct OptimizeInstructions } } - // Tries pushing the struct.new down so that it is closer to a potential - // struct.set. + // Helper function for optimizeHeapStores. Tries pushing the struct.new at + // index i down to index j, swapping it with the instruction already at j, so + // that it is closer to (potential) later struct.sets. bool trySwap(ExpressionList& list, Index i, Index j) { if (j == list.size() - 1) { // There is no reason to swap with the last element of the list as it @@ -1929,22 +1930,18 @@ struct OptimizeInstructions // Don't swap two struct.new instructions to avoid going back and forth. return false; } - // Check if the local is referenced by the instruction we want to swap it - // with. - auto* localSet = list[i]->dynCast(); - auto otherEffects = effects(list[j]); - if (otherEffects.localsRead.count(localSet->index) || - otherEffects.localsWritten.count(localSet->index)) { - return false; - } - // or if the effects don't permit moving one past the other. - auto structNewEffects = effects(localSet->value); - if (otherEffects.invalidates(structNewEffects)) { + // Check if the two expressions can be swapped safely considering their + // effects. + auto firstEffects = effects(list[i]); + auto secondEffects = effects(list[j]); + if (secondEffects.invalidates(firstEffects)) { return false; } + // Swap the expressions. + auto* tmp = list[i]; list[i] = list[j]; - list[j] = localSet; + list[j] = tmp; return true; } From c751f8bf5df3d6603552ed5ec986d6d60ae25ef9 Mon Sep 17 00:00:00 2001 From: Roberto Lublinerman Date: Tue, 21 May 2024 13:32:47 -0700 Subject: [PATCH 6/7] Fixes --- src/passes/OptimizeInstructions.cpp | 5 +-- .../passes/optimize-instructions-gc-heap.wast | 41 +++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp index dd6c7fcea07..82c35fa9937 100644 --- a/src/passes/OptimizeInstructions.cpp +++ b/src/passes/OptimizeInstructions.cpp @@ -1938,10 +1938,7 @@ struct OptimizeInstructions return false; } - // Swap the expressions. - auto* tmp = list[i]; - list[i] = list[j]; - list[j] = tmp; + std::swap(list[i], list[j]); return true; } diff --git a/test/lit/passes/optimize-instructions-gc-heap.wast b/test/lit/passes/optimize-instructions-gc-heap.wast index 39d7a2330dd..fb418bfe73f 100644 --- a/test/lit/passes/optimize-instructions-gc-heap.wast +++ b/test/lit/passes/optimize-instructions-gc-heap.wast @@ -302,6 +302,47 @@ ) ) + ;; CHECK: (func $dont-swap-subsequent-struct-new (type $1) + ;; CHECK-NEXT: (local $ref (ref null $struct)) + ;; CHECK-NEXT: (local $ref2 (ref null $struct)) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (local.set $ref + ;; CHECK-NEXT: (struct.new $struct + ;; CHECK-NEXT: (i32.const 10) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.set $ref2 + ;; CHECK-NEXT: (struct.new $struct + ;; CHECK-NEXT: (i32.const 20) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (struct.set $struct 0 + ;; CHECK-NEXT: (local.get $ref) + ;; CHECK-NEXT: (i32.const 20) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $dont-swap-subsequent-struct-new + (local $ref (ref null $struct)) + (local $ref2 (ref null $struct)) + (local.set $ref + (struct.new $struct + (i32.const 10) + ) + ) + (nop) + ;; + (local.set $ref2 + (struct.new $struct + (i32.const 20) + ) + ) + ;; last instruction in the block won't be swapped. + (struct.set $struct 0 + (local.get $ref) + (i32.const 20) + ) + ) + ;; CHECK: (func $ref-local-write (type $1) ;; CHECK-NEXT: (local $ref (ref null $struct)) ;; CHECK-NEXT: (local.set $ref From 89129a671da6d254780eade4a00d759dd4ddb932 Mon Sep 17 00:00:00 2001 From: Roberto Lublinerman Date: Fri, 24 May 2024 15:23:29 -0700 Subject: [PATCH 7/7] More fixes. --- test/lit/passes/optimize-instructions-gc-heap.wast | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/lit/passes/optimize-instructions-gc-heap.wast b/test/lit/passes/optimize-instructions-gc-heap.wast index fb418bfe73f..2988b1bd8de 100644 --- a/test/lit/passes/optimize-instructions-gc-heap.wast +++ b/test/lit/passes/optimize-instructions-gc-heap.wast @@ -1,5 +1,5 @@ ;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited. -;; RUN: wasm-opt %s --remove-unused-names --optimize-instructions -all -S -o - \ +;; RUN: wasm-opt %s --remove-unused-names --optimize-instructions -all -S -o - \ ;; RUN: | filecheck %s ;; ;; --remove-unused-names allows the optimizer to see that the blocks have no @@ -330,7 +330,7 @@ ) ) (nop) - ;; + ;; We do not swap with another local.set of struct.new. (local.set $ref2 (struct.new $struct (i32.const 20)