Make all load-named-fields depend on their map-check, unless explicitly ignored.

BUG=
R=titzer@chromium.org

Review URL: https://chromiumcodereview.appspot.com/22555004

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@16139 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/code-stubs-hydrogen.cc

@@ -352,7 +352,7 @@ HValue* CodeStubGraphBuilder<FastCloneShallowArrayStub>::BuildCodeStub() {
   HObjectAccess access = HObjectAccess::ForAllocationSiteTransitionInfo();
   HInstruction* boilerplate = Add<HLoadNamedField>(allocation_site, access);
   if (mode == FastCloneShallowArrayStub::CLONE_ANY_ELEMENTS) {
-    HValue* elements = AddLoadElements(boilerplate);
+    HValue* elements = AddLoadElements(boilerplate, NULL);
 
     IfBuilder if_fixed_cow(this);
     if_fixed_cow.If<HCompareMap>(elements, factory->fixed_cow_array_map());
@@ -513,7 +513,7 @@ HValue* CodeStubGraphBuilder<LoadFieldStub>::BuildCodeStub() {
   HObjectAccess access = casted_stub()->is_inobject() ?
       HObjectAccess::ForJSObjectOffset(casted_stub()->offset(), rep) :
       HObjectAccess::ForBackingStoreOffset(casted_stub()->offset(), rep);
-  return AddInstruction(BuildLoadNamedField(GetParameter(0), access));
+  return AddInstruction(BuildLoadNamedField(GetParameter(0), access, NULL));
 }
 
 
@@ -528,7 +528,7 @@ HValue* CodeStubGraphBuilder<KeyedLoadFieldStub>::BuildCodeStub() {
   HObjectAccess access = casted_stub()->is_inobject() ?
       HObjectAccess::ForJSObjectOffset(casted_stub()->offset(), rep) :
       HObjectAccess::ForBackingStoreOffset(casted_stub()->offset(), rep);
-  return AddInstruction(BuildLoadNamedField(GetParameter(0), access));
+  return AddInstruction(BuildLoadNamedField(GetParameter(0), access, NULL));
 }
 
 

src/hydrogen-gvn.h

@@ -48,7 +48,7 @@ class HGlobalValueNumberingPhase : public HPhase {
     // instructions during the first pass.
     if (FLAG_smi_only_arrays && removed_side_effects_) {
       Analyze();
-      ASSERT(!removed_side_effects_);
+      // TODO(danno): Turn this into a fixpoint iteration.
     }
   }
 

src/hydrogen.cc

@@ -1191,7 +1191,7 @@ void HGraphBuilder::BuildTransitionElementsKind(HValue* object,
   }
 
   if (!IsSimpleMapChangeTransition(from_kind, to_kind)) {
-    HInstruction* elements = AddLoadElements(object);
+    HInstruction* elements = AddLoadElements(object, NULL);
 
     HInstruction* empty_fixed_array = Add<HConstant>(
         isolate()->factory()->empty_fixed_array());
@@ -1700,7 +1700,7 @@ HValue* HGraphBuilder::BuildCloneShallowArray(HValue* boilerplate,
   if (length > 0) {
     // Get hold of the elements array of the boilerplate and setup the
     // elements pointer in the resulting object.
-    HValue* boilerplate_elements = AddLoadElements(boilerplate);
+    HValue* boilerplate_elements = AddLoadElements(boilerplate, NULL);
     HValue* object_elements = Add<HInnerAllocatedObject>(object, elems_offset);
     Add<HStoreNamedField>(object, HObjectAccess::ForElementsPointer(),
                           object_elements);
@@ -1833,7 +1833,7 @@ HValue* HGraphBuilder::JSArrayBuilder::EmitMapCode() {
     // map, because we can just load the map in that case.
     HObjectAccess access = HObjectAccess::ForPrototypeOrInitialMap();
     return builder()->AddInstruction(
-        builder()->BuildLoadNamedField(constructor_function_, access));
+        builder()->BuildLoadNamedField(constructor_function_, access, NULL));
   }
 
   HInstruction* native_context = builder()->BuildGetNativeContext();
@@ -1854,7 +1854,7 @@ HValue* HGraphBuilder::JSArrayBuilder::EmitInternalMapCode() {
   // Find the map near the constructor function
   HObjectAccess access = HObjectAccess::ForPrototypeOrInitialMap();
   return builder()->AddInstruction(
-      builder()->BuildLoadNamedField(constructor_function_, access));
+      builder()->BuildLoadNamedField(constructor_function_, access, NULL));
 }
 
 
@@ -4298,6 +4298,7 @@ void HOptimizedGraphBuilder::VisitArrayLiteral(ArrayLiteral* expr) {
   int data_size = 0;
   int pointer_size = 0;
   int max_properties = kMaxFastLiteralProperties;
+  HCheckMaps* type_check = NULL;
   if (IsFastLiteral(original_boilerplate_object,
                     kMaxFastLiteralDepth,
                     &max_properties,
@@ -4335,7 +4336,7 @@ void HOptimizedGraphBuilder::VisitArrayLiteral(ArrayLiteral* expr) {
     // De-opt if elements kind changed from boilerplate_elements_kind.
     Handle<Map> map = Handle<Map>(original_boilerplate_object->map(),
                                   isolate());
-    Add<HCheckMaps>(literal, map, top_info());
+    type_check = Add<HCheckMaps>(literal, map, top_info());
   }
 
   // The array is expected in the bailout environment during computation
@@ -4356,7 +4357,7 @@ void HOptimizedGraphBuilder::VisitArrayLiteral(ArrayLiteral* expr) {
     HValue* value = Pop();
     if (!Smi::IsValid(i)) return Bailout(kNonSmiKeyInArrayLiteral);
 
-    elements = AddLoadElements(literal);
+    elements = AddLoadElements(literal, type_check);
 
     HValue* key = Add<HConstant>(i);
 
@@ -4410,9 +4411,10 @@ static bool ComputeLoadStoreField(Handle<Map> type,
 }
 
 
-void HOptimizedGraphBuilder::AddCheckMap(HValue* object, Handle<Map> map) {
+HCheckMaps* HOptimizedGraphBuilder::AddCheckMap(HValue* object,
+                                                Handle<Map> map) {
   BuildCheckHeapObject(object);
-  Add<HCheckMaps>(object, map, top_info());
+  return Add<HCheckMaps>(object, map, top_info());
 }
 
 
@@ -4578,8 +4580,8 @@ HInstruction* HOptimizedGraphBuilder::TryLoadPolymorphicAsMonomorphic(
   if (count == types->length()) {
     // Everything matched; can use monomorphic load.
     BuildCheckHeapObject(object);
-    Add<HCheckMaps>(object, types);
-    return BuildLoadNamedField(object, access);
+    HCheckMaps* type_check = Add<HCheckMaps>(object, types);
+    return BuildLoadNamedField(object, access, type_check);
   }
 
   if (count != 0) return NULL;
@@ -4600,14 +4602,14 @@ HInstruction* HOptimizedGraphBuilder::TryLoadPolymorphicAsMonomorphic(
   if (!lookup.IsField()) return NULL;
 
   BuildCheckHeapObject(object);
-  Add<HCheckMaps>(object, types);
+  HCheckMaps* type_check = Add<HCheckMaps>(object, types);
 
   Handle<JSObject> holder(lookup.holder());
   Handle<Map> holder_map(holder->map());
   BuildCheckPrototypeMaps(Handle<JSObject>::cast(prototype), holder);
   HValue* holder_value = Add<HConstant>(holder);
   return BuildLoadNamedField(holder_value,
-      HObjectAccess::ForField(holder_map, &lookup, name));
+      HObjectAccess::ForField(holder_map, &lookup, name), type_check);
 }
 
 
@@ -4682,7 +4684,7 @@ void HOptimizedGraphBuilder::HandlePolymorphicLoadNamedField(
       // TODO(verwaest): Merge logic with BuildLoadNamedMonomorphic.
       if (lookup.IsField()) {
         HObjectAccess access = HObjectAccess::ForField(map, &lookup, name);
-        HLoadNamedField* load = BuildLoadNamedField(object, access);
+        HLoadNamedField* load = BuildLoadNamedField(object, access, compare);
         load->set_position(expr->position());
         AddInstruction(load);
         if (!ast_context()->IsEffect()) Push(load);
@@ -5419,18 +5421,18 @@ HInstruction* HOptimizedGraphBuilder::BuildLoadNamedMonomorphic(
   // Handle access to various length properties
   if (name->Equals(isolate()->heap()->length_string())) {
     if (map->instance_type() == JS_ARRAY_TYPE) {
-      AddCheckMap(object, map);
+      HCheckMaps* type_check = AddCheckMap(object, map);
       return New<HLoadNamedField>(object,
-          HObjectAccess::ForArrayLength(map->elements_kind()));
+          HObjectAccess::ForArrayLength(map->elements_kind()), type_check);
     }
   }
 
   LookupResult lookup(isolate());
   map->LookupDescriptor(NULL, *name, &lookup);
   if (lookup.IsField()) {
-    AddCheckMap(object, map);
+    HCheckMaps* type_check = AddCheckMap(object, map);
     return BuildLoadNamedField(object,
-        HObjectAccess::ForField(map, &lookup, name));
+        HObjectAccess::ForField(map, &lookup, name), type_check);
   }
 
   // Handle a load of a constant known function.
@@ -5446,11 +5448,11 @@ HInstruction* HOptimizedGraphBuilder::BuildLoadNamedMonomorphic(
     Handle<JSObject> prototype(JSObject::cast(map->prototype()));
     Handle<JSObject> holder(lookup.holder());
     Handle<Map> holder_map(holder->map());
-    AddCheckMap(object, map);
+    HCheckMaps* type_check = AddCheckMap(object, map);
     BuildCheckPrototypeMaps(prototype, holder);
     HValue* holder_value = Add<HConstant>(holder);
     return BuildLoadNamedField(holder_value,
-        HObjectAccess::ForField(holder_map, &lookup, name));
+        HObjectAccess::ForField(holder_map, &lookup, name), type_check);
   }
 
   // Handle a load of a constant function somewhere in the prototype chain.

src/hydrogen.h

@@ -1254,10 +1254,10 @@ class HGraphBuilder {
   HLoadNamedField* BuildLoadNamedField(
       HValue* object,
       HObjectAccess access,
-      HValue* typecheck = NULL);
-  HInstruction* BuildLoadStringLength(HValue* object, HValue* typecheck = NULL);
+      HValue* typecheck);
+  HInstruction* BuildLoadStringLength(HValue* object, HValue* typecheck);
   HStoreNamedField* AddStoreMapConstant(HValue *object, Handle<Map>);
-  HLoadNamedField* AddLoadElements(HValue *object, HValue *typecheck = NULL);
+  HLoadNamedField* AddLoadElements(HValue *object, HValue *typecheck);
   HLoadNamedField* AddLoadFixedArrayLength(HValue *object);
 
   HValue* AddLoadJSBuiltin(Builtins::JavaScript builtin);
@@ -2036,7 +2036,7 @@ class HOptimizedGraphBuilder: public HGraphBuilder, public AstVisitor {
                                           Property* expr,
                                           Handle<Map> map);
 
-  void AddCheckMap(HValue* object, Handle<Map> map);
+  HCheckMaps* AddCheckMap(HValue* object, Handle<Map> map);
 
   void BuildStoreNamed(Expression* expression,
                        BailoutId id,

test/mjsunit/regress/regress-hoist-load-named-field.js

+// Copyright 2013 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --allow-natives-syntax
+
+// Load fields should not be hoisted beyond their check maps when the check maps
+// cannot be hoisted due to changes in elements kinds.
+function f(o, a) {
+  var v;
+  var i = 1;
+  while (i < 2) {
+    v = o.y;
+    a[0] = 1.5;
+    i++;
+  }
+  return v;
+}
+
+f({y:1.4}, [1]);
+f({y:1.6}, [1]);
+%OptimizeFunctionOnNextCall(f);
+f({x:1}, [1]);
+
+// Polymorphic loads should not be hoisted beyond their compare maps.
+function f2(o) {
+  var i = 0;
+  var v;
+  while (i < 1) {
+    v = o.x;
+    i++;
+  }
+  return v;
+}
+
+var o1 = { x: 1.5 };
+var o2 = { y: 1, x: 1 };
+
+f2(o1);
+f2(o1);
+f2(o2);
+%OptimizeFunctionOnNextCall(f2);
+f2(o2);