[turbofan] Optimize stores to global properties.

This adds support to also lower stores to global property cells in state
kConstant or kConstantType, where we need to deoptimize eagerly in case
we have a value/type mismatch.

Also fixes bugs in the construction of the frame states in the
AstGraphBuilder.

R=jarin@chromium.org
BUG=v8:4470
LOG=n

Review URL: https://codereview.chromium.org/1398723002

Cr-Commit-Position: refs/heads/master@{#31178}

src/compiler/ast-graph-builder.cc

@@ -1359,7 +1359,7 @@ void AstGraphBuilder::VisitForInStatement(ForInStatement* stmt) {
         // Bind value and do loop body.
         VectorSlotPair feedback =
             CreateVectorSlotPair(stmt->EachFeedbackSlot());
-        VisitForInAssignment(stmt->each(), value, feedback,
+        VisitForInAssignment(stmt->each(), value, feedback, stmt->FilterId(),
                              stmt->AssignmentId());
         VisitIterationBody(stmt, &for_loop);
       }
@@ -1987,7 +1987,8 @@ void AstGraphBuilder::VisitArrayLiteral(ArrayLiteral* expr) {
 
 void AstGraphBuilder::VisitForInAssignment(Expression* expr, Node* value,
                                            const VectorSlotPair& feedback,
-                                           BailoutId bailout_id) {
+                                           BailoutId bailout_id_before,
+                                           BailoutId bailout_id_after) {
   DCHECK(expr->IsValidReferenceExpressionOrThis());
 
   // Left-hand side can only be a property, a global or a variable slot.
@@ -1998,9 +1999,11 @@ void AstGraphBuilder::VisitForInAssignment(Expression* expr, Node* value,
   switch (assign_type) {
     case VARIABLE: {
       Variable* var = expr->AsVariableProxy()->var();
-      FrameStateBeforeAndAfter states(this, BailoutId::None());
-      BuildVariableAssignment(var, value, Token::ASSIGN, feedback, bailout_id,
-                              states);
+      environment()->Push(value);
+      FrameStateBeforeAndAfter states(this, bailout_id_before);
+      value = environment()->Pop();
+      BuildVariableAssignment(var, value, Token::ASSIGN, feedback,
+                              bailout_id_after, states);
       break;
     }
     case NAMED_PROPERTY: {
@@ -2012,7 +2015,8 @@ void AstGraphBuilder::VisitForInAssignment(Expression* expr, Node* value,
       Handle<Name> name = property->key()->AsLiteral()->AsPropertyName();
       Node* store = BuildNamedStore(object, name, value, feedback,
                                     TypeFeedbackId::None());
-      states.AddToNode(store, bailout_id, OutputFrameStateCombine::Ignore());
+      states.AddToNode(store, bailout_id_after,
+                       OutputFrameStateCombine::Ignore());
       break;
     }
     case KEYED_PROPERTY: {
@@ -2025,7 +2029,8 @@ void AstGraphBuilder::VisitForInAssignment(Expression* expr, Node* value,
       value = environment()->Pop();
       Node* store =
           BuildKeyedStore(object, key, value, feedback, TypeFeedbackId::None());
-      states.AddToNode(store, bailout_id, OutputFrameStateCombine::Ignore());
+      states.AddToNode(store, bailout_id_after,
+                       OutputFrameStateCombine::Ignore());
       break;
     }
     case NAMED_SUPER_PROPERTY: {
@@ -2039,7 +2044,8 @@ void AstGraphBuilder::VisitForInAssignment(Expression* expr, Node* value,
       Handle<Name> name = property->key()->AsLiteral()->AsPropertyName();
       Node* store = BuildNamedSuperStore(receiver, home_object, name, value,
                                          TypeFeedbackId::None());
-      states.AddToNode(store, bailout_id, OutputFrameStateCombine::Ignore());
+      states.AddToNode(store, bailout_id_after,
+                       OutputFrameStateCombine::Ignore());
       break;
     }
     case KEYED_SUPER_PROPERTY: {
@@ -2054,7 +2060,8 @@ void AstGraphBuilder::VisitForInAssignment(Expression* expr, Node* value,
       value = environment()->Pop();
       Node* store = BuildKeyedSuperStore(receiver, home_object, key, value,
                                          TypeFeedbackId::None());
-      states.AddToNode(store, bailout_id, OutputFrameStateCombine::Ignore());
+      states.AddToNode(store, bailout_id_after,
+                       OutputFrameStateCombine::Ignore());
       break;
     }
   }
@@ -2675,13 +2682,16 @@ void AstGraphBuilder::VisitCountOperation(CountOperation* expr) {
                       OutputFrameStateCombine::Push());
   }
 
-  // TODO(titzer): combine this framestate with the above?
-  FrameStateBeforeAndAfter store_states(this, assign_type == KEYED_PROPERTY
-                                                  ? expr->ToNumberId()
-                                                  : BailoutId::None());
-
   // Save result for postfix expressions at correct stack depth.
-  if (is_postfix) environment()->Poke(stack_depth, old_value);
+  if (is_postfix) {
+    environment()->Poke(stack_depth, old_value);
+  } else {
+    environment()->Push(old_value);
+  }
+  // TODO(bmeurer): This might not match the fullcodegen in case of non VARIABLE
+  // eager deoptimization; we will figure out when we get there.
+  FrameStateBeforeAndAfter store_states(this, expr->ToNumberId());
+  if (!is_postfix) old_value = environment()->Pop();
 
   // Create node to perform +1/-1 operation.
   Node* value;

src/compiler/ast-graph-builder.h

@@ -402,7 +402,8 @@ class AstGraphBuilder : public AstVisitor {
   // Dispatched from VisitForInStatement.
   void VisitForInAssignment(Expression* expr, Node* value,
                             const VectorSlotPair& feedback,
-                            BailoutId bailout_id);
+                            BailoutId bailout_id_before,
+                            BailoutId bailout_id_after);
 
   // Dispatched from VisitObjectLiteral.
   void VisitObjectLiteralAccessor(Node* home_object,

src/compiler/js-global-specialization.cc

@@ -184,6 +184,7 @@ Reduction JSGlobalSpecialization::ReduceStoreToPropertyCell(
   Node* value = NodeProperties::GetValueInput(node, 2);
   Node* effect = NodeProperties::GetEffectInput(node);
   Node* control = NodeProperties::GetControlInput(node);
+  Node* frame_state = NodeProperties::GetFrameStateInput(node, 1);
   // We only specialize global data property access.
   PropertyDetails property_details = property_cell->property_details();
   DCHECK_EQ(kData, property_details.kind());
@@ -193,19 +194,75 @@ Reduction JSGlobalSpecialization::ReduceStoreToPropertyCell(
   if (property_details.IsReadOnly()) return NoChange();
   // Not much we can do if we run the generic pipeline here.
   if (!(flags() & kTypingEnabled)) return NoChange();
-  // TODO(bmeurer): For now we deal only with cells in mutable state.
-  if (property_details.cell_type() != PropertyCellType::kMutable) {
-    return NoChange();
-  }
-  // Store to non-configurable, data property on the global can be lowered to
-  // a field store, even without deoptimization, because the property cannot be
-  // deleted or reconfigured to an accessor/interceptor property.
-  if (property_details.IsConfigurable()) {
-    // With deoptimization support, we can lower stores even to configurable
-    // data properties on the global object, by adding a code dependency on
-    // the cell.
-    if (!(flags() & kDeoptimizationEnabled)) return NoChange();
-    dependencies()->AssumePropertyCell(property_cell);
+  switch (property_details.cell_type()) {
+    case PropertyCellType::kUndefined: {
+      return NoChange();
+    }
+    case PropertyCellType::kConstant: {
+      // Store to constant property cell requires deoptimization support,
+      // because we might even need to eager deoptimize for mismatch.
+      if (!(flags() & kDeoptimizationEnabled)) return NoChange();
+      dependencies()->AssumePropertyCell(property_cell);
+      Node* check =
+          graph()->NewNode(simplified()->ReferenceEqual(Type::Tagged()), value,
+                           jsgraph()->Constant(property_cell_value));
+      Node* branch =
+          graph()->NewNode(common()->Branch(BranchHint::kTrue), check, control);
+      Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
+      Node* deoptimize = graph()->NewNode(common()->Deoptimize(), frame_state,
+                                          effect, if_false);
+      // TODO(bmeurer): This should be on the AdvancedReducer somehow.
+      NodeProperties::MergeControlToEnd(graph(), common(), deoptimize);
+      control = graph()->NewNode(common()->IfTrue(), branch);
+      return Replace(node, value, effect, control);
+    }
+    case PropertyCellType::kConstantType: {
+      // Store to constant-type property cell requires deoptimization support,
+      // because we might even need to eager deoptimize for mismatch.
+      if (!(flags() & kDeoptimizationEnabled)) return NoChange();
+      dependencies()->AssumePropertyCell(property_cell);
+      Node* check = graph()->NewNode(simplified()->ObjectIsSmi(), value);
+      if (property_cell_value->IsHeapObject()) {
+        Node* branch = graph()->NewNode(common()->Branch(BranchHint::kFalse),
+                                        check, control);
+        Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+        Node* deoptimize = graph()->NewNode(common()->Deoptimize(), frame_state,
+                                            effect, if_true);
+        // TODO(bmeurer): This should be on the AdvancedReducer somehow.
+        NodeProperties::MergeControlToEnd(graph(), common(), deoptimize);
+        control = graph()->NewNode(common()->IfFalse(), branch);
+        Node* value_map =
+            graph()->NewNode(simplified()->LoadField(AccessBuilder::ForMap()),
+                             value, effect, control);
+        Handle<Map> property_cell_value_map(
+            Handle<HeapObject>::cast(property_cell_value)->map(), isolate());
+        check = graph()->NewNode(simplified()->ReferenceEqual(Type::Internal()),
+                                 value_map,
+                                 jsgraph()->Constant(property_cell_value_map));
+      }
+      Node* branch =
+          graph()->NewNode(common()->Branch(BranchHint::kTrue), check, control);
+      Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
+      Node* deoptimize = graph()->NewNode(common()->Deoptimize(), frame_state,
+                                          effect, if_false);
+      // TODO(bmeurer): This should be on the AdvancedReducer somehow.
+      NodeProperties::MergeControlToEnd(graph(), common(), deoptimize);
+      control = graph()->NewNode(common()->IfTrue(), branch);
+      break;
+    }
+    case PropertyCellType::kMutable: {
+      // Store to non-configurable, data property on the global can be lowered
+      // to a field store, even without deoptimization, because the property
+      // cannot be deleted or reconfigured to an accessor/interceptor property.
+      if (property_details.IsConfigurable()) {
+        // With deoptimization support, we can lower stores even to configurable
+        // data properties on the global object, by adding a code dependency on
+        // the cell.
+        if (!(flags() & kDeoptimizationEnabled)) return NoChange();
+        dependencies()->AssumePropertyCell(property_cell);
+      }
+      break;
+    }
   }
   effect = graph()->NewNode(
       simplified()->StoreField(AccessBuilder::ForPropertyCellValue()),
@@ -230,6 +287,11 @@ Isolate* JSGlobalSpecialization::isolate() const {
 }
 
 
+CommonOperatorBuilder* JSGlobalSpecialization::common() const {
+  return jsgraph()->common();
+}
+
+
 JSOperatorBuilder* JSGlobalSpecialization::javascript() const {
   return jsgraph()->javascript();
 }

src/compiler/js-global-specialization.h

@@ -19,6 +19,7 @@ class CompilationDependencies;
 namespace compiler {
 
 // Forward declarations.
+class CommonOperatorBuilder;
 class JSGraph;
 class JSOperatorBuilder;
 
@@ -60,6 +61,7 @@ class JSGlobalSpecialization final : public AdvancedReducer {
   Graph* graph() const;
   JSGraph* jsgraph() const { return jsgraph_; }
   Isolate* isolate() const;
+  CommonOperatorBuilder* common() const;
   JSOperatorBuilder* javascript() const;
   SimplifiedOperatorBuilder* simplified() { return &simplified_; }
   Flags flags() const { return flags_; }