diff options
-rw-r--r-- | src/ir/LocalGraph.cpp | 28 | ||||
-rw-r--r-- | src/ir/local-graph.h | 8 | ||||
-rw-r--r-- | src/passes/Precompute.cpp | 14 | ||||
-rw-r--r-- | test/lit/passes/precompute-gc.wast | 91 |
4 files changed, 112 insertions, 29 deletions
diff --git a/src/ir/LocalGraph.cpp b/src/ir/LocalGraph.cpp index 7865bc404..beef635b1 100644 --- a/src/ir/LocalGraph.cpp +++ b/src/ir/LocalGraph.cpp @@ -31,6 +31,10 @@ struct Info { std::vector<Expression*> actions; // for each index, the last local.set for it std::unordered_map<Index, LocalSet*> lastSets; + + void dump(Function* func) { + std::cout << " info: " << actions.size() << " actions\n"; + } }; // flow helper class. flows the gets to their sets @@ -106,10 +110,6 @@ struct Flower : public CFGWalker<Flower, Visitor<Flower>, Info> { auto numLocals = func->getNumLocals(); std::vector<FlowBlock*> work; - // Track if we have unreachable code anywhere, as if we do that may inhibit - // certain optimizations below. - bool hasUnreachable = false; - // Convert input blocks (basicBlocks) into more efficient flow blocks to // improve memory access. std::vector<FlowBlock> flowBlocks; @@ -120,11 +120,6 @@ struct Flower : public CFGWalker<Flower, Visitor<Flower>, Info> { for (Index i = 0; i < basicBlocks.size(); ++i) { auto* block = basicBlocks[i].get(); basicToFlowMap[block] = &flowBlocks[i]; - // Check for unreachable code. Note we ignore the entry block (index 0) as - // that is always reached when we are called. - if (i != 0 && block->in.empty()) { - hasUnreachable = true; - } } // We note which local indexes have local.sets, as that can help us @@ -200,19 +195,16 @@ struct Flower : public CFGWalker<Flower, Visitor<Flower>, Info> { if (gets.empty()) { continue; } - if (!hasUnreachable && !hasSet[index]) { + if (!hasSet[index]) { // This local index has no sets, so we know all gets will end up // reaching the entry block. Do that here as an optimization to avoid // flowing through the (potentially very many) blocks in the function. // - // Note that we must check for unreachable code in this function, as - // if there is any then we would not be precise: in that case, the - // gets may either have the entry value, or no value at all. It would - // be safe to mark the entry value in that case anyhow (as it only - // matters in unreachable code), but to keep the IR consistent and to - // avoid confusion when debugging, simply do not optimize if - // there is anything unreachable (which will not happen normally, as - // DCE should run before passes that use this utility). + // Note that we may be in unreachable code, and if so, we might add + // the entry values when they are not actually relevant. That is, we + // are not precise in the case of unreachable code. This can be + // confusing when debugging, but it does not have any downside for + // optimization (since unreachable code should be removed anyhow). for (auto* get : gets) { getSetses[get].insert(nullptr); } diff --git a/src/ir/local-graph.h b/src/ir/local-graph.h index 8cc92d0fd..fd9306d5c 100644 --- a/src/ir/local-graph.h +++ b/src/ir/local-graph.h @@ -30,6 +30,14 @@ namespace wasm { // (see the SSA pass for actually creating new local indexes based // on this). // +// Note that this is not guaranteed to be precise in unreachable code. That is, +// we do not make the effort to represent the exact sets for each get, and may +// overestimate them (specifically, we may mark the entry value as possible, +// even if unreachability prevents that; doing so helps simplify and optimize +// the code, which we think is worthwhile for the possible annoyance in +// debugging etc.; and it has no downside for optimization, since unreachable +// code will be removed anyhow). +// struct LocalGraph { // main API diff --git a/src/passes/Precompute.cpp b/src/passes/Precompute.cpp index cddf8785f..a08848810 100644 --- a/src/passes/Precompute.cpp +++ b/src/passes/Precompute.cpp @@ -442,10 +442,18 @@ private: if (getFunction()->isVar(get->index)) { auto localType = getFunction()->getLocalType(get->index); if (localType.isNonNullable()) { - Fatal() << "Non-nullable local accessing the default value in " - << getFunction()->name << " (" << get->index << ')'; + // This is a non-nullable local that seems to read the default + // value at the function entry. This is either an internal error + // or a case of unreachable code; the latter is possible as + // LocalGraph is not precise in unreachable code. + // + // We cannot set zeros here (as applying them, even in + // unreachable code, would not validate), so just mark this as + // a hopeless case to ignore. + values = {}; + } else { + curr = Literal::makeZeros(localType); } - curr = Literal::makeZeros(localType); } else { // it's a param, so it's hopeless values = {}; diff --git a/test/lit/passes/precompute-gc.wast b/test/lit/passes/precompute-gc.wast index b2e9593d5..3a16eddbc 100644 --- a/test/lit/passes/precompute-gc.wast +++ b/test/lit/passes/precompute-gc.wast @@ -229,7 +229,7 @@ (struct.get $struct 0 (local.get $x)) ) ) - ;; CHECK: (func $ref-comparisons (type $9) (param $x (ref null $struct)) (param $y (ref null $struct)) + ;; CHECK: (func $ref-comparisons (type $10) (param $x (ref null $struct)) (param $y (ref null $struct)) ;; CHECK-NEXT: (local $z (ref null $struct)) ;; CHECK-NEXT: (local $w (ref null $struct)) ;; CHECK-NEXT: (call $log @@ -407,7 +407,7 @@ (local.get $tempresult) ) - ;; CHECK: (func $propagate-different-params (type $10) (param $input1 (ref $empty)) (param $input2 (ref $empty)) (result i32) + ;; CHECK: (func $propagate-different-params (type $11) (param $input1 (ref $empty)) (param $input2 (ref $empty)) (result i32) ;; CHECK-NEXT: (local $tempresult i32) ;; CHECK-NEXT: (local.set $tempresult ;; CHECK-NEXT: (ref.eq @@ -723,7 +723,7 @@ ) ) - ;; CHECK: (func $helper (type $11) (param $0 i32) (result i32) + ;; CHECK: (func $helper (type $12) (param $0 i32) (result i32) ;; CHECK-NEXT: (unreachable) ;; CHECK-NEXT: ) (func $helper (param i32) (result i32) @@ -801,14 +801,14 @@ ) ) - ;; CHECK: (func $receive-f64 (type $12) (param $0 f64) + ;; CHECK: (func $receive-f64 (type $13) (param $0 f64) ;; CHECK-NEXT: (unreachable) ;; CHECK-NEXT: ) (func $receive-f64 (param f64) (unreachable) ) - ;; CHECK: (func $odd-cast-and-get-non-null (type $13) (param $temp (ref $func-return-i32)) + ;; CHECK: (func $odd-cast-and-get-non-null (type $14) (param $temp (ref $func-return-i32)) ;; CHECK-NEXT: (local.set $temp ;; CHECK-NEXT: (ref.cast (ref nofunc) ;; CHECK-NEXT: (ref.func $receive-f64) @@ -857,7 +857,7 @@ ) ) - ;; CHECK: (func $br_on_cast-on-creation (type $14) (result (ref $empty)) + ;; CHECK: (func $br_on_cast-on-creation (type $15) (result (ref $empty)) ;; CHECK-NEXT: (block $label (result (ref $empty)) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (br_on_cast $label (ref $empty) (ref $empty) @@ -952,7 +952,7 @@ ) ) - ;; CHECK: (func $remove-set (type $15) (result (ref func)) + ;; CHECK: (func $remove-set (type $16) (result (ref func)) ;; CHECK-NEXT: (local $nn funcref) ;; CHECK-NEXT: (local $i i32) ;; CHECK-NEXT: (loop $loop @@ -995,7 +995,7 @@ ) ) - ;; CHECK: (func $strings (type $16) (param $param (ref string)) + ;; CHECK: (func $strings (type $17) (param $param (ref string)) ;; CHECK-NEXT: (local $s (ref string)) ;; CHECK-NEXT: (local.set $s ;; CHECK-NEXT: (string.const "hello, world") @@ -1066,4 +1066,79 @@ ) (local.get $x) ) + + ;; CHECK: (func $get-nonnullable-in-unreachable-entry (type $9) (param $x i32) (param $y (ref any)) + ;; CHECK-NEXT: (local $0 (ref any)) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: (local.set $0 + ;; CHECK-NEXT: (local.get $y) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (loop $loop + ;; CHECK-NEXT: (br_if $loop + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $get-nonnullable-in-unreachable-entry (param $x i32) (param $y (ref any)) + (local $0 (ref any)) + ;; As above, but now the first basic block is unreachable, and we need to + ;; detect that specifically, as the block after it *does* have entries even + ;; though it is unreachable (it is a loop, and has itself as an entry). + (unreachable) + (local.set $0 + (local.get $y) + ) + (loop $loop + (br_if $loop + (local.get $x) + ) + (drop + (local.get $0) + ) + ) + ) + + ;; CHECK: (func $get-nonnullable-in-unreachable-later-loop (type $9) (param $x i32) (param $y (ref any)) + ;; CHECK-NEXT: (local $0 (ref any)) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: (local.set $0 + ;; CHECK-NEXT: (local.get $y) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (loop $loop + ;; CHECK-NEXT: (br_if $loop + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $get-nonnullable-in-unreachable-later-loop (param $x i32) (param $y (ref any)) + (local $0 (ref any)) + ;; This |if| is added, which means the loop is later in the function. + ;; Otherwise this is the same as before. + (if + (local.get $x) + (nop) + ) + (unreachable) + (local.set $0 + (local.get $y) + ) + (loop $loop + (br_if $loop + (local.get $x) + ) + (drop + (local.get $0) + ) + ) + ) ) |