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

src/ia32/lithium-codegen-ia32.cc

Index: src/ia32/lithium-codegen-ia32.cc
===================================================================
--- src/ia32/lithium-codegen-ia32.cc	(revision 7299)
+++ src/ia32/lithium-codegen-ia32.cc	(working copy)
@@ -2122,6 +2122,58 @@
 }
 
 
+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));