diff --git a/src/ir/effects.h b/src/ir/effects.h index 60f87d33479..19a5615c391 100644 --- a/src/ir/effects.h +++ b/src/ir/effects.h @@ -20,6 +20,7 @@ #include "ir/intrinsics.h" #include "pass.h" #include "wasm-traversal.h" +#include "wasm.h" namespace wasm { @@ -512,6 +513,32 @@ class EffectAnalyzer { } } + // Handle effects due to an explicit null check of the operands in `exprs`. + // Returns true iff there is no need to consider further effects. + bool trapOnNull(std::initializer_list exprs) { + for (auto* expr : exprs) { + if (expr && expr->type == Type::unreachable) { + return true; + } + } + for (auto* expr : exprs) { + assert(!expr || expr->type.isRef()); + if (expr && expr->type.isNull()) { + parent.trap = true; + return true; + } + } + for (auto* expr : exprs) { + if (expr && expr->type.isNullable()) { + parent.implicitTrap = true; + break; + } + } + return false; + } + + bool trapOnNull(Expression* expr) { return trapOnNull({expr}); } + void visitBlock(Block* curr) { if (curr->name.is()) { parent.breakTargets.erase(curr->name); // these were internal breaks @@ -830,11 +857,12 @@ class EffectAnalyzer { } } void visitThrowRef(ThrowRef* curr) { + if (trapOnNull(curr->exnref)) { + return; + } if (parent.tryDepth == 0) { parent.throws_ = true; } - // traps when the arg is null - parent.implicitTrap = true; } void visitNop(Nop* curr) {} void visitUnreachable(Unreachable* curr) { parent.trap = true; } @@ -846,28 +874,17 @@ class EffectAnalyzer { void visitTupleMake(TupleMake* curr) {} void visitTupleExtract(TupleExtract* curr) {} void visitRefI31(RefI31* curr) {} - void visitI31Get(I31Get* curr) { - // traps when the ref is null - if (curr->i31->type.isNullable()) { - parent.implicitTrap = true; - } - } + void visitI31Get(I31Get* curr) { trapOnNull(curr->i31); } void visitCallRef(CallRef* curr) { + if (trapOnNull(curr->target)) { + return; + } if (curr->isReturn) { parent.branchesOut = true; if (parent.features.hasExceptionHandling()) { parent.hasReturnCallThrow = true; } } - if (curr->target->type.isNull()) { - parent.trap = true; - return; - } - // traps when the call target is null - if (curr->target->type.isNullable()) { - parent.implicitTrap = true; - } - parent.calls = true; if (parent.features.hasExceptionHandling() && (parent.tryDepth == 0 && !curr->isReturn)) { @@ -875,36 +892,24 @@ class EffectAnalyzer { } } void visitRefTest(RefTest* curr) {} - void maybeHandleDescriptor(Expression* desc) { - if (desc) { - // Traps when the descriptor is null. - if (desc->type.isNull()) { - parent.trap = true; - } else if (desc->type.isNullable()) { - parent.implicitTrap = true; - } - } - } + void visitRefCast(RefCast* curr) { + if (trapOnNull(curr->desc)) { + return; + } // Traps if the cast fails. parent.implicitTrap = true; - maybeHandleDescriptor(curr->desc); - } - void visitRefGetDesc(RefGetDesc* curr) { - // Traps if the ref is null. - parent.implicitTrap = true; } + void visitRefGetDesc(RefGetDesc* curr) { trapOnNull(curr->ref); } void visitBrOn(BrOn* curr) { + if (trapOnNull(curr->desc)) { + return; + } parent.breakTargets.insert(curr->name); - maybeHandleDescriptor(curr->desc); } - void visitStructNew(StructNew* curr) { maybeHandleDescriptor(curr->desc); } + void visitStructNew(StructNew* curr) { trapOnNull(curr->desc); } void visitStructGet(StructGet* curr) { - if (curr->ref->type == Type::unreachable) { - return; - } - if (curr->ref->type.isNull()) { - parent.trap = true; + if (trapOnNull(curr->ref)) { return; } if (curr->ref->type.getHeapType() @@ -913,63 +918,41 @@ class EffectAnalyzer { .mutable_ == Mutable) { parent.readsMutableStruct = true; } - // traps when the arg is null - if (curr->ref->type.isNullable()) { - parent.implicitTrap = true; - } parent.isAtomic |= curr->isAtomic() && curr->ref->type.getHeapType().isShared(); } void visitStructSet(StructSet* curr) { - if (curr->ref->type.isNull()) { - parent.trap = true; + if (trapOnNull(curr->ref)) { return; } parent.writesStruct = true; - // traps when the arg is null - if (curr->ref->type.isNullable()) { - parent.implicitTrap = true; - } parent.isAtomic |= curr->isAtomic() && curr->ref->type.getHeapType().isShared(); } void visitStructRMW(StructRMW* curr) { - if (curr->ref->type.isNull()) { - parent.trap = true; + if (trapOnNull(curr->ref)) { return; } parent.readsMutableStruct = true; parent.writesStruct = true; - if (curr->ref->type.isNullable()) { - parent.implicitTrap = true; - } assert(curr->order != MemoryOrder::Unordered); parent.isAtomic = true; } void visitStructCmpxchg(StructCmpxchg* curr) { - if (curr->ref->type.isNull()) { - parent.trap = true; + if (trapOnNull(curr->ref)) { return; } parent.readsMutableStruct = true; parent.writesStruct = true; - if (curr->ref->type.isNullable()) { - parent.implicitTrap = true; - } assert(curr->order != MemoryOrder::Unordered); parent.isAtomic = true; } void visitStructWait(StructWait* curr) { - if (curr->ref->type.isNull()) { - parent.trap = true; + if (trapOnNull(curr->ref)) { return; } parent.isAtomic = true; - if (curr->ref->type.isNullable()) { - parent.implicitTrap = true; - } - // If the timeout is negative and no-one wakes us. parent.mayNotReturn = true; @@ -978,10 +961,6 @@ class EffectAnalyzer { // isAtomic == true). parent.writesStruct = true; - if (curr->ref->type == Type::unreachable) { - return; - } - // If the ref isn't `unreachable`, then the field must exist and be a // packed waitqueue due to validation. assert(curr->ref->type.isStruct()); @@ -998,16 +977,11 @@ class EffectAnalyzer { .mutable_ == Mutable; } void visitStructNotify(StructNotify* curr) { - if (curr->ref->type.isNull()) { - parent.trap = true; + if (trapOnNull(curr->ref)) { return; } parent.isAtomic = true; - if (curr->ref->type.isNullable()) { - parent.implicitTrap = true; - } - // struct.notify mutates an opaque waiter queue which isn't visible in // user code. Model this as a struct write which prevents reorderings // (since isAtomic == true). @@ -1026,8 +1000,7 @@ class EffectAnalyzer { } void visitArrayNewFixed(ArrayNewFixed* curr) {} void visitArrayGet(ArrayGet* curr) { - if (curr->ref->type.isNull()) { - parent.trap = true; + if (trapOnNull(curr->ref)) { return; } parent.readsArray = true; @@ -1037,8 +1010,7 @@ class EffectAnalyzer { curr->isAtomic() && curr->ref->type.getHeapType().isShared(); } void visitArraySet(ArraySet* curr) { - if (curr->ref->type.isNull()) { - parent.trap = true; + if (trapOnNull(curr->ref)) { return; } parent.writesArray = true; @@ -1048,18 +1020,12 @@ class EffectAnalyzer { curr->isAtomic() && curr->ref->type.getHeapType().isShared(); } void visitArrayLen(ArrayLen* curr) { - if (curr->ref->type.isNull()) { - parent.trap = true; + if (trapOnNull(curr->ref)) { return; } - // traps when the arg is null - if (curr->ref->type.isNullable()) { - parent.implicitTrap = true; - } } void visitArrayCopy(ArrayCopy* curr) { - if (curr->destRef->type.isNull() || curr->srcRef->type.isNull()) { - parent.trap = true; + if (trapOnNull({curr->srcRef, curr->destRef})) { return; } parent.readsArray = true; @@ -1068,8 +1034,7 @@ class EffectAnalyzer { parent.implicitTrap = true; } void visitArrayFill(ArrayFill* curr) { - if (curr->ref->type.isNull()) { - parent.trap = true; + if (trapOnNull(curr->ref)) { return; } parent.writesArray = true; @@ -1077,37 +1042,33 @@ class EffectAnalyzer { parent.implicitTrap = true; } template void visitArrayInit(ArrayInit* curr) { - if (curr->ref->type.isNull()) { - parent.trap = true; + if (trapOnNull(curr->ref)) { return; } parent.writesArray = true; - // Traps when the destination is null, when out of bounds in source or - // destination, or when the source segment has been dropped. + // OOB access to array or element segment. parent.implicitTrap = true; } void visitArrayInitData(ArrayInitData* curr) { visitArrayInit(curr); } void visitArrayInitElem(ArrayInitElem* curr) { visitArrayInit(curr); } void visitArrayRMW(ArrayRMW* curr) { - if (curr->ref->type.isNull()) { - parent.trap = true; + if (trapOnNull(curr->ref)) { return; } parent.readsArray = true; parent.writesArray = true; - // traps when the arg is null or the index out of bounds + // OOB access to array. parent.implicitTrap = true; assert(curr->order != MemoryOrder::Unordered); parent.isAtomic = true; } void visitArrayCmpxchg(ArrayCmpxchg* curr) { - if (curr->ref->type.isNull()) { - parent.trap = true; + if (trapOnNull(curr->ref)) { return; } parent.readsArray = true; parent.writesArray = true; - // traps when the arg is null or the index out of bounds + // OOB access to array. parent.implicitTrap = true; assert(curr->order != MemoryOrder::Unordered); parent.isAtomic = true; @@ -1117,63 +1078,69 @@ class EffectAnalyzer { // These conversions are infallible. return; } - // traps when the arg is not valid - parent.implicitTrap = true; - // Note: We could be more precise here and report the lack of a possible - // trap if the input is non-nullable (and also of the right kind for - // RefAsFunc etc.). However, we have optimization passes that will - // remove a RefAs in such a case (in OptimizeInstructions, and also - // Vacuum in trapsNeverHappen mode), so duplicating that code here would - // only help until the next time those optimizations run. As a tradeoff, - // we keep the code here simpler, but it does mean another optimization - // cycle may be needed in some cases. + assert(curr->op == RefAsNonNull); + trapOnNull(curr->value); } void visitStringNew(StringNew* curr) { - // traps when ref is null - parent.implicitTrap = true; - if (curr->op != StringNewFromCodePoint) { - parent.readsArray = true; + switch (curr->op) { + case StringNewLossyUTF8Array: + case StringNewWTF16Array: + if (trapOnNull(curr->ref)) { + return; + } + // OOB access to array. + parent.implicitTrap = true; + parent.readsArray = true; + return; + case StringNewFromCodePoint: + // Invalid code points. + parent.implicitTrap = true; + return; } } void visitStringConst(StringConst* curr) {} - void visitStringMeasure(StringMeasure* curr) { - // traps when ref is null. - parent.implicitTrap = true; - } + void visitStringMeasure(StringMeasure* curr) { trapOnNull(curr->ref); } void visitStringEncode(StringEncode* curr) { - // traps when ref is null or we write out of bounds. + if (trapOnNull({curr->str, curr->array})) { + return; + } + // OOB array access. parent.implicitTrap = true; parent.writesArray = true; } void visitStringConcat(StringConcat* curr) { - // traps when an input is null. - parent.implicitTrap = true; + trapOnNull({curr->left, curr->right}); } void visitStringEq(StringEq* curr) { - if (curr->op == StringEqCompare) { - // traps when either input is null. - if (curr->left->type.isNullable() || curr->right->type.isNullable()) { - parent.implicitTrap = true; - } + switch (curr->op) { + case StringEqEqual: + // Nulls are ok. + return; + case StringEqCompare: + trapOnNull({curr->left, curr->right}); + return; } } void visitStringTest(StringTest* curr) {} void visitStringWTF16Get(StringWTF16Get* curr) { - // traps when ref is null. + if (trapOnNull(curr->ref)) { + return; + } + // OOB string access. parent.implicitTrap = true; } void visitStringSliceWTF(StringSliceWTF* curr) { - // traps when ref is null. - parent.implicitTrap = true; - } - void visitContNew(ContNew* curr) { - // traps when curr->func is null ref. + if (trapOnNull(curr->ref)) { + return; + } + // OOB string access. parent.implicitTrap = true; } + void visitContNew(ContNew* curr) { trapOnNull(curr->func); } void visitContBind(ContBind* curr) { - // traps when curr->cont is null ref. - parent.implicitTrap = true; - + if (trapOnNull(curr->cont)) { + return; + } // The input continuation is modified, as it will trap if resumed. This is // a globally-noticeable effect, which we model as a call for now, but we // could in theory use something more refined here (|modifiesContinuation| @@ -1192,37 +1159,36 @@ class EffectAnalyzer { parent.implicitTrap = true; } void visitResume(Resume* curr) { + if (trapOnNull(curr->cont)) { + return; + } // This acts as a kitchen sink effect. parent.calls = true; - // resume instructions accept nullable continuation references and trap - // on null. - parent.implicitTrap = true; - if (parent.features.hasExceptionHandling() && parent.tryDepth == 0) { parent.throws_ = true; } } void visitResumeThrow(ResumeThrow* curr) { + if (trapOnNull(curr->cont)) { + return; + } + // This acts as a kitchen sink effect. parent.calls = true; - // resume_throw instructions accept nullable continuation - // references and trap on null. - parent.implicitTrap = true; - if (parent.features.hasExceptionHandling() && parent.tryDepth == 0) { parent.throws_ = true; } } void visitStackSwitch(StackSwitch* curr) { + if (trapOnNull(curr->cont)) { + return; + } + // This acts as a kitchen sink effect. parent.calls = true; - // switch instructions accept nullable continuation references - // and trap on null. - parent.implicitTrap = true; - if (parent.features.hasExceptionHandling() && parent.tryDepth == 0) { parent.throws_ = true; } diff --git a/src/passes/OptimizeCasts.cpp b/src/passes/OptimizeCasts.cpp index aa7168f97c2..55f9a72899e 100644 --- a/src/passes/OptimizeCasts.cpp +++ b/src/passes/OptimizeCasts.cpp @@ -168,8 +168,13 @@ struct EarlyCastFinder // TODO: generalize this when we handle more than RefAsNonNull. RefCast dummyRefCast(module->allocator); dummyRefCast.desc = nullptr; + // Use an arbitrary nullable reference operand to get conservative + // ref.as_non_null effects. + LocalGet dummyRefAsOperand(module->allocator); + dummyRefAsOperand.type = Type(HeapType::any, Nullable); RefAs dummyRefAs(module->allocator); dummyRefAs.op = RefAsNonNull; + dummyRefAs.value = &dummyRefAsOperand; testRefCast.visit(&dummyRefCast); testRefAs.visit(&dummyRefAs); diff --git a/test/lit/passes/gufa-desc.wast b/test/lit/passes/gufa-desc.wast index 50903078792..c5a3face6c4 100644 --- a/test/lit/passes/gufa-desc.wast +++ b/test/lit/passes/gufa-desc.wast @@ -12,9 +12,9 @@ (type $desc (sub (describes $struct) (struct (field funcref)))) ) - ;; CHECK: (type $2 (func (result i32))) + ;; CHECK: (type $2 (func)) - ;; CHECK: (type $3 (func)) + ;; CHECK: (type $3 (func (result i32))) ;; CHECK: (global $desc (ref (exact $desc)) (struct.new $desc ;; CHECK-NEXT: (ref.func $func) @@ -42,38 +42,19 @@ ;; CHECK: (export "struct" (func $struct)) - ;; CHECK: (func $func (type $2) (result i32) + ;; CHECK: (func $func (type $3) (result i32) ;; CHECK-NEXT: (i32.const -1) ;; CHECK-NEXT: ) (func $func (result i32) (i32.const -1) ) - ;; CHECK: (func $desc (type $3) + ;; CHECK: (func $desc (type $2) ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (block (result (ref (exact $desc))) - ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (ref.get_desc $struct - ;; CHECK-NEXT: (global.get $struct) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (global.get $desc) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (global.get $desc) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (block (result (ref (exact $2))) - ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (block (result (ref (exact $desc))) - ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (ref.get_desc $struct - ;; CHECK-NEXT: (global.get $struct) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (global.get $desc) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (ref.func $func) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (ref.func $func) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $desc (export "desc") @@ -93,7 +74,7 @@ ) ) - ;; CHECK: (func $struct (type $3) + ;; CHECK: (func $struct (type $2) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (i32.const 100) ;; CHECK-NEXT: ) @@ -214,14 +195,7 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (block (result (ref (exact $6))) - ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (ref.get_desc $struct - ;; CHECK-NEXT: (global.get $struct) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (ref.func $func) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (ref.func $func) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $desc (export "desc") @@ -288,14 +262,7 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (block (result (ref (exact $subdesc))) - ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (ref.get_desc $sub - ;; CHECK-NEXT: (local.get $temp) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (global.get $subdesc) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (global.get $subdesc) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (i32.const 100) @@ -452,10 +419,8 @@ ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (block (result (ref (exact $desc))) ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (ref.get_desc $struct - ;; CHECK-NEXT: (ref.cast (ref (exact $struct)) - ;; CHECK-NEXT: (global.get $struct) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (ref.cast (ref (exact $struct)) + ;; CHECK-NEXT: (global.get $struct) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (global.get $desc) @@ -473,10 +438,8 @@ ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (block (result (ref (exact $desc))) ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (ref.get_desc $struct - ;; CHECK-NEXT: (ref.cast (ref (exact $struct)) - ;; CHECK-NEXT: (global.get $struct) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (ref.cast (ref (exact $struct)) + ;; CHECK-NEXT: (global.get $struct) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (global.get $desc) @@ -576,46 +539,13 @@ ;; CHECK: (func $test (type $3) ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (block (result (ref (exact $B))) - ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (ref.get_desc $A - ;; CHECK-NEXT: (global.get $A) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (global.get $B) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (global.get $B) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (block (result (ref (exact $C))) - ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (ref.get_desc $B - ;; CHECK-NEXT: (global.get $B) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (global.get $C) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (global.get $C) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (block (result i32) - ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (block (result (ref (exact $C))) - ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (ref.get_desc $B - ;; CHECK-NEXT: (block (result (ref (exact $B))) - ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (ref.get_desc $A - ;; CHECK-NEXT: (global.get $A) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (global.get $B) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (global.get $C) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (i32.const 10) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (i32.const 10) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $test (export "test") diff --git a/test/lit/passes/type-refining-gufa.wast b/test/lit/passes/type-refining-gufa.wast index a33b70c29b4..d6330890d8f 100644 --- a/test/lit/passes/type-refining-gufa.wast +++ b/test/lit/passes/type-refining-gufa.wast @@ -524,8 +524,6 @@ ;; GUFA: (export "a" (func $a)) ;; O3O3: (type $3 (func (result (ref $cont)))) - ;; O3O3: (elem declare func $ref) - ;; O3O3: (export "a" (func $a)) (export "a" (func $a)) ;; NRML: (export "b" (func $b)) @@ -556,15 +554,10 @@ ;; GUFA-NEXT: ) ;; GUFA-NEXT: ) ;; O3O3: (func $a (type $func) - ;; O3O3-NEXT: (drop - ;; O3O3-NEXT: (cont.new $cont - ;; O3O3-NEXT: (ref.func $ref) - ;; O3O3-NEXT: ) - ;; O3O3-NEXT: ) + ;; O3O3-NEXT: (nop) ;; O3O3-NEXT: ) (func $a - ;; GUFA cannot improve things here (-O3 can remove the struct operations, - ;; though). + ;; GUFA cannot improve things here (-O3 can remove everything, though). (drop (struct.get $wrap-cont 0 (struct.new $wrap-cont @@ -607,9 +600,6 @@ ;; GUFA-NEXT: ) ;; GUFA-NEXT: ) ;; GUFA-NEXT: ) - ;; O3O3: (func $ref (type $func) - ;; O3O3-NEXT: (nop) - ;; O3O3-NEXT: ) (func $ref (type $func) (drop (struct.new $wrap-array