Remove now-incorrect DataView accessor optimization

In ES2015, the "byteLength" and "byteOffset" properties of DataViews are
getters on the prototype, so the previously-used strategy of special-casing
them using only the receiver map is invalid.

A future CL will need to use the same strategy which will be taken for
TypedArray "length", "byteLength", and "byteOffset": adding a prototype
chain check.

BUG=v8:5018, chromium:593634

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

src/accessors.cc

@@ -86,23 +86,6 @@ bool Accessors::IsJSObjectFieldAccessor(Handle<Map> map, Handle<Name> name,
 }
 
 
-bool Accessors::IsJSArrayBufferViewFieldAccessor(Handle<Map> map,
-                                                 Handle<Name> name,
-                                                 int* object_offset) {
-  DCHECK(name->IsUniqueName());
-  Isolate* isolate = name->GetIsolate();
-
-  switch (map->instance_type()) {
-    case JS_DATA_VIEW_TYPE:
-      return CheckForName(name, isolate->factory()->byte_length_string(),
-                          JSDataView::kByteLengthOffset, object_offset) ||
-             CheckForName(name, isolate->factory()->byte_offset_string(),
-                          JSDataView::kByteOffsetOffset, object_offset);
-    default:
-      return false;
-  }
-}
-
 namespace {
 
 MUST_USE_RESULT MaybeHandle<Object> ReplaceAccessorWithDataProperty(

src/accessors.h

@@ -95,14 +95,6 @@ class Accessors : public AllStatic {
   static bool IsJSObjectFieldAccessor(Handle<Map> map, Handle<Name> name,
                                       int* object_offset);
 
-  // Returns true for properties that are accessors to ArrayBufferView and
-  // derived classes fields. If true, *object_offset contains offset of
-  // object field. The caller still has to check whether the underlying
-  // buffer was neutered.
-  static bool IsJSArrayBufferViewFieldAccessor(Handle<Map> map,
-                                               Handle<Name> name,
-                                               int* object_offset);
-
   static Handle<AccessorInfo> MakeAccessor(
       Isolate* isolate,
       Handle<Name> name,

src/compiler/access-info.cc

@@ -404,26 +404,6 @@ bool AccessInfoFactory::LookupSpecialFieldAccessor(
                                                  field_index, field_type);
     return true;
   }
-  // Check for special JSArrayBufferView field accessors.
-  if (Accessors::IsJSArrayBufferViewFieldAccessor(map, name, &offset)) {
-    FieldIndex field_index = FieldIndex::ForInObjectOffset(offset);
-    Type* field_type = Type::Tagged();
-    if (Name::Equals(factory()->byte_length_string(), name) ||
-        Name::Equals(factory()->byte_offset_string(), name)) {
-      // The JSArrayBufferView::byte_length and JSArrayBufferView::byte_offset
-      // properties are always numbers in the range [0, kMaxSafeInteger].
-      field_type = type_cache_.kPositiveSafeInteger;
-    } else if (map->IsJSTypedArrayMap()) {
-      DCHECK(Name::Equals(factory()->length_string(), name));
-      // The JSTypedArray::length property is always a number in the range
-      // [0, kMaxSafeInteger].
-      field_type = type_cache_.kPositiveSafeInteger;
-    }
-    *access_info = PropertyAccessInfo::DataField(
-        Type::Class(map, zone()), field_index, field_type,
-        FieldCheck::kJSArrayBufferViewBufferNotNeutered);
-    return true;
-  }
   return false;
 }
 

src/crankshaft/hydrogen.cc

@@ -6559,7 +6559,6 @@ bool HOptimizedGraphBuilder::PropertyAccessInfo::IsIntegerIndexedExotic() {
 bool HOptimizedGraphBuilder::PropertyAccessInfo::CanAccessMonomorphic() {
   if (!CanInlinePropertyAccess(map_)) return false;
   if (IsJSObjectFieldAccessor()) return IsLoad();
-  if (IsJSArrayBufferViewFieldAccessor()) return IsLoad();
   if (map_->IsJSFunctionMap() && map_->is_constructor() &&
       !map_->has_non_instance_prototype() &&
       name_.is_identical_to(isolate()->factory()->prototype_string())) {
@@ -6607,17 +6606,6 @@ bool HOptimizedGraphBuilder::PropertyAccessInfo::CanAccessAsMonomorphic(
     }
     return true;
   }
-  if (GetJSArrayBufferViewFieldAccess(&access)) {
-    for (int i = 1; i < maps->length(); ++i) {
-      PropertyAccessInfo test_info(builder_, access_type_, maps->at(i), name_);
-      HObjectAccess test_access = HObjectAccess::ForMap();  // bogus default
-      if (!test_info.GetJSArrayBufferViewFieldAccess(&test_access)) {
-        return false;
-      }
-      if (!access.Equals(test_access)) return false;
-    }
-    return true;
-  }
 
   // Currently only handle numbers as a polymorphic case.
   // TODO(verwaest): Support monomorphic handling of numbers with a HCheckNumber
@@ -6671,12 +6659,6 @@ HValue* HOptimizedGraphBuilder::BuildMonomorphicAccess(
     return New<HLoadNamedField>(object, checked_object, access);
   }
 
-  if (info->GetJSArrayBufferViewFieldAccess(&access)) {
-    DCHECK(info->IsLoad());
-    checked_object = Add<HCheckArrayBufferNotNeutered>(checked_object);
-    return New<HLoadNamedField>(object, checked_object, access);
-  }
-
   if (info->name().is_identical_to(isolate()->factory()->prototype_string()) &&
       info->map()->IsJSFunctionMap() && info->map()->is_constructor()) {
     DCHECK(!info->map()->has_non_instance_prototype());

src/crankshaft/hydrogen.h

@@ -2622,20 +2622,6 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor {
       return false;
     }
 
-    bool IsJSArrayBufferViewFieldAccessor() {
-      int offset;  // unused
-      return Accessors::IsJSArrayBufferViewFieldAccessor(map_, name_, &offset);
-    }
-
-    bool GetJSArrayBufferViewFieldAccess(HObjectAccess* access) {
-      int offset;
-      if (Accessors::IsJSArrayBufferViewFieldAccessor(map_, name_, &offset)) {
-        *access = HObjectAccess::ForMapAndOffset(map_, offset);
-        return true;
-      }
-      return false;
-    }
-
     bool has_holder() { return !holder_.is_null(); }
     bool IsLoad() const { return access_type_ == LOAD; }
 

src/ic/ic.cc

@@ -1039,13 +1039,6 @@ Handle<Code> LoadIC::GetMapIndependentHandler(LookupIterator* lookup) {
         FieldIndex index = FieldIndex::ForInObjectOffset(object_offset, *map);
         return SimpleFieldLoad(index);
       }
-      if (Accessors::IsJSArrayBufferViewFieldAccessor(map, lookup->name(),
-                                                      &object_offset)) {
-        TRACE_HANDLER_STATS(isolate(), LoadIC_ArrayBufferViewLoadFieldStub);
-        FieldIndex index = FieldIndex::ForInObjectOffset(object_offset, *map);
-        ArrayBufferViewLoadFieldStub stub(isolate(), index);
-        return stub.GetCode();
-      }
 
       if (IsCompatibleReceiver(lookup, map)) {
         Handle<Object> accessors = lookup->GetAccessors();
@@ -1181,8 +1174,6 @@ Handle<Code> LoadIC::CompileHandler(LookupIterator* lookup,
       int object_offset;
       DCHECK(!Accessors::IsJSObjectFieldAccessor(map, lookup->name(),
                                                  &object_offset));
-      DCHECK(!Accessors::IsJSArrayBufferViewFieldAccessor(map, lookup->name(),
-                                                          &object_offset));
 #endif
 
       DCHECK(IsCompatibleReceiver(lookup, map));

test/mjsunit/regress/regress-5018.js

+// Copyright 2016 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.
+
+var dv = new DataView(new ArrayBuffer(4), 2);
+
+function getByteLength(a) {
+  return a.byteLength;
+}
+
+assertEquals(2, getByteLength(dv));
+assertEquals(2, getByteLength(dv));
+
+Object.defineProperty(dv.__proto__, 'byteLength', {value: 42});
+
+assertEquals(42, dv.byteLength);
+assertEquals(42, getByteLength(dv));
+
+function getByteOffset(a) {
+  return a.byteOffset;
+}
+
+assertEquals(2, getByteOffset(dv));
+assertEquals(2, getByteOffset(dv));
+
+Object.defineProperty(dv.__proto__, 'byteOffset', {value: 42});
+
+assertEquals(42, dv.byteOffset);
+assertEquals(42, getByteOffset(dv));