Remove half of the checks for comparing two Obejcts

src/hydrogen-check-elimination.cc

 // 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
@@ skipping 83 matching lines @@
         break;
       }
       case HValue::kCheckMapValue: {
         ReduceCheckMapValue(HCheckMapValue::cast(instr));
         break;
       }
       case HValue::kCheckHeapObject: {
         ReduceCheckHeapObject(HCheckHeapObject::cast(instr));
         break;
       }
+      case HValue::kCompareObjectEqAndBranch: {
+        ReduceCompareObjectEqAndBranch(HCompareObjectEqAndBranch::cast(instr));
+        break;
+      }
       default: {
         // If the instruction changes maps uncontrollably, drop everything.
         if (instr->CheckGVNFlag(kChangesMaps) ||
             instr->CheckGVNFlag(kChangesOsrEntries)) {
           Kill();
         }
       }
       // Improvements possible:
       // - eliminate redundant HCheckSmi, HCheckInstanceType instructions
       // - track which values have been HCheckHeapObject'd
@@ skipping 145 matching lines @@
   }
 
   void ReduceCheckHeapObject(HCheckHeapObject* instr) {
     if (FindMaps(instr->value()->ActualValue()) != NULL) {
       // If the object has known maps, it's definitely a heap object.
       instr->DeleteAndReplaceWith(instr->value());
       INC_STAT(removed_cho_);
     }
   }
 
+  void ReduceCompareObjectEqAndBranch(HCompareObjectEqAndBranch* instr) {
+    HValue* left = instr->left();
+    HValue* right = instr->right();
+    if (left->IsCheckMaps() && right->IsCheckMaps()) {
+      // Check one operand is enough. After licm, remove the correct one.
+      HCheckMaps* removed = NULL;
+      if (left->block()->block_id() > right->block()->block_id()) {
+        removed = HCheckMaps::cast(left);
+      } else {
+        removed = HCheckMaps::cast(right);
+      }
+
+      if (removed->previous()->IsCheckHeapObject()) {
+        HCheckHeapObject* cho = HCheckHeapObject::cast(removed->previous());
+        if (cho->value() == removed->ActualValue()) {
+          cho->DeleteAndReplaceWith(cho->value());
+          INC_STAT(removed_cho_);
+        }
+      }
+      removed->DeleteAndReplaceWith(removed->ActualValue());

[titzer, 2013/12/04 16:32:00]

This transformation is completely incorrect. First, you haven't even checked
whether the two CheckMaps refer to the same object. Second, you haven't checked
the relationship between the CheckMaps either (does one entail another?). Third,
an object's map can change in between these instructions. Fourth, removing the
check heap object in between is only legal if the CheckMaps refer to the the
same object. Fourth, I have no idea why you choose to match this pattern at the
CompareObjectEqAndBranch instruction.

But, you did bring one optimization opportunity to mind. We can remove some
CheckHeapObject instructions for objects whose maps may no longer be known, but
they are still known to be heap objects.

+      INC_STAT(removed_);
+    }
+  }
+
   void ReduceStoreNamedField(HStoreNamedField* instr) {
     HValue* object = instr->object()->ActualValue();
     if (instr->has_transition()) {
       // This store transitions the object to a new map.
       Kill(object);
       Insert(object, MapConstant(instr->transition()));
     } else if (IsMapAccess(instr->access())) {
       // This is a store directly to the map field of the object.
       Kill(object);
       if (!instr->value()->IsConstant()) return;
@@ skipping 248 matching lines @@
   PRINT_STAT(removed_cho);
   PRINT_STAT(narrowed);
   PRINT_STAT(loads);
   PRINT_STAT(empty);
   PRINT_STAT(compares_true);
   PRINT_STAT(compares_false);
   PRINT_STAT(transitions);
 }
 
 } }  // namespace v8::internal