summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--src/ir/LocalGraph.cpp28
-rw-r--r--src/ir/local-graph.h8
-rw-r--r--src/passes/Precompute.cpp14
-rw-r--r--test/lit/passes/precompute-gc.wast91
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)
+ )
+ )
+ )
)