diff options
| -rw-r--r-- | src/passes/OptimizeInstructions.cpp | 61 | ||||
| -rw-r--r-- | test/lit/passes/optimize-instructions-gc.wast | 133 |
2 files changed, 132 insertions, 62 deletions
diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp index ed99705ae..7902eaebd 100644 --- a/src/passes/OptimizeInstructions.cpp +++ b/src/passes/OptimizeInstructions.cpp @@ -1902,6 +1902,19 @@ struct OptimizeInstructions return HeapType::isSubType(a, b) || HeapType::isSubType(b, a); } + bool canBeCastTo(Type a, Type b) { + // A value can be cast to the other if the heap type can be cast, or if a + // null can work. + if (a.isNullable() && b.isNullable()) { + return true; + } + if (a.isRef() && b.isRef() && + canBeCastTo(a.getHeapType(), b.getHeapType())) { + return true; + } + return false; + } + void visitRefCast(RefCast* curr) { // Note we must check the ref's type here and not our own, since we only // refinalize at the end, which means our type may not have been updated yet @@ -1943,26 +1956,21 @@ struct OptimizeInstructions // looking into. } - // For the cast to be able to succeed, the value being cast must be a - // subtype of the desired type. For example, trying to cast an array to a - // struct would be incompatible. - if (!canBeCastTo(curr->ref->type.getHeapType(), intendedType)) { - // This cast cannot succeed. If the input is not a null, it will - // definitely trap. - if (fallthrough->type.isNonNullable()) { - // Make sure to emit a block with the same type as us; leave updating - // types for other passes. - replaceCurrent(builder.makeBlock( - {builder.makeDrop(curr->ref), builder.makeUnreachable()}, - curr->type)); - return; - } - // Otherwise, we are not sure what it is, and need to wait for runtime - // to see if it is a null or not. (We've already handled the case where - // we can see the value is definitely a null at compile time, earlier.) + // Check whether the cast will definitely fail. + if (!canBeCastTo(fallthrough->type, curr->type)) { + // This cast cannot succeed, so it will trap. + // Make sure to emit a block with the same type as us; leave updating + // types for other passes. + replaceCurrent(builder.makeBlock( + {builder.makeDrop(curr->ref), builder.makeUnreachable()}, curr->type)); + return; } // Check whether the cast will definitely succeed. + // + // Note that we could look at the fallthrough for the ref, but that would + // require additional work to make sure we emit something that validates + // properly. TODO if (Type::isSubType(curr->ref->type, curr->type)) { replaceCurrent(curr->ref); @@ -1987,10 +1995,25 @@ struct OptimizeInstructions return; } - // The cast will not definitely succeed, but perhaps the heap type part of - // the cast will, at least. That would leave only nullability as an issue, + // The cast will not definitely succeed nor will it definitely fail. + // + // Perhaps the heap type part of the cast can be reasoned about, at least. + // E.g. if the heap type part of the cast is definitely compatible, but the + // cast as a whole is not, that would leave only nullability as an issue, // that is, this means that the input ref is nullable but we are casting to // non-null. + // + // Note that we could do something similar for a failed cast, that is, + // handle the situation where the entire cast might succeed, but the heap + // type part will definitely fail. For example, the input might be a + // nullable array while the output might be a nullable struct. That is, a + // situation where the only way the cast succeeds is if the input is null. + // However, optimizing this would mean emitting something like + // + // ref == null ? null : trap + // + // which is strictly larger. However, it might be more efficient, so could + // be worth investigating TODO if (HeapType::isSubType(curr->ref->type.getHeapType(), intendedType)) { assert(curr->ref->type.isNullable()); assert(curr->type.isNonNullable()); diff --git a/test/lit/passes/optimize-instructions-gc.wast b/test/lit/passes/optimize-instructions-gc.wast index 04ae7a5cc..abf3da432 100644 --- a/test/lit/passes/optimize-instructions-gc.wast +++ b/test/lit/passes/optimize-instructions-gc.wast @@ -2089,49 +2089,6 @@ ) ) - ;; CHECK: (func $consecutive-opts-with-unreachable (type $funcref_=>_none) (param $func funcref) - ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (ref.cast $struct - ;; CHECK-NEXT: (block (result (ref i31)) - ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (local.get $func) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (unreachable) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; NOMNL: (func $consecutive-opts-with-unreachable (type $funcref_=>_none) (param $func funcref) - ;; NOMNL-NEXT: (drop - ;; NOMNL-NEXT: (ref.cast $struct - ;; NOMNL-NEXT: (block (result (ref i31)) - ;; NOMNL-NEXT: (drop - ;; NOMNL-NEXT: (local.get $func) - ;; NOMNL-NEXT: ) - ;; NOMNL-NEXT: (unreachable) - ;; NOMNL-NEXT: ) - ;; NOMNL-NEXT: ) - ;; NOMNL-NEXT: ) - ;; NOMNL-NEXT: ) - (func $consecutive-opts-with-unreachable (param $func funcref) - (drop - (ref.cast $struct - ;; Casting a funcref to i31 will definitely fail, so this will be - ;; replaced with an unreachable. But it should be enclosed in a block of - ;; the previous type, so that the outside ref.cast null is not confused. This - ;; is a regression test for a bug where we replace this node with an - ;; unreachable one, but we left refinalize til the end of all the other - ;; opts - and that meant that we got to our parent, the ref.cast, with - ;; one unreachable child but before it itself was refinalized, so its - ;; type was *not* unreachable yet, which meant it saw inconsistent IR - ;; that then led to an assertion. - (ref.as_i31 - (local.get $func) - ) - ) - ) - ) - ;; CHECK: (func $ref-cast-static-null (type $void) ;; CHECK-NEXT: (local $a (ref null $A)) ;; CHECK-NEXT: (drop @@ -3183,4 +3140,94 @@ ) ) ) + + ;; CHECK: (func $ref-cast-heap-type-incompatible (type $ref?|$B|_ref|$B|_=>_none) (param $null-b (ref null $B)) (param $b (ref $B)) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (block (result (ref $struct)) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $b) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (block (result (ref $struct)) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $null-b) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (block (result (ref $struct)) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $b) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.cast null $struct + ;; CHECK-NEXT: (local.get $null-b) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; NOMNL: (func $ref-cast-heap-type-incompatible (type $ref?|$B|_ref|$B|_=>_none) (param $null-b (ref null $B)) (param $b (ref $B)) + ;; NOMNL-NEXT: (drop + ;; NOMNL-NEXT: (block (result (ref $struct)) + ;; NOMNL-NEXT: (drop + ;; NOMNL-NEXT: (local.get $b) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: (unreachable) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: (drop + ;; NOMNL-NEXT: (block (result (ref $struct)) + ;; NOMNL-NEXT: (drop + ;; NOMNL-NEXT: (local.get $null-b) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: (unreachable) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: (drop + ;; NOMNL-NEXT: (block (result (ref $struct)) + ;; NOMNL-NEXT: (drop + ;; NOMNL-NEXT: (local.get $b) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: (unreachable) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: (drop + ;; NOMNL-NEXT: (ref.cast null $struct + ;; NOMNL-NEXT: (local.get $null-b) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: ) + ;; NOMNL-NEXT: ) + (func $ref-cast-heap-type-incompatible (param $null-b (ref null $B)) (param $b (ref $B)) + ;; As above, but replacing $A with $struct. Now we have two incompatible + ;; types, $B and $struct, so the only possible way the cast succeeds is if + ;; the cast allows null and the input is a null. + (drop + (ref.cast $struct + (local.get $b) + ) + ) + (drop + (ref.cast $struct + (local.get $null-b) + ) + ) + (drop + (ref.cast null $struct + (local.get $b) + ) + ) + ;; This last case is the only one that can succeed. We leave it alone, but + ;; in theory we could optimize it to { ref == null ? null : trap }. + (drop + (ref.cast null $struct + (local.get $null-b) + ) + ) + ) ) |