[turbofan] Add support for accessor inlining.

Allow inlining of getters and setters into TurboFan optimized code.
This just adds the basic machinery required to essentially inline
the setter and getter dispatch code for the (keyed) load/store ICs.
There'll be follow up CLs to also actually inline some of the interesting
accessor functions itself, like the byteLength and friends for the
TypedArrays.

R=jarin@chromium.org

Review-Url: https://codereview.chromium.org/2198473002
Cr-Commit-Position: refs/heads/master@{#38192}

src/bootstrapper.cc

@@ -372,6 +372,7 @@ Handle<JSFunction> InstallGetter(Handle<JSObject> target,
   Handle<Object> setter = target->GetIsolate()->factory()->undefined_value();
   JSObject::DefineAccessor(target, property_name, getter, setter, attributes)
       .Check();
+  getter->shared()->set_native(true);
   return getter;
 }
 

src/compiler.h

@@ -162,6 +162,7 @@ class CompilationInfo final {
     kBailoutOnUninitialized = 1 << 16,
     kOptimizeFromBytecode = 1 << 17,
     kTypeFeedbackEnabled = 1 << 18,
+    kAccessorInliningEnabled = 1 << 19,
   };
 
   CompilationInfo(ParseInfo* parse_info, Handle<JSFunction> closure);
@@ -278,6 +279,12 @@ class CompilationInfo final {
     return GetFlag(kTypeFeedbackEnabled);
   }
 
+  void MarkAsAccessorInliningEnabled() { SetFlag(kAccessorInliningEnabled); }
+
+  bool is_accessor_inlining_enabled() const {
+    return GetFlag(kAccessorInliningEnabled);
+  }
+
   void MarkAsSourcePositionsEnabled() { SetFlag(kSourcePositionsEnabled); }
 
   bool is_source_positions_enabled() const {

src/compiler/access-info.cc

@@ -75,7 +75,7 @@ PropertyAccessInfo PropertyAccessInfo::NotFound(MapList const& receiver_maps,
 PropertyAccessInfo PropertyAccessInfo::DataConstant(
     MapList const& receiver_maps, Handle<Object> constant,
     MaybeHandle<JSObject> holder) {
-  return PropertyAccessInfo(holder, constant, receiver_maps);
+  return PropertyAccessInfo(kDataConstant, holder, constant, receiver_maps);
 }
 
 // static
@@ -86,6 +86,13 @@ PropertyAccessInfo PropertyAccessInfo::DataField(
                             receiver_maps);
 }
 
+// static
+PropertyAccessInfo PropertyAccessInfo::AccessorConstant(
+    MapList const& receiver_maps, Handle<Object> constant,
+    MaybeHandle<JSObject> holder) {
+  return PropertyAccessInfo(kAccessorConstant, holder, constant, receiver_maps);
+}
+
 PropertyAccessInfo::PropertyAccessInfo()
     : kind_(kInvalid), field_type_(Type::Any()) {}
 
@@ -96,10 +103,10 @@ PropertyAccessInfo::PropertyAccessInfo(MaybeHandle<JSObject> holder,
       holder_(holder),
       field_type_(Type::Any()) {}
 
-PropertyAccessInfo::PropertyAccessInfo(MaybeHandle<JSObject> holder,
+PropertyAccessInfo::PropertyAccessInfo(Kind kind, MaybeHandle<JSObject> holder,
                                        Handle<Object> constant,
                                        MapList const& receiver_maps)
-    : kind_(kDataConstant),
+    : kind_(kind),
       receiver_maps_(receiver_maps),
       constant_(constant),
       holder_(holder),
@@ -141,7 +148,8 @@ bool PropertyAccessInfo::Merge(PropertyAccessInfo const* that) {
       return false;
     }
 
-    case kDataConstant: {
+    case kDataConstant:
+    case kAccessorConstant: {
       // Check if we actually access the same constant.
       if (this->constant_.address() == that->constant_.address()) {
         this->receiver_maps_.insert(this->receiver_maps_.end(),
@@ -288,50 +296,73 @@ bool AccessInfoFactory::ComputePropertyAccessInfo(
           return LookupTransition(receiver_map, name, holder, access_info);
         }
       }
-      if (details.type() == DATA_CONSTANT) {
-        *access_info = PropertyAccessInfo::DataConstant(
-            MapList{receiver_map},
-            handle(descriptors->GetValue(number), isolate()), holder);
-        return true;
-      } else if (details.type() == DATA) {
-        int index = descriptors->GetFieldIndex(number);
-        Representation field_representation = details.representation();
-        FieldIndex field_index = FieldIndex::ForPropertyIndex(
-            *map, index, field_representation.IsDouble());
-        Type* field_type = Type::Tagged();
-        if (field_representation.IsSmi()) {
-          field_type = type_cache_.kSmi;
-        } else if (field_representation.IsDouble()) {
-          field_type = type_cache_.kFloat64;
-        } else if (field_representation.IsHeapObject()) {
-          // Extract the field type from the property details (make sure its
-          // representation is TaggedPointer to reflect the heap object case).
-          field_type = Type::Intersect(
-              descriptors->GetFieldType(number)->Convert(zone()),
-              Type::TaggedPointer(), zone());
-          if (field_type->Is(Type::None())) {
-            // Store is not safe if the field type was cleared.
-            if (access_mode == AccessMode::kStore) return false;
-
-            // The field type was cleared by the GC, so we don't know anything
-            // about the contents now.
-            // TODO(bmeurer): It would be awesome to make this saner in the
-            // runtime/GC interaction.
-            field_type = Type::TaggedPointer();
-          } else if (!Type::Any()->Is(field_type)) {
-            // Add proper code dependencies in case of stable field map(s).
-            Handle<Map> field_owner_map(map->FindFieldOwner(number), isolate());
-            dependencies()->AssumeFieldType(field_owner_map);
+      switch (details.type()) {
+        case DATA_CONSTANT: {
+          *access_info = PropertyAccessInfo::DataConstant(
+              MapList{receiver_map},
+              handle(descriptors->GetValue(number), isolate()), holder);
+          return true;
+        }
+        case DATA: {
+          int index = descriptors->GetFieldIndex(number);
+          Representation field_representation = details.representation();
+          FieldIndex field_index = FieldIndex::ForPropertyIndex(
+              *map, index, field_representation.IsDouble());
+          Type* field_type = Type::Tagged();
+          if (field_representation.IsSmi()) {
+            field_type = type_cache_.kSmi;
+          } else if (field_representation.IsDouble()) {
+            field_type = type_cache_.kFloat64;
+          } else if (field_representation.IsHeapObject()) {
+            // Extract the field type from the property details (make sure its
+            // representation is TaggedPointer to reflect the heap object case).
+            field_type = Type::Intersect(
+                descriptors->GetFieldType(number)->Convert(zone()),
+                Type::TaggedPointer(), zone());
+            if (field_type->Is(Type::None())) {
+              // Store is not safe if the field type was cleared.
+              if (access_mode == AccessMode::kStore) return false;
+
+              // The field type was cleared by the GC, so we don't know anything
+              // about the contents now.
+              // TODO(bmeurer): It would be awesome to make this saner in the
+              // runtime/GC interaction.
+              field_type = Type::TaggedPointer();
+            } else if (!Type::Any()->Is(field_type)) {
+              // Add proper code dependencies in case of stable field map(s).
+              Handle<Map> field_owner_map(map->FindFieldOwner(number),
+                                          isolate());
+              dependencies()->AssumeFieldType(field_owner_map);
+            }
+            DCHECK(field_type->Is(Type::TaggedPointer()));
           }
-          DCHECK(field_type->Is(Type::TaggedPointer()));
+          *access_info = PropertyAccessInfo::DataField(
+              MapList{receiver_map}, field_index, field_type, holder);
+          return true;
+        }
+        case ACCESSOR_CONSTANT: {
+          Handle<Object> accessors(descriptors->GetValue(number), isolate());
+          if (!accessors->IsAccessorPair()) return false;
+          Handle<Object> accessor(
+              access_mode == AccessMode::kLoad
+                  ? Handle<AccessorPair>::cast(accessors)->getter()
+                  : Handle<AccessorPair>::cast(accessors)->setter(),
+              isolate());
+          if (!accessor->IsJSFunction()) {
+            // TODO(turbofan): Add support for API accessors.
+            return false;
+          }
+          *access_info = PropertyAccessInfo::AccessorConstant(
+              MapList{receiver_map}, accessor, holder);
+          return true;
+        }
+        case ACCESSOR: {
+          // TODO(turbofan): Add support for general accessors?
+          return false;
         }
-        *access_info = PropertyAccessInfo::DataField(
-            MapList{receiver_map}, field_index, field_type, holder);
-        return true;
-      } else {
-        // TODO(bmeurer): Add support for accessors.
-        return false;
       }
+      UNREACHABLE();
+      return false;
     }
 
     // Don't search on the prototype chain for special indices in case of

src/compiler/access-info.h

@@ -55,7 +55,13 @@ class ElementAccessInfo final {
 // object property, either on the object itself or on the prototype chain.
 class PropertyAccessInfo final {
  public:
-  enum Kind { kInvalid, kNotFound, kDataConstant, kDataField };
+  enum Kind {
+    kInvalid,
+    kNotFound,
+    kDataConstant,
+    kDataField,
+    kAccessorConstant
+  };
 
   static PropertyAccessInfo NotFound(MapList const& receiver_maps,
                                      MaybeHandle<JSObject> holder);
@@ -66,6 +72,9 @@ class PropertyAccessInfo final {
       MapList const& receiver_maps, FieldIndex field_index, Type* field_type,
       MaybeHandle<JSObject> holder = MaybeHandle<JSObject>(),
       MaybeHandle<Map> transition_map = MaybeHandle<Map>());
+  static PropertyAccessInfo AccessorConstant(MapList const& receiver_maps,
+                                             Handle<Object> constant,
+                                             MaybeHandle<JSObject> holder);
 
   PropertyAccessInfo();
 
@@ -74,6 +83,7 @@ class PropertyAccessInfo final {
   bool IsNotFound() const { return kind() == kNotFound; }
   bool IsDataConstant() const { return kind() == kDataConstant; }
   bool IsDataField() const { return kind() == kDataField; }
+  bool IsAccessorConstant() const { return kind() == kAccessorConstant; }
 
   bool HasTransitionMap() const { return !transition_map().is_null(); }
 
@@ -88,8 +98,8 @@ class PropertyAccessInfo final {
  private:
   PropertyAccessInfo(MaybeHandle<JSObject> holder,
                      MapList const& receiver_maps);
-  PropertyAccessInfo(MaybeHandle<JSObject> holder, Handle<Object> constant,
-                     MapList const& receiver_maps);
+  PropertyAccessInfo(Kind kind, MaybeHandle<JSObject> holder,
+                     Handle<Object> constant, MapList const& receiver_maps);
   PropertyAccessInfo(MaybeHandle<JSObject> holder,
                      MaybeHandle<Map> transition_map, FieldIndex field_index,
                      Type* field_type, MapList const& receiver_maps);

src/compiler/code-generator.cc

@@ -778,6 +778,12 @@ void CodeGenerator::BuildTranslationForFrameStateDescriptor(
           shared_info_id,
           static_cast<unsigned int>(descriptor->parameters_count()));
       break;
+    case FrameStateType::kGetterStub:
+      translation->BeginGetterStubFrame(shared_info_id);
+      break;
+    case FrameStateType::kSetterStub:
+      translation->BeginSetterStubFrame(shared_info_id);
+      break;
   }
 
   TranslateFrameStateDescriptorOperands(descriptor, iter, state_combine,

src/compiler/frame-states.cc

@@ -64,6 +64,12 @@ std::ostream& operator<<(std::ostream& os, FrameStateType type) {
     case FrameStateType::kConstructStub:
       os << "CONSTRUCT_STUB";
       break;
+    case FrameStateType::kGetterStub:
+      os << "GETTER_STUB";
+      break;
+    case FrameStateType::kSetterStub:
+      os << "SETTER_STUB";
+      break;
   }
   return os;
 }

src/compiler/frame-states.h

@@ -80,7 +80,9 @@ enum class FrameStateType {
   kInterpretedFunction,  // Represents an InterpretedFrame.
   kArgumentsAdaptor,     // Represents an ArgumentsAdaptorFrame.
   kTailCallerFunction,   // Represents a frame removed by tail call elimination.
-  kConstructStub         // Represents a ConstructStubFrame.
+  kConstructStub,        // Represents a ConstructStubFrame.
+  kGetterStub,           // Represents a GetterStubFrame.
+  kSetterStub            // Represents a SetterStubFrame.
 };
 
 class FrameStateFunctionInfo {

src/compiler/js-native-context-specialization.cc

@@ -110,6 +110,9 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess(
          node->opcode() == IrOpcode::kJSLoadProperty ||
          node->opcode() == IrOpcode::kJSStoreProperty);
   Node* receiver = NodeProperties::GetValueInput(node, 0);
+  Node* context = NodeProperties::GetContextInput(node);
+  Node* frame_state_eager = NodeProperties::FindFrameStateBefore(node);
+  Node* frame_state_lazy = NodeProperties::GetFrameStateInput(node, 0);
   Node* effect = NodeProperties::GetEffectInput(node);
   Node* control = NodeProperties::GetControlInput(node);
 
@@ -129,6 +132,14 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess(
     return NoChange();
   }
 
+  // TODO(turbofan): Add support for inlining into try blocks.
+  if (NodeProperties::IsExceptionalCall(node) ||
+      !(flags() & kAccessorInliningEnabled)) {
+    for (auto access_info : access_infos) {
+      if (access_info.IsAccessorConstant()) return NoChange();
+    }
+  }
+
   // Nothing to do if we have no non-deprecated maps.
   if (access_infos.empty()) {
     return ReduceSoftDeoptimize(
@@ -162,9 +173,9 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess(
     }
 
     // Generate the actual property access.
-    ValueEffectControl continuation =
-        BuildPropertyAccess(receiver, value, effect, control, name,
-                            native_context, access_info, access_mode);
+    ValueEffectControl continuation = BuildPropertyAccess(
+        receiver, value, context, frame_state_lazy, effect, control, name,
+        native_context, access_info, access_mode);
     value = continuation.value();
     effect = continuation.effect();
     control = continuation.control();
@@ -250,26 +261,32 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess(
           receiverissmi_effect = receiverissmi_control = nullptr;
         }
 
-        // Create dominating Merge+EffectPhi for this {receiver} type.
+        // Create single chokepoint for the control.
         int const this_control_count = static_cast<int>(this_controls.size());
-        this_control =
-            (this_control_count == 1)
-                ? this_controls.front()
-                : graph()->NewNode(common()->Merge(this_control_count),
-                                   this_control_count, &this_controls.front());
-        this_effects.push_back(this_control);
-        int const this_effect_count = static_cast<int>(this_effects.size());
-        this_effect =
-            (this_control_count == 1)
-                ? this_effects.front()
-                : graph()->NewNode(common()->EffectPhi(this_control_count),
-                                   this_effect_count, &this_effects.front());
+        if (this_control_count == 1) {
+          this_control = this_controls.front();
+          this_effect = this_effects.front();
+        } else {
+          this_control =
+              graph()->NewNode(common()->Merge(this_control_count),
+                               this_control_count, &this_controls.front());
+          this_effects.push_back(this_control);
+          this_effect =
+              graph()->NewNode(common()->EffectPhi(this_control_count),
+                               this_control_count + 1, &this_effects.front());
+
+          // TODO(turbofan): The effect/control linearization will not find a
+          // FrameState after the EffectPhi that is generated above.
+          this_effect =
+              graph()->NewNode(common()->Checkpoint(), frame_state_eager,
+                               this_effect, this_control);
+        }
       }
 
       // Generate the actual property access.
       ValueEffectControl continuation = BuildPropertyAccess(
-          this_receiver, this_value, this_effect, this_control, name,
-          native_context, access_info, access_mode);
+          this_receiver, this_value, context, frame_state_lazy, this_effect,
+          this_control, name, native_context, access_info, access_mode);
       values.push_back(continuation.value());
       effects.push_back(continuation.effect());
       controls.push_back(continuation.control());
@@ -555,10 +572,7 @@ Reduction JSNativeContextSpecialization::ReduceElementAccess(
                                this_control_count + 1, &this_effects.front());
 
           // TODO(turbofan): The effect/control linearization will not find a
-          // FrameState after the StoreField or Call that is generated for the
-          // elements kind transition above. This is because those operators
-          // don't have the kNoWrite flag on it, even though they are not
-          // observable by JavaScript.
+          // FrameState after the EffectPhi that is generated above.
           this_effect = graph()->NewNode(common()->Checkpoint(), frame_state,
                                          this_effect, this_control);
         }
@@ -734,9 +748,9 @@ Reduction JSNativeContextSpecialization::ReduceJSStoreProperty(Node* node) {
 
 JSNativeContextSpecialization::ValueEffectControl
 JSNativeContextSpecialization::BuildPropertyAccess(
-    Node* receiver, Node* value, Node* effect, Node* control, Handle<Name> name,
-    Handle<Context> native_context, PropertyAccessInfo const& access_info,
-    AccessMode access_mode) {
+    Node* receiver, Node* value, Node* context, Node* frame_state, Node* effect,
+    Node* control, Handle<Name> name, Handle<Context> native_context,
+    PropertyAccessInfo const& access_info, AccessMode access_mode) {
   // Determine actual holder and perform prototype chain checks.
   Handle<JSObject> holder;
   if (access_info.holder().ToHandle(&holder)) {
@@ -755,6 +769,58 @@ JSNativeContextSpecialization::BuildPropertyAccess(
       effect =
           graph()->NewNode(simplified()->CheckIf(), check, effect, control);
     }
+  } else if (access_info.IsAccessorConstant()) {
+    // TODO(bmeurer): Properly rewire the IfException edge here if there's any.
+    Node* target = jsgraph()->Constant(access_info.constant());
+    FrameStateInfo const& frame_info = OpParameter<FrameStateInfo>(frame_state);
+    Handle<SharedFunctionInfo> shared_info =
+        frame_info.shared_info().ToHandleChecked();
+    switch (access_mode) {
+      case AccessMode::kLoad: {
+        // We need a FrameState for the getter stub to restore the correct
+        // context before returning to fullcodegen.
+        FrameStateFunctionInfo const* frame_info0 =
+            common()->CreateFrameStateFunctionInfo(FrameStateType::kGetterStub,
+                                                   1, 0, shared_info);
+        Node* frame_state0 = graph()->NewNode(
+            common()->FrameState(BailoutId::None(),
+                                 OutputFrameStateCombine::Ignore(),
+                                 frame_info0),
+            graph()->NewNode(common()->StateValues(1), receiver),
+            jsgraph()->EmptyStateValues(), jsgraph()->EmptyStateValues(),
+            context, target, frame_state);
+
+        // Introduce the call to the getter function.
+        value = effect = graph()->NewNode(
+            javascript()->CallFunction(
+                2, VectorSlotPair(), ConvertReceiverMode::kNotNullOrUndefined),
+            target, receiver, context, frame_state0, effect, control);
+        control = graph()->NewNode(common()->IfSuccess(), value);
+        break;
+      }
+      case AccessMode::kStore: {
+        // We need a FrameState for the setter stub to restore the correct
+        // context and return the appropriate value to fullcodegen.
+        FrameStateFunctionInfo const* frame_info0 =
+            common()->CreateFrameStateFunctionInfo(FrameStateType::kSetterStub,
+                                                   2, 0, shared_info);
+        Node* frame_state0 = graph()->NewNode(
+            common()->FrameState(BailoutId::None(),
+                                 OutputFrameStateCombine::Ignore(),
+                                 frame_info0),
+            graph()->NewNode(common()->StateValues(2), receiver, value),
+            jsgraph()->EmptyStateValues(), jsgraph()->EmptyStateValues(),
+            context, target, frame_state);
+
+        // Introduce the call to the setter function.
+        effect = graph()->NewNode(
+            javascript()->CallFunction(
+                3, VectorSlotPair(), ConvertReceiverMode::kNotNullOrUndefined),
+            target, receiver, value, context, frame_state0, effect, control);
+        control = graph()->NewNode(common()->IfSuccess(), effect);
+        break;
+      }
+    }
   } else {
     DCHECK(access_info.IsDataField());
     FieldIndex const field_index = access_info.field_index();

src/compiler/js-native-context-specialization.h

@@ -41,8 +41,9 @@ class JSNativeContextSpecialization final : public AdvancedReducer {
   // Flags that control the mode of operation.
   enum Flag {
     kNoFlags = 0u,
-    kBailoutOnUninitialized = 1u << 0,
-    kDeoptimizationEnabled = 1u << 1,
+    kAccessorInliningEnabled = 1u << 0,
+    kBailoutOnUninitialized = 1u << 1,
+    kDeoptimizationEnabled = 1u << 2,
   };
   typedef base::Flags<Flag> Flags;
 
@@ -100,6 +101,7 @@ class JSNativeContextSpecialization final : public AdvancedReducer {
 
   // Construct the appropriate subgraph for property access.
   ValueEffectControl BuildPropertyAccess(Node* receiver, Node* value,
+                                         Node* context, Node* frame_state,
                                          Node* effect, Node* control,
                                          Handle<Name> name,
                                          Handle<Context> native_context,

src/compiler/pipeline.cc

@@ -606,6 +606,9 @@ PipelineCompilationJob::Status PipelineCompilationJob::CreateGraphImpl() {
     info()->MarkAsDeoptimizationEnabled();
   }
   if (!info()->is_optimizing_from_bytecode()) {
+    if (FLAG_inline_accessors) {
+      info()->MarkAsAccessorInliningEnabled();
+    }
     if (info()->is_deoptimization_enabled() && FLAG_turbo_type_feedback) {
       info()->MarkAsTypeFeedbackEnabled();
     }
@@ -796,6 +799,9 @@ struct InliningPhase {
         data->info()->dependencies());
     JSNativeContextSpecialization::Flags flags =
         JSNativeContextSpecialization::kNoFlags;
+    if (data->info()->is_accessor_inlining_enabled()) {
+      flags |= JSNativeContextSpecialization::kAccessorInliningEnabled;
+    }
     if (data->info()->is_bailout_on_uninitialized()) {
       flags |= JSNativeContextSpecialization::kBailoutOnUninitialized;
     }