[ignition] Fix building lookup graph when search depth is 0

src/compiler/bytecode-graph-builder.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/bytecode-graph-builder.h"
 
 #include "src/ast/ast.h"
 #include "src/ast/scopes.h"
 #include "src/compilation-info.h"
 #include "src/compiler/bytecode-branch-analysis.h"
@@ skipping 878 matching lines @@
 
 void BytecodeGraphBuilder::VisitLdaLookupSlotInsideTypeof() {
   BuildLdaLookupSlot(TypeofMode::INSIDE_TYPEOF);
 }
 
 BytecodeGraphBuilder::Environment* BytecodeGraphBuilder::CheckContextExtensions(
     uint32_t depth) {
   // Output environment where the context has an extension
   Environment* slow_environment = nullptr;
 
-  DCHECK_GT(depth, 0u);
-
   // We only need to check up to the last-but-one depth, because the an eval in
   // the same scope as the variable itself has no way of shadowing it.
   for (uint32_t d = 0; d < depth; d++) {
     Node* extension_slot =
         NewNode(javascript()->LoadContext(d, Context::EXTENSION_INDEX, false),
                 environment()->Context());
 
     Node* check_no_extension =
         NewNode(javascript()->StrictEqual(CompareOperationHint::kAny),
                 extension_slot, jsgraph()->TheHoleConstant());
@@ skipping 13 matching lines @@
     }
 
     {
       set_environment(true_environment);
       NewIfTrue();
       // Do nothing on if there is no extension, eventually falling through to
       // the fast path.
     }
   }
 
-  DCHECK_NOT_NULL(slow_environment);
+  // The depth can be zero, in which case no slow-path checks are built, and the
+  // slow path environment can be null.
+  DCHECK(depth == 0 || slow_environment != nullptr);
 
   return slow_environment;
 }
 
 void BytecodeGraphBuilder::BuildLdaLookupContextSlot(TypeofMode typeof_mode) {
   uint32_t depth = bytecode_iterator().GetUnsignedImmediateOperand(2);
 
   // Check if any context in the depth has an extension.
   Environment* slow_environment = CheckContextExtensions(depth);
 
   // Fast path, do a context load.
   {
     uint32_t slot_index = bytecode_iterator().GetIndexOperand(1);
 
     const Operator* op = javascript()->LoadContext(depth, slot_index, false);
     Node* context = environment()->Context();
     environment()->BindAccumulator(NewNode(op, context));
     NewMerge();

[Michael Starzinger, 2016/10/04 09:43:10]

nit: Shouldn't we only create the singleton merge node in the case we have a
slow path (i.e. move it into the below conditional)?

[Leszek Swirski, 2016/10/04 10:46:55]

Done.

   }
-  Environment* fast_environment = environment();
 
-  // Slow path, do a runtime load lookup.
-  set_environment(slow_environment);
-  {
-    FrameStateBeforeAndAfter states(this);
+  // Only build the slow path if there were any slow-path checks.
+  if (slow_environment != nullptr) {
+    Environment* fast_environment = environment();
 
-    Node* name =
-        jsgraph()->Constant(bytecode_iterator().GetConstantForIndexOperand(0));
+    // Slow path, do a runtime load lookup.
+    set_environment(slow_environment);
+    {
+      FrameStateBeforeAndAfter states(this);
 
-    const Operator* op =
-        javascript()->CallRuntime(typeof_mode == TypeofMode::NOT_INSIDE_TYPEOF
-                                      ? Runtime::kLoadLookupSlot
-                                      : Runtime::kLoadLookupSlotInsideTypeof);
-    Node* value = NewNode(op, name);
-    environment()->BindAccumulator(value, &states);
+      Node* name = jsgraph()->Constant(
+          bytecode_iterator().GetConstantForIndexOperand(0));
+
+      const Operator* op =
+          javascript()->CallRuntime(typeof_mode == TypeofMode::NOT_INSIDE_TYPEOF
+                                        ? Runtime::kLoadLookupSlot
+                                        : Runtime::kLoadLookupSlotInsideTypeof);
+      Node* value = NewNode(op, name);
+      environment()->BindAccumulator(value, &states);
+    }
+
+    fast_environment->Merge(environment());
+    set_environment(fast_environment);
   }
-
-  fast_environment->Merge(environment());
-  set_environment(fast_environment);
 }
 
 void BytecodeGraphBuilder::VisitLdaLookupContextSlot() {
   BuildLdaLookupContextSlot(TypeofMode::NOT_INSIDE_TYPEOF);
 }
 
 void BytecodeGraphBuilder::VisitLdaLookupContextSlotInsideTypeof() {
   BuildLdaLookupContextSlot(TypeofMode::INSIDE_TYPEOF);
 }
 
 void BytecodeGraphBuilder::BuildLdaLookupGlobalSlot(TypeofMode typeof_mode) {
   uint32_t depth = bytecode_iterator().GetUnsignedImmediateOperand(2);
 
   // Check if any context in the depth has an extension.
   Environment* slow_environment = CheckContextExtensions(depth);
 
   // Fast path, do a global load.
   {
     FrameStateBeforeAndAfter states(this);
     Node* node =
         BuildLoadGlobal(bytecode_iterator().GetIndexOperand(1), typeof_mode);
     environment()->BindAccumulator(node, &states);
 
     NewMerge();

[Michael Starzinger, 2016/10/04 09:43:10]

nit: Likewise.

[Leszek Swirski, 2016/10/04 10:46:55]

Done.

   }
-  Environment* fast_environment = environment();
 
-  // Slow path, do a runtime load lookup.
-  set_environment(slow_environment);
-  {
-    FrameStateBeforeAndAfter states(this);
+  // Only build the slow path if there were any slow-path checks.
+  if (slow_environment != nullptr) {
+    Environment* fast_environment = environment();
 
-    Node* name =
-        jsgraph()->Constant(bytecode_iterator().GetConstantForIndexOperand(0));
+    // Slow path, do a runtime load lookup.
+    set_environment(slow_environment);
+    {
+      FrameStateBeforeAndAfter states(this);
 
-    const Operator* op =
-        javascript()->CallRuntime(typeof_mode == TypeofMode::NOT_INSIDE_TYPEOF
-                                      ? Runtime::kLoadLookupSlot
-                                      : Runtime::kLoadLookupSlotInsideTypeof);
-    Node* value = NewNode(op, name);
-    environment()->BindAccumulator(value, &states);
+      Node* name = jsgraph()->Constant(
+          bytecode_iterator().GetConstantForIndexOperand(0));
+
+      const Operator* op =
+          javascript()->CallRuntime(typeof_mode == TypeofMode::NOT_INSIDE_TYPEOF
+                                        ? Runtime::kLoadLookupSlot
+                                        : Runtime::kLoadLookupSlotInsideTypeof);
+      Node* value = NewNode(op, name);
+      environment()->BindAccumulator(value, &states);
+    }
+
+    fast_environment->Merge(environment());
+    set_environment(fast_environment);
   }
-
-  fast_environment->Merge(environment());
-  set_environment(fast_environment);
 }
 
 void BytecodeGraphBuilder::VisitLdaLookupGlobalSlot() {
   BuildLdaLookupGlobalSlot(TypeofMode::NOT_INSIDE_TYPEOF);
 }
 
 void BytecodeGraphBuilder::VisitLdaLookupGlobalSlotInsideTypeof() {
   BuildLdaLookupGlobalSlot(TypeofMode::INSIDE_TYPEOF);
 }
 
@@ skipping 1196 matching lines @@
     // Phi does not exist yet, introduce one.
     value = NewPhi(inputs, value, control);
     value->ReplaceInput(inputs - 1, other);
   }
   return value;
 }
 
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8