[wasm] Fold bounds checks during graph building.

src/compiler/wasm-compiler.cc

 // Copyright 2015 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/compiler/wasm-compiler.h"
 
 #include "src/isolate-inl.h"
 
 #include "src/base/platform/elapsed-timer.h"
 #include "src/base/platform/platform.h"
@@ skipping 2553 matching lines @@
       StoreRepresentation(mem_type.representation(), kNoWriteBarrier));
   Node* node = graph()->NewNode(op, addr, jsgraph()->Int32Constant(0), val,
                                 *effect_, *control_);
   *effect_ = node;
   return node;
 }
 
 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);
 }
 
 Node* WasmGraphBuilder::LoadMem(wasm::LocalType type, MachineType memtype,
                                 Node* index, uint32_t offset,
                                 wasm::WasmCodePosition position) {
   Node* load;
 
   if (module_ && module_->asm_js()) {
     // asm.js semantics use CheckedLoad (i.e. OOB reads return 0ish).
@@ skipping 471 matching lines @@
                                  const wasm::WasmFunction* function) {
   WasmCompilationUnit* unit =
       CreateWasmCompilationUnit(thrower, isolate, module_env, function, 0);
   ExecuteCompilation(unit);
   return FinishCompilation(unit);
 }
 
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8