[modules] Factor out cell load into helper function.

src/compiler/js-typed-lowering.cc

 // Copyright 2014 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/js-typed-lowering.h"
 
 #include "src/ast/modules.h"
 #include "src/builtins/builtins-utils.h"
 #include "src/code-factory.h"
 #include "src/compilation-dependencies.h"
@@ skipping 1373 matching lines @@
   }
   node->ReplaceInput(0, context);
   node->ReplaceInput(1, value);
   node->ReplaceInput(2, effect);
   NodeProperties::ChangeOp(
       node,
       simplified()->StoreField(AccessBuilder::ForContextSlot(access.index())));
   return Changed(node);
 }
 
+Node* JSTypedLowering::BuildGetModuleCell(Node* node) {
+  DCHECK(node->opcode() == IrOpcode::kJSLoadModule ||
+         node->opcode() == IrOpcode::kJSStoreModule);
+  Node* effect = NodeProperties::GetEffectInput(node);
+  Node* control = NodeProperties::GetControlInput(node);
+
+  int32_t cell_index = OpParameter<int32_t>(node);
+  Node* module = NodeProperties::GetValueInput(node, 0);
+
+  FieldAccess field_access;
+  int index;
+  if (ModuleDescriptor::GetCellIndexKind(cell_index) ==
+      ModuleDescriptor::kExport) {
+    field_access = AccessBuilder::ForModuleRegularExports();
+    index = cell_index - 1;
+  } else {
+    DCHECK_EQ(ModuleDescriptor::GetCellIndexKind(cell_index),
+              ModuleDescriptor::kImport);
+    field_access = AccessBuilder::ForModuleRegularImports();
+    index = -cell_index - 1;
+  }
+  Node* array = effect = graph()->NewNode(simplified()->LoadField(field_access),
+                                          module, effect, control);
+  return graph()->NewNode(
+      simplified()->LoadField(AccessBuilder::ForFixedArraySlot(index)), array,
+      effect, control);
+}
+
 Reduction JSTypedLowering::ReduceJSLoadModule(Node* node) {
   DCHECK_EQ(IrOpcode::kJSLoadModule, node->opcode());
   Node* effect = NodeProperties::GetEffectInput(node);
   Node* control = NodeProperties::GetControlInput(node);
 
-  int32_t cell_index = OpParameter<int32_t>(node);
-  Node* module = NodeProperties::GetValueInput(node, 0);
-
-  Node* array;
-  int index;
-  if (ModuleDescriptor::GetCellIndexKind(cell_index) ==
-      ModuleDescriptor::kExport) {
-    array = effect = graph()->NewNode(
-        simplified()->LoadField(AccessBuilder::ForModuleRegularExports()),
-        module, effect, control);
-    index = cell_index - 1;
-  } else {
-    DCHECK_EQ(ModuleDescriptor::GetCellIndexKind(cell_index),
-              ModuleDescriptor::kImport);
-    array = effect = graph()->NewNode(
-        simplified()->LoadField(AccessBuilder::ForModuleRegularImports()),
-        module, effect, control);
-    index = -cell_index - 1;
-  }
-
-  Node* cell = effect = graph()->NewNode(
-      simplified()->LoadField(AccessBuilder::ForFixedArraySlot(index)), array,
-      effect, control);
-
+  Node* cell = BuildGetModuleCell(node);
+  if (cell->op()->EffectOutputCount() > 0) effect = cell;

[Michael Starzinger, 2017/04/27 09:51:02]

The returned {LoadField} node will always have an effect output. Can we just
unconditionally say "Node* cell = effect = BuildGetModuleCell(node)" here? If we
want to play it safe we could turn this into a DCHECK.

   Node* value = effect =
       graph()->NewNode(simplified()->LoadField(AccessBuilder::ForCellValue()),
                        cell, effect, control);
 
   ReplaceWithValue(node, value, effect, control);
   return Changed(value);
 }
 
 Reduction JSTypedLowering::ReduceJSStoreModule(Node* node) {
   DCHECK_EQ(IrOpcode::kJSStoreModule, node->opcode());
   Node* effect = NodeProperties::GetEffectInput(node);
   Node* control = NodeProperties::GetControlInput(node);
+  Node* value = NodeProperties::GetValueInput(node, 1);
+  DCHECK_EQ(ModuleDescriptor::GetCellIndexKind(OpParameter<int32_t>(node)),
+            ModuleDescriptor::kExport);
 
-  int32_t cell_index = OpParameter<int32_t>(node);
-  Node* module = NodeProperties::GetValueInput(node, 0);
-  Node* value = NodeProperties::GetValueInput(node, 1);
-
-  Node* array;
-  int index;
-  if (ModuleDescriptor::GetCellIndexKind(cell_index) ==
-      ModuleDescriptor::kExport) {
-    array = effect = graph()->NewNode(
-        simplified()->LoadField(AccessBuilder::ForModuleRegularExports()),
-        module, effect, control);
-    index = cell_index - 1;
-  } else {
-    DCHECK_EQ(ModuleDescriptor::GetCellIndexKind(cell_index),
-              ModuleDescriptor::kImport);
-    array = effect = graph()->NewNode(
-        simplified()->LoadField(AccessBuilder::ForModuleRegularImports()),
-        module, effect, control);
-    index = -cell_index - 1;
-  }
-
-  Node* cell = effect = graph()->NewNode(
-      simplified()->LoadField(AccessBuilder::ForFixedArraySlot(index)), array,
-      effect, control);
-
+  Node* cell = BuildGetModuleCell(node);
+  if (cell->op()->EffectOutputCount() > 0) effect = cell;

[Michael Starzinger, 2017/04/27 09:51:02]

Likewise.

   effect =
       graph()->NewNode(simplified()->StoreField(AccessBuilder::ForCellValue()),
                        cell, value, effect, control);
 
   ReplaceWithValue(node, effect, effect, control);
   return Changed(value);
 }
 
 Reduction JSTypedLowering::ReduceJSConvertReceiver(Node* node) {
   DCHECK_EQ(IrOpcode::kJSConvertReceiver, node->opcode());
@@ skipping 770 matching lines @@
 }
 
 
 CompilationDependencies* JSTypedLowering::dependencies() const {
   return dependencies_;
 }
 
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8