Request for comments on overall approach.

runtime/vm/opt_code_generator_ia32.cc

 // Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/globals.h"  // Needed here to get TARGET_ARCH_IA32.
 #if defined(TARGET_ARCH_IA32)
 
 #include "vm/opt_code_generator.h"
 
 #include "vm/assembler_macros.h"
@@ skipping 965 matching lines @@
     VisitLoadTwo(node->left(), node->right(), EAX, EDX);
     __ testl(EDX, Immediate(kSmiTagMask));
     __ j(NOT_ZERO, &slow_case);  // Call operator if right is not Smi.
     __ cmpl(EDX, Immediate(0));
     __ j(LESS, &slow_case);  // Result will not be Smi.
 
     // Test left.
     __ testl(EAX, Immediate(kSmiTagMask));
     __ j(ZERO, &is_smi);
 
-    __ movl(EBX, FieldAddress(EAX, Object::class_offset()));
-    __ CompareObject(EBX, Class::ZoneHandle(object_store->mint_class()));
+    __ CompareClassOf(EAX, Class::Handle(object_store->mint_class()), EBX);
     __ j(NOT_EQUAL, deopt_blob->label());
 
     // Load lower Mint word, convert to Smi. It is OK to loose bits.
     __ movl(EAX, FieldAddress(EAX, Mint::value_offset()));
     __ SmiTag(EAX);
 
     __ Bind(&is_smi);
     __ andl(EAX, EDX);
     __ jmp(&done);
     __ Bind(&slow_case);
@@ skipping 44 matching lines @@
 
 // 'reg' is not modified, 'temp' is trashed.
 // Fall through if double, jump to 'is_smi' if Smi and
 // jump to 'not_double_or_smi' if neither double nor Smi.
 void OptimizingCodeGenerator::CheckIfDoubleOrSmi(Register reg,
                                                  Register temp,
                                                  Label* is_smi,
                                                  Label* not_double_or_smi) {
   __ testl(reg, Immediate(kSmiTagMask));
   __ j(ZERO, is_smi);
-  __ movl(temp, FieldAddress(reg, Object::class_offset()));
-  __ CompareObject(temp, double_class_);
+  __ CompareClassOf(reg, double_class_, temp);
   __ j(NOT_EQUAL, not_double_or_smi);
 }
 
 
 // Result of the computation is a newly allocated double object or
 // a temporary object if the parent node specifies a CodeGenInfo for this node
 // and therefore knows how to handle a temporary. A temporary object cannot
 // be used for long living values (e.g., the ones stored on stack or into other
 // objects).
 // Implement for combinations: Double/Double, Double/Smi, Smi/Double, as
@@ skipping 346 matching lines @@
     // TODO(srdjan): Do not hardwire register.
     UNIMPLEMENTED();
   }
   DeoptimizationBlob* deopt_blob =
       AddDeoptimizationBlob(node, EBX, kDeoptInstanceGetterSameTarget);
   if (NodeMayBeSmi(receiver)) {
     __ testl(EBX, Immediate(kSmiTagMask));
     __ j(ZERO, deopt_blob->label());
   }
 
-  __ movl(EAX, FieldAddress(EBX, Object::class_offset()));
+  __ LoadObjectClassId(EAX, EBX);
   const ICData& ic_data = node->ic_data();
   Function& target = Function::Handle();
   Label load_field;
   for (intptr_t i = 0; i < ic_data.NumberOfChecks(); i++) {
-    Class& cls = Class::ZoneHandle();
+    Class& cls = Class::Handle();
     ic_data.GetOneClassCheckAt(i, &cls, &target);
-    __ CompareObject(EAX, cls);
+    __ CompareClassId(EAX, cls);

[Ivan Posva, 2012/05/26 04:34:46]

Not sure about others, but I find this more readable:
__ cmpl(EAX, Immediate(cls.index()));

[Vyacheslav Egorov (Google), 2012/05/26 16:48:22]

ok. I will inline them. I agree that macros does not buy as anything in this
case. I introduced it only because I wanted to be able to quickly find places
where I compare loaded class ids.

     if (i == (ic_data.NumberOfChecks() - 1)) {
       __ j(NOT_EQUAL, deopt_blob->label());
     } else {
       __ j(EQUAL, &load_field);
     }
   }
   Class& cls = Class::Handle();
   ic_data.GetOneClassCheckAt(0, &cls, &target);
 
   __ Bind(&load_field);
@@ skipping 182 matching lines @@
     }
   }
   // TODO(srdjan): sort classes/target by their invocation count.
   DeoptimizationBlob* deopt_blob = AddDeoptimizationBlob(
       node, recv_reg, value_reg, kDeoptInstanceSetterSameTarget);
   // Deoptimize if Smi, since they do not have setters.
   if (NodeMayBeSmi(receiver)) {
     __ testl(recv_reg, Immediate(kSmiTagMask));
     __ j(ZERO, deopt_blob->label());
   }
-  __ movl(EBX, FieldAddress(recv_reg, Object::class_offset()));
+  __ LoadObjectClassId(EBX, recv_reg);
   // Initialize setter arguments, but leave the class and target fields NULL.
   InstanceSetterArgs setter_args =
       {NULL, NULL, &field_name, recv_reg, value_reg,
       node->id(), node->token_index()};
 
   if (unique_target) {
     Label store_field;
     for (intptr_t i = 0; i < classes.length(); i++) {
-      __ CompareObject(EBX, *classes[i]);
+      __ CompareClassId(EBX, *classes[i]);
       if (i == (classes.length() - 1)) {
         __ j(NOT_EQUAL, deopt_blob->label());
       } else {
         __ j(EQUAL, &store_field);
       }
     }
     __ Bind(&store_field);
     setter_args.cls = classes[0];
     setter_args.target = targets[0];
     GenerateInstanceSetter(setter_args);
@@ skipping 333 matching lines @@
     DeoptimizationBlob* deopt_blob =
         AddDeoptimizationBlob(node, EAX, EDX, kDeoptEqualityNoFeedback);
     __ j(NOT_EQUAL, deopt_blob->label());
   } else {
     DeoptimizationBlob* deopt_blob =
         AddDeoptimizationBlob(node, EAX, EDX, kDeoptEqualityClassCheck);
     __ j(EQUAL, &compare);
     // Smi causes deoptimization.
     __ testl(EAX, Immediate(kSmiTagMask));
     __ j(ZERO, deopt_blob->label());
-    __ movl(EBX, FieldAddress(EAX, Object::class_offset()));
+    __ LoadObjectClassId(EBX, EAX);
     for (intptr_t i = 0; i < num_classes; i++) {
       const Class& cls = *(*classes)[i];
-      __ CompareObject(EBX, cls);
+      __ CompareClassId(EBX, cls);
       if (i == (num_classes - 1)) {
         __ j(NOT_EQUAL, deopt_blob->label());
       } else {
         __ j(EQUAL, &compare);
       }
     }
   }
   __ Bind(&compare);
   __ cmpl(EAX, EDX);
   if (NodeInfoHasLabels(node)) {
@@ skipping 157 matching lines @@
     CodeGenInfo index_info(node->index_expr());
     VisitLoadTwo(node->array(), node->index_expr(), EBX, EDX);
     DeoptimizationBlob* deopt_blob =
         AddDeoptimizationBlob(node, EBX, EDX, kDeoptLoadIndexedFixedArray);
     const Class& test_class = NodeHasClassAt(node, object_array_class, 0) ?
         object_array_class : immutable_object_array_class;
     // Type checks of array.
     if (!array_info.IsClass(test_class)) {
       __ testl(EBX, Immediate(kSmiTagMask));  // Deoptimize if Smi.
       __ j(ZERO, deopt_blob->label());
-      __ movl(EAX, FieldAddress(EBX, Object::class_offset()));
-      __ CompareObject(EAX, test_class);
+      __ CompareClassOf(EBX, test_class, EAX);
       __ j(NOT_EQUAL, deopt_blob->label());
       PropagateBackLocalClass(node->array(), test_class);
     }
 
     // Type check of index.
     if (!index_info.IsClass(smi_class_)) {
       __ testl(EDX, Immediate(kSmiTagMask));
       __ j(NOT_ZERO, deopt_blob->label());
       PropagateBackLocalClass(node->index_expr(), smi_class_);
     }
@@ skipping 16 matching lines @@
         AddDeoptimizationBlob(node, EDX, EAX, kDeoptLoadIndexedGrowableArray);
     // EAX: index, EDX: array.
     if (!index_info.IsClass(smi_class_)) {
       __ testl(EAX, Immediate(kSmiTagMask));
       __ j(NOT_ZERO, deopt_blob->label());  // Not Smi index.
       PropagateBackLocalClass(node->index_expr(), smi_class_);
     }
     if (!array_info.IsClass(growable_object_array_class_)) {
       __ testl(EDX, Immediate(kSmiTagMask));
       __ j(ZERO, deopt_blob->label());  // Array is Smi.
-      __ movl(EBX, FieldAddress(EDX, Object::class_offset()));
-      __ CompareObject(EBX, growable_object_array_class_);
+      __ CompareClassOf(EDX, growable_object_array_class_, EBX);
       __ j(NOT_EQUAL, deopt_blob->label());  // Not GrowableObjectArray.
       PropagateBackLocalClass(node->array(), growable_object_array_class_);
     }
     // Range check: deoptimize if out of bounds.
     __ cmpl(EAX, FieldAddress(EDX, GrowableObjectArray::length_offset()));
     __ j(ABOVE_EQUAL, deopt_blob->label());
     __ movl(EDX, FieldAddress(EDX, GrowableObjectArray::data_offset()));
     // Note that EAX is Smi, i.e, times 2.
     ASSERT(kSmiTagShift == 1);
     __ movl(EAX, FieldAddress(EDX, EAX, TIMES_2, sizeof(RawArray)));
@@ skipping 46 matching lines @@
     VisitLoadOne(node->value(), ECX);
     DeoptimizationBlob* deopt_blob =
         AddDeoptimizationBlob(node, EAX, EBX, ECX, kDeoptStoreIndexed);
     __ popl(EBX);  // index.
     __ popl(EAX);  // array.
     // ECX: value, EBX:index, EAX: array.
     // Check class of array.
     if (class_of_this_array.raw() != object_array_class.raw()) {
       __ testl(EAX, Immediate(kSmiTagMask));
       __ j(ZERO, deopt_blob->label());  // Array is smi -> deopt.
-      __ movl(EDX, FieldAddress(EAX, Object::class_offset()));
-      __ CompareObject(EDX, object_array_class);
+      __ CompareClassOf(EAX, object_array_class, EDX);
       __ j(NOT_EQUAL, deopt_blob->label());  // Not ObjectArray -> deopt.
       PropagateBackLocalClass(node->array(), object_array_class);
     }
     // Check class of index.
     if (!index_is_smi) {
       __ testl(EBX, Immediate(kSmiTagMask));
       __ j(NOT_ZERO, deopt_blob->label());  // Index not Smi -> deopt.
       PropagateBackLocalClass(node->index_expr(), smi_class_);
     }
     // Range check.
@@ skipping 19 matching lines @@
     VisitLoadOne(node->value(), ECX);
     DeoptimizationBlob* deopt_blob =
         AddDeoptimizationBlob(node, EAX, EBX, ECX, kDeoptStoreIndexed);
     __ popl(EBX);  // index.
     __ popl(EAX);  // array.
     // ECX: value, EBX:index, EAX: array, EDX: scratch.
     // Check class of array.
     if (class_of_this_array.raw() != growable_object_array_class_.raw()) {
       __ testl(EAX, Immediate(kSmiTagMask));
       __ j(ZERO, deopt_blob->label());  // Array is smi -> deopt.
-      __ movl(EDX, FieldAddress(EAX, Object::class_offset()));
-      __ CompareObject(EDX, growable_object_array_class_);
+      __ CompareClassOf(EAX, growable_object_array_class_, EDX);
       __ j(NOT_EQUAL, deopt_blob->label());  // Not GrowableObjectArray.
       PropagateBackLocalClass(node->array(), growable_object_array_class_);
     }
     // Check class of index.
     if (!index_is_smi) {
       __ testl(EBX, Immediate(kSmiTagMask));
       __ j(NOT_ZERO, deopt_blob->label());  // Index not Smi -> deopt.
       PropagateBackLocalClass(node->index_expr(), smi_class_);
     }
     // Range check: deoptimize if out of bounds.
@@ skipping 307 matching lines @@
     __ jmp(&done);
     __ Bind(&not_smi);  // Continue with other test below.
   } else if (NodeMayBeSmi(receiver)) {
     DeoptimizationBlob* deopt_blob =
         AddDeoptimizationBlob(node, kDeoptCheckedInstanceCallSmiFail);
     __ testl(EAX, Immediate(kSmiTagMask));
     __ j(ZERO, deopt_blob->label());
   } else {
     // Receiver cannot be Smi, no need to test it.
   }
-  __ movl(EAX, FieldAddress(EAX, Object::class_offset()));  // Receiver's class.
+  __ LoadObjectClassId(EAX, EAX);  // Receiver's class.
   for (intptr_t i = start_ix; i < classes.length(); i++) {
     const Class& cls = *classes[i];
     const Function& target = *targets[i];
-    __ CompareObject(EAX, cls);
+    __ CompareClassId(EAX, cls);
     if (i == (classes.length() - 1)) {
       // Last check.
       DeoptimizationBlob* deopt_blob =
           AddDeoptimizationBlob(node, kDeoptCheckedInstanceCallCheckFail);
       __ j(NOT_EQUAL, deopt_blob->label());
       GenerateDirectCall(node->id(),
                          token_index,
                          target,
                          num_args,
                          optional_arguments_names);
@@ skipping 236 matching lines @@
     }
   }
   // TODO(srdjan): Implement unary kSUB (negate) Mint.
   CodeGenerator::VisitUnaryOpNode(node);
 }
 
 
 }  // namespace dart
 
 #endif  // defined TARGET_ARCH_IA32