[v8-dev] Simplified HCheckMaps handling a bit. (issue 11343011)

svenpanne Mon, 29 Oct 2012 08:20:24 -0700

Reviewers: Toon Verwaest,

Description:
Simplified HCheckMaps handling a bit.

This is a refactoring-only CL which simplifies the way we emit combinationsof

Smi+map checks.


Please review this at http://codereview.chromium.org/11343011/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files:
  M src/hydrogen.h
  M src/hydrogen.cc


Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc

index374e54c97389e448721421ca80898aeb6d42df1d..b705a24686f84dd1d447fab459098616a0124b05100644

--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc

@@ -5281,18 +5281,19 @@ static int ComputeLoadStoreFieldIndex(Handle<Map>type,

 }


+void HGraphBuilder::AddCheckMapsWithTransitions(HValue* object,
+                                                Handle<Map> map) {
+  AddInstruction(new(zone()) HCheckNonSmi(object));
+  AddInstruction(HCheckMaps::NewWithTransitions(object, map, zone()));
+}
+
+
 HInstruction* HGraphBuilder::BuildStoreNamedField(HValue* object,
                                                   Handle<String> name,
                                                   HValue* value,
                                                   Handle<Map> map,
-                                                  LookupResult* lookup,
-                                                  bool smi_and_map_check) {
+                                                  LookupResult* lookup) {
   ASSERT(lookup->IsFound());
-  if (smi_and_map_check) {
-    AddInstruction(new(zone()) HCheckNonSmi(object));
-    AddInstruction(HCheckMaps::NewWithTransitions(object, map, zone()));
-  }
-

// If the property does not exist yet, we have to check that it wasn'tmade// readonly or turned into a setter by some meanwhile modifications onthe

   // prototype chain.

@@ -5361,7 +5362,7 @@ HInstruction* HGraphBuilder::BuildCallSetter(HValue*object,

                                              Handle<Map> map,
                                              Handle<JSFunction> setter,
                                              Handle<JSObject> holder) {
-  AddCheckConstantFunction(holder, object, map, true);
+  AddCheckConstantFunction(holder, object, map);
   AddInstruction(new(zone()) HPushArgument(object));
   AddInstruction(new(zone()) HPushArgument(value));
   return new(zone()) HCallConstantFunction(setter, 2);

@@ -5375,8 +5376,8 @@ HInstruction*HGraphBuilder::BuildStoreNamedMonomorphic(HValue* object,

   // Handle a store to a known field.
   LookupResult lookup(isolate());
   if (ComputeLoadStoreField(map, name, &lookup, true)) {
-    // true = needs smi and map check.
-    return BuildStoreNamedField(object, name, value, map, &lookup, true);
+    AddCheckMapsWithTransitions(object, map);
+    return BuildStoreNamedField(object, name, value, map, &lookup);
   }

   // No luck, do a generic store.

@@ -5424,7 +5425,7 @@ voidHGraphBuilder::HandlePolymorphicLoadNamedField(Property* expr,

   HInstruction* instr;
   if (count == types->length() && is_monomorphic_field) {
     AddInstruction(new(zone()) HCheckMaps(object, types, zone()));
-    instr = BuildLoadNamedField(object, map, &lookup, false);
+    instr = BuildLoadNamedField(object, map, &lookup);
   } else {
     HValue* context = environment()->LookupContext();
     instr = new(zone()) HLoadNamedFieldPolymorphic(context,

@@ -5467,7 +5468,7 @@ voidHGraphBuilder::HandlePolymorphicStoreNamedField(Assignment* expr,

       set_current_block(if_true);
       HInstruction* instr;
       CHECK_ALIVE(instr =
-          BuildStoreNamedField(object, name, value, map, &lookup, false));
+          BuildStoreNamedField(object, name, value, map, &lookup));
       instr->set_position(expr->position());
       // Goto will add the HSimulate for the store.
       AddInstruction(instr);

@@ -5543,7 +5544,7 @@ voidHGraphBuilder::HandlePropertyAssignment(Assignment* expr) {

       Handle<JSFunction> setter;
       Handle<JSObject> holder;
       if (LookupSetter(map, name, &setter, &holder)) {
-        AddCheckConstantFunction(holder, object, map, true);
+        AddCheckConstantFunction(holder, object, map);

if (FLAG_inline_accessors && TryInlineSetter(setter, expr, value)){

           return;
         }
@@ -5967,13 +5968,7 @@ void HGraphBuilder::VisitThrow(Throw* expr) {

 HLoadNamedField* HGraphBuilder::BuildLoadNamedField(HValue* object,
                                                     Handle<Map> map,
-                                                    LookupResult* lookup,

- boolsmi_and_map_check) {

-  if (smi_and_map_check) {
-    AddInstruction(new(zone()) HCheckNonSmi(object));
-    AddInstruction(HCheckMaps::NewWithTransitions(object, map, zone()));
-  }
-
+                                                    LookupResult* lookup) {
   int index = lookup->GetLocalFieldIndexFromMap(*map);
   if (index < 0) {
     // Negative property indices are in-object properties, indexed

@@ -6004,7 +5999,7 @@ HInstruction* HGraphBuilder::BuildCallGetter(HValue*object,

                                              Handle<Map> map,
                                              Handle<JSFunction> getter,
                                              Handle<JSObject> holder) {
-  AddCheckConstantFunction(holder, object, map, true);
+  AddCheckConstantFunction(holder, object, map);
   AddInstruction(new(zone()) HPushArgument(object));
   return new(zone()) HCallConstantFunction(getter, 1);
 }

@@ -6019,13 +6014,13 @@ HInstruction*HGraphBuilder::BuildLoadNamedMonomorphic(HValue* object,

   LookupResult lookup(isolate());
   map->LookupDescriptor(NULL, *name, &lookup);
   if (lookup.IsField()) {
-    return BuildLoadNamedField(object, map, &lookup, true);
+    AddCheckMapsWithTransitions(object, map);
+    return BuildLoadNamedField(object, map, &lookup);
   }

   // Handle a load of a constant known function.
   if (lookup.IsConstantFunction()) {
-    AddInstruction(new(zone()) HCheckNonSmi(object));
-    AddInstruction(HCheckMaps::NewWithTransitions(object, map, zone()));
+    AddCheckMapsWithTransitions(object, map);
     Handle<JSFunction> function(lookup.GetConstantFunctionFromMap(*map));
     return new(zone()) HConstant(function, Representation::Tagged());
   }
@@ -6669,7 +6664,7 @@ void HGraphBuilder::VisitProperty(Property* expr) {
       Handle<JSFunction> getter;
       Handle<JSObject> holder;
       if (LookupGetter(map, name, &getter, &holder)) {
-        AddCheckConstantFunction(holder, Top(), map, true);
+        AddCheckConstantFunction(holder, Top(), map);
         if (FLAG_inline_accessors && TryInlineGetter(getter, expr)) return;
         AddInstruction(new(zone()) HPushArgument(Pop()));
         instr = new(zone()) HCallConstantFunction(getter, 1);

@@ -6717,9 +6712,7 @@ voidHGraphBuilder::AddCheckConstantFunction(Handle<JSObject> holder,// are overwritten. Therefore it is enough to check the map of theholder and

   // its prototypes.
   if (smi_and_map_check) {
-    AddInstruction(new(zone()) HCheckNonSmi(receiver));
-    AddInstruction(HCheckMaps::NewWithTransitions(receiver, receiver_map,
-                                                  zone()));
+    AddCheckMapsWithTransitions(receiver, receiver_map);
   }
   if (!holder.is_null()) {
     AddInstruction(new(zone()) HCheckPrototypeMaps(

@@ -7361,7 +7354,7 @@ bool HGraphBuilder::TryInlineBuiltinMethodCall(Call*expr,

     case kMathCos:
     case kMathTan:
       if (argument_count == 2 && check_type == RECEIVER_MAP_CHECK) {