Enable GVN for polymorphic loads by not expanding them at the HIR level.

src/ia32/lithium-codegen-ia32.cc

 // Copyright 2011 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 2104 matching lines @@
   Register result = ToRegister(instr->result());
   if (instr->hydrogen()->is_in_object()) {
     __ mov(result, FieldOperand(object, instr->hydrogen()->offset()));
   } else {
     __ mov(result, FieldOperand(object, JSObject::kPropertiesOffset));
     __ mov(result, FieldOperand(result, instr->hydrogen()->offset()));
   }
 }
 
 
+void LCodeGen::EmitLoadField(Register result,
+                              Register object,

[Kevin Millikin (Chromium), 2011/03/23 08:08:53]

Indentation is off here.

[fschneider, 2011/03/23 13:57:56]

Done.

+                              Handle<Map> type,
+                              LookupResult* lookup) {
+  ASSERT(lookup->IsProperty() && lookup->type() == FIELD);
+  int index = lookup->GetLocalFieldIndexFromMap(*type);
+  int offset = index * kPointerSize;
+  if (index < 0) {
+    // Negative property indices are in-object properties, indexed
+    // from the end of the fixed part of the object.
+    __ mov(result, FieldOperand(object, offset + type->instance_size()));
+  } else {
+    // Non-negative property indices are in the properties array.
+    __ mov(result, FieldOperand(object, JSObject::kPropertiesOffset));
+    __ mov(result, FieldOperand(result, offset + FixedArray::kHeaderSize));
+  }
+}
+
+
+void LCodeGen::DoLoadNamedFieldPolymorphic(LLoadNamedFieldPolymorphic* instr) {
+  Register object = ToRegister(instr->InputAt(0));

[Kevin Millikin (Chromium), 2011/03/23 08:08:53]

I don't like relying on the order of the operands. This will still compile if
the order changes, and it's hard to grep for InputAt and find sites that need to
change.

We should act as if InputAt is private and define an accessor for object().

[fschneider, 2011/03/23 13:57:56]

Done.

+  Register result = ToRegister(instr->result());
+  NearLabel done;
+  int num_maps = instr->hydrogen()->types()->length();

[Kevin Millikin (Chromium), 2011/03/23 08:08:53]

map_count is only one character longer and avoids the abbreviation :)

[fschneider, 2011/03/23 13:57:56]

Done.

+  for (int i = 0; i < num_maps; ++i) {
+    Handle<Map> map = instr->hydrogen()->types()->at(i);
+    LookupResult lookup;
+    map->LookupInDescriptors(NULL, *instr->hydrogen()->name(), &lookup);
+
+    NearLabel next;
+    __ cmp(FieldOperand(object, HeapObject::kMapOffset), map);

[Kevin Millikin (Chromium), 2011/03/23 08:08:53]

We could avoid multiple loads of the map by using a temp register.

[fschneider, 2011/03/23 13:57:56]

I'll leave it as is in this change. Another way would be to have a HLoadMap
instructions. It would also allow reusing the map for a sequence of loads from
the same object.

[Kevin Millikin (Chromium), 2011/03/23 14:17:04]

OK, leaving it for later sounds good to me.

+    if (i == num_maps - 1 && !instr->hydrogen()->need_generic()) {
+      DeoptimizeIf(not_equal, instr->environment());
+    } else {
+      __ j(not_equal, &next);
+    }
+    EmitLoadField(result, object, map, &lookup);
+    if (i < num_maps - 1  || instr->hydrogen()->need_generic()) {

[Kevin Millikin (Chromium), 2011/03/23 08:08:53]

If you handle all but the last map in the loop, I think the code is simpler:

int count = instr->hydrogen()->types()->length();
for (int i = 0; i < count - 1; ++i) {
  // Lookup.
  __ cmp(FieldOperand(object, HeapObject::kMapOffset), map);
  __ j(not_equal, &next);
  EmitLoadField(result, object, map, &lookup);
  __ jmp(&done);
  __ bind(&next);
}
if (count > 0) {  // This is probably always true.
  __ cmp(FieldOperand(object, HeapObject::kMapOffset),
         instr->hydrogen()->types()->last());
  if (instr->hydrogen()->needs_generic()) {
    // Call
  } else {
    // Deoptimize
  }
}
__ bind(&done)

[fschneider, 2011/03/23 13:57:56]

Done.

+      __ jmp(&done);
+      __ bind(&next);
+    }
+  }
+  if (instr->hydrogen()->need_generic()) {
+    __ mov(ecx, instr->hydrogen()->name());
+    Handle<Code> ic(
+        isolate()->builtins()->builtin(Builtins::LoadIC_Initialize));
+    CallCode(ic, RelocInfo::CODE_TARGET, instr, false);
+  }
+  __ bind(&done);
+}
+
+
 void LCodeGen::DoLoadNamedGeneric(LLoadNamedGeneric* instr) {
   ASSERT(ToRegister(instr->context()).is(esi));
   ASSERT(ToRegister(instr->object()).is(eax));
   ASSERT(ToRegister(instr->result()).is(eax));
 
   __ mov(ecx, instr->name());
   Handle<Code> ic(isolate()->builtins()->builtin(Builtins::LoadIC_Initialize));
   CallCode(ic, RelocInfo::CODE_TARGET, instr);
 }
 
@@ skipping 1868 matching lines @@
   ASSERT(osr_pc_offset_ == -1);
   osr_pc_offset_ = masm()->pc_offset();
 }
 
 
 #undef __
 
 } }  // namespace v8::internal
 
 #endif  // V8_TARGET_ARCH_IA32