Collect type feedback for fields.

runtime/vm/intermediate_language_ia32.cc

Index: runtime/vm/intermediate_language_ia32.cc
diff --git a/runtime/vm/intermediate_language_ia32.cc b/runtime/vm/intermediate_language_ia32.cc
index ad278969fa2cec01bd270a3125373bd443d4b9dc..ef9998ceb7e0695933b7763874c1a3545d217a5e 100644
--- a/runtime/vm/intermediate_language_ia32.cc
+++ b/runtime/vm/intermediate_language_ia32.cc
@@ -13,6 +13,7 @@
 #include "vm/locations.h"
 #include "vm/object_store.h"
 #include "vm/parser.h"
+#include "vm/stack_frame.h"
 #include "vm/stub_code.h"
 #include "vm/symbols.h"
 
@@ -1508,6 +1509,163 @@ void StoreIndexedInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
 }
 
 
+LocationSummary* GuardFieldInstr::MakeLocationSummary() const {
+  const intptr_t kNumInputs = 1;
+  LocationSummary* summary =
+      new LocationSummary(kNumInputs, 0, LocationSummary::kNoCall);
+  summary->set_in(0, Location::RequiresRegister());
+  if ((value()->Type()->ToCid() == kDynamicCid) &&
+      (field().guarded_cid() != kSmiCid)) {
+    summary->AddTemp(Location::RequiresRegister());
+  }
+  if (field().guarded_cid() == kIllegalCid) {
+    summary->AddTemp(Location::RequiresRegister());
+  }
+  return summary;
+}
+
+
+static void LoadValueCid(FlowGraphCompiler* compiler,

[Kevin Millikin (Google), 2013/03/15 12:17:21]

I think we have this pattern in a few places. EmitEqualityAsPolymorphicCall is
one.  In  StubCode::GenerateNArgsCheckInlineCacheStub we have almost the same
thing, but with a return in the middle an a pair of smi tag assembly code
sequences.  If we combined the returns we could have a single smi tag sequence
and it would fit the pattern too.

I'm not sure if it's worth it to combine, but it seems like it's getting
duplicated in a bunch of places now.

[Vyacheslav Egorov (Google), 2013/03/15 13:01:10]

Done.

+                         Register value_reg,
+                         Register value_cid_reg) {

[Kevin Millikin (Google), 2013/03/15 12:17:21]

I guess the order of these registers should be swapped to fit our dst, src
ordering convention.

[Vyacheslav Egorov (Google), 2013/03/15 13:01:10]

Done.

+  Label not_smi, cid_loaded;
+  __ testl(value_reg, Immediate(kSmiTagMask));
+  __ j(NOT_ZERO, &not_smi, Assembler::kNearJump);
+  __ movl(value_cid_reg, Immediate(kSmiCid));
+  __ jmp(&cid_loaded);
+  __ Bind(&not_smi);
+  __ LoadClassId(value_cid_reg, value_reg);
+  __ Bind(&cid_loaded);
+}
+
+void GuardFieldInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
+  const intptr_t field_cid = field().guarded_cid();
+  const intptr_t nullability = field().is_nullable() ? kNullCid : kIllegalCid;
+
+  if (field_cid == kDynamicCid) {
+    ASSERT(!compiler->is_optimizing());
+    return;  // Nothing to emit.
+  }
+
+  const intptr_t value_cid = value()->Type()->ToCid();
+
+  Register value_reg = locs()->in(0).reg();
+
+  Register value_cid_reg = ((value_cid == kDynamicCid) &&
+      (field_cid != kSmiCid)) ? locs()->temp(0).reg() : kNoRegister;
+
+  Register field_reg = (field_cid == kIllegalCid) ?
+      locs()->temp(locs()->temp_count() - 1).reg() : kNoRegister;
+
+  Label ok, fail_label;
+
+  Label* deopt = compiler->is_optimizing() ?
+      compiler->AddDeoptStub(deopt_id(), kDeoptGuardField) : NULL;
+
+  Label* fail = (deopt != NULL) ? deopt : &fail_label;
+
+  const bool ok_is_fall_through = (deopt != NULL);
+
+  if (!compiler->is_optimizing() || (field_cid == kIllegalCid)) {
+    if (field_reg == kNoRegister) {

[Kevin Millikin (Google), 2013/03/15 12:17:21]

It seems to me that

(field_reg == kNoRegister) iff. (field_cid != kIllegalCid)

And by hypothesis

!compiler->is_optimizing() || (field_cid == kIllegalCid)

So instead of this condition you could write

!compiler->is_optimizing()

which makes the choice of EBX below less surprising.  Or assert it as below for
the value_cid_reg case.  Also, a comment.

[Vyacheslav Egorov (Google), 2013/03/15 13:01:10]

Done.

+      field_reg = EBX;
+      ASSERT((field_reg != value_reg) && (field_reg != value_cid_reg));
+    }
+
+    __ LoadObject(field_reg, Field::ZoneHandle(field().raw()));
+
+    FieldAddress field_cid_operand(field_reg, Field::guarded_cid_offset());
+    FieldAddress field_nullability_operand(
+        field_reg, Field::is_nullable_offset());
+
+    if (value_cid == kDynamicCid) {
+      if (value_cid_reg == kNoRegister) {
+        ASSERT(!compiler->is_optimizing());
+        value_cid_reg = EDX;
+        ASSERT((value_cid_reg != value_reg) && (field_reg != value_cid_reg));
+      }
+
+      LoadValueCid(compiler, value_reg, value_cid_reg);
+
+      __ cmpl(value_cid_reg, field_cid_operand);
+      __ j(EQUAL, &ok);
+      __ cmpl(value_cid_reg, field_nullability_operand);
+    } else if (value_cid == kNullCid) {
+      __ cmpl(field_nullability_operand, Immediate(value_cid));
+    } else {
+      __ cmpl(field_cid_operand, Immediate(value_cid));
+    }
+    __ j(EQUAL, &ok);
+
+    __ cmpl(field_cid_operand, Immediate(kIllegalCid));
+    __ j(NOT_EQUAL, fail);
+
+    if (value_cid == kDynamicCid) {
+      __ movl(field_cid_operand, value_cid_reg);
+      __ movl(field_nullability_operand, value_cid_reg);
+    } else {
+      __ movl(field_cid_operand, Immediate(value_cid));
+      __ movl(field_nullability_operand, Immediate(value_cid));
+    }
+
+    if (!ok_is_fall_through) {
+      __ jmp(&ok);
+    }
+  } else {
+    if (value_cid == kDynamicCid) {
+      // Field's guarded class id is fixed by value's class id is not known.

[Kevin Millikin (Google), 2013/03/15 12:17:21]

by ==> but

[Vyacheslav Egorov (Google), 2013/03/15 13:01:10]

Done.

+      __ testl(value_reg, Immediate(kSmiTagMask));
+
+      if (field_cid != kSmiCid) {
+        __ j(ZERO, fail);
+        __ LoadClassId(value_cid_reg, value_reg);
+        __ cmpl(value_cid_reg, Immediate(field_cid));
+      }
+
+      if (field().is_nullable() && (field_cid != kNullCid)) {
+        __ j(EQUAL, &ok);
+        const Immediate& raw_null =
+            Immediate(reinterpret_cast<intptr_t>(Object::null()));
+        __ cmpl(value_reg, raw_null);
+      }
+
+      if (ok_is_fall_through) {
+        __ j(NOT_EQUAL, fail);
+      } else {
+        __ j(EQUAL, &ok);
+      }
+    } else {
+      // Both value's and field's class id is known.
+      if ((value_cid != field_cid) && (value_cid != nullability)) {
+        if (ok_is_fall_through) {
+          __ jmp(fail);
+        }
+      } else {
+        // Nothing to emit.
+        ASSERT(!compiler->is_optimizing());
+        return;
+      }
+    }
+  }
+
+  if (deopt == NULL) {
+    ASSERT(!compiler->is_optimizing());
+    __ Bind(fail);
+
+    __ cmpl(FieldAddress(field_reg, Field::guarded_cid_offset()),
+            Immediate(kDynamicCid));
+    __ j(EQUAL, &ok);
+
+    __ pushl(field_reg);
+    __ pushl(value_reg);
+    __ CallRuntime(kUpdateFieldCidRuntimeEntry);
+    __ Drop(2);  // Drop the field and the value.

[Kevin Millikin (Google), 2013/03/15 12:17:21]

You're right, it's still spaghetti.  But now it's OUR kind of spaghetti.  :(

+  }
+
+  __ Bind(&ok);
+}
+
+
 LocationSummary* StoreInstanceFieldInstr::MakeLocationSummary() const {
   const intptr_t kNumInputs = 2;
   const intptr_t num_temps =  0;
@@ -2828,16 +2986,28 @@ void BranchInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
 
 LocationSummary* CheckClassInstr::MakeLocationSummary() const {
   const intptr_t kNumInputs = 1;
-  const intptr_t kNumTemps = 1;
+  const intptr_t kNumTemps = 0;
   LocationSummary* summary =
       new LocationSummary(kNumInputs, kNumTemps, LocationSummary::kNoCall);
   summary->set_in(0, Location::RequiresRegister());
-  summary->set_temp(0, Location::RequiresRegister());
+  if (!null_check()) {
+    summary->AddTemp(Location::RequiresRegister());
+  }
   return summary;
 }
 
 
 void CheckClassInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
+  if (null_check()) {
+    Label* deopt = compiler->AddDeoptStub(deopt_id(),
+                                          kDeoptCheckClass);
+    const Immediate& raw_null =
+        Immediate(reinterpret_cast<intptr_t>(Object::null()));
+    __ cmpl(locs()->in(0).reg(), raw_null);
+    __ j(EQUAL, deopt);
+    return;
+  }
+
   ASSERT((unary_checks().GetReceiverClassIdAt(0) != kSmiCid) ||
          (unary_checks().NumberOfChecks() > 1));
   Register value = locs()->in(0).reg();