[wasm] Fold bounds checks during graph building.

src/compiler/wasm-compiler.cc

Index: src/compiler/wasm-compiler.cc
diff --git a/src/compiler/wasm-compiler.cc b/src/compiler/wasm-compiler.cc
index fa9f9588801cb30763f20174bec67d05f000b99d..b543c80e5722fc7828a65e019371359e0d8a1f01 100644
--- a/src/compiler/wasm-compiler.cc
+++ b/src/compiler/wasm-compiler.cc
@@ -2571,23 +2571,34 @@ Node* WasmGraphBuilder::StoreGlobal(uint32_t index, Node* val) {
 void WasmGraphBuilder::BoundsCheckMem(MachineType memtype, Node* index,
                                       uint32_t offset,
                                       wasm::WasmCodePosition position) {
-  // TODO(turbofan): fold bounds checks for constant indexes.
   DCHECK(module_ && module_->instance);
   size_t size = module_->instance->mem_size;
   byte memsize = wasm::WasmOpcodes::MemSize(memtype);
-  Node* cond;
+
   if (offset >= size || (static_cast<uint64_t>(offset) + memsize) > size) {
-    // The access will always throw.
-    cond = jsgraph()->Int32Constant(0);
-  } else {
-    // Check against the limit.
-    size_t limit = size - offset - memsize;
-    CHECK(limit <= kMaxUInt32);
-    cond = graph()->NewNode(
-        jsgraph()->machine()->Uint32LessThanOrEqual(), index,
-        jsgraph()->Int32Constant(static_cast<uint32_t>(limit)));
+    // The access will always throw (unless memory is grown).
+    Node* cond = jsgraph()->Int32Constant(0);
+    trap_->AddTrapIfFalse(wasm::kTrapMemOutOfBounds, cond, position);

[Clemens Hammacher, 2016/05/09 08:32:31]

Why not trap_->TrapAlways?

[titzer, 2016/05/09 08:38:17]

Yes, that would be great, but the nodes that will be build after the load needs
effect and control dependencies. A natural choice would be {Dead}, but that
causes problems if it is latent in the graph before scheduling (i.e. it requires
running the dead code elimination phase to clean up the graph). We can come back
to that when we want to run the dead code elimination phase and other
optimizations on the graph before scheduling.

+    return;
   }
 
+  // Check against the effective size.
+  size_t effective_size = size - offset - memsize;
+  CHECK(effective_size <= kMaxUInt32);

[Clemens Hammacher, 2016/05/09 08:32:31]

Wouldn't a DCHECK suffice here?

[titzer, 2016/05/09 08:38:17]

I really, really, don't want to wrap around in production, so being extra
careful here.

[Clemens Hammacher, 2016/05/09 08:47:40]

It really should not be possible here, since (offset+memsize <= size) here, and
offset+memsize cannot overflow either.
But I see the point.
And maybe the compiler is even smart enough to remove this check in production
code :)

+
+  Uint32Matcher m(index);
+  if (m.HasValue()) {
+    uint32_t value = m.Value();
+    if (value <= effective_size) {
+      // The bounds check will always succeed.
+      return;
+    }
+  }
+
+  Node* cond = graph()->NewNode(
+      jsgraph()->machine()->Uint32LessThanOrEqual(), index,
+      jsgraph()->Int32Constant(static_cast<uint32_t>(effective_size)));
+
   trap_->AddTrapIfFalse(wasm::kTrapMemOutOfBounds, cond, position);
 }