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..5aa43347aac2e36a7e82c571c6121b8f9800c55d 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,256 @@ 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) {
+    summary->AddTemp(Location::RequiresRegister());
+  }
+  if (field().guarded_cid() == kIllegalCid) {
+    summary->AddTemp(Location::RequiresRegister());
+  }
+  return summary;
+}
+
+
+static void PushCid(FlowGraphCompiler* compiler, const Immediate& imm) {
+  __ pushl(Immediate(Smi::RawValue(imm.value())));
+}
+
+
+static void PushCid(FlowGraphCompiler* compiler, Register reg) {
+  __ SmiTag(reg);
+  __ pushl(reg);
+}
+
+
+template<typename ValueCidOperandType>

[Kevin Millikin (Google), 2013/03/13 16:03:36]

I'm still not liking the templates.  As the writer, it's probably great to avoid
duplication.  But as the reader, I would rather see more explicitly the code
that will be generated.

I don't think it's so bad to break down all the cases.  Correct me if my
reasoning here is wrong.

There are three type parameters, value, field, and nullability.  Value can be
register or an immediate, field and nullability are always the same type---both
immediate or both address---so there are only four possibilities.  One
possibility, immediate value and immediate field, is impossible because it
represents a statically known value type and a statically expected field type. 
The statically known value type can only happen in optimized code, but in that
case we have either definitely eliminated the guard or compiled an unconditional
deopt.

So there are three possibilities:

1. Statically known expected field type, unknown value type, not optimizing
2. Not statically known expected field type, statically known value type,
optimizing
3. Not statically known expected field type, unknown value type, may or may not
be optimizing

If that's right, maybe we could try a pair of functions: one for case 1 (static
cid guard) and one templated over the value operand type for case 2 (dynamic cid
guard).

+static void EmitCidGuardInit(FlowGraphCompiler* compiler,
+                                       ValueCidOperandType value_cid,

[Kevin Millikin (Google), 2013/03/13 16:03:36]

Indentation is screwy.

+                                       const Immediate& field_cid,
+                                       const Immediate& nullability,
+                                       Label* deopt,
+                                       Label* ok) {
+  // nothing to do.
+}
+
+
+template<typename ValueCidOperandType>
+static void EmitCidGuardInit(FlowGraphCompiler* compiler,

[Kevin Millikin (Google), 2013/03/13 16:03:36]

According to the suggestion above, you could inline this code.

+                             ValueCidOperandType value_cid,
+                             const FieldAddress& field_cid,
+                                       const FieldAddress& nullability,

[Kevin Millikin (Google), 2013/03/13 16:03:36]

Indentation is screwy.

+                                       Label* deopt,
+                                       Label* ok) {
+  // Check if field is initialized for the first time.
+  Label fail;
+  __ cmpl(field_cid, Immediate(kIllegalCid));
+  __ j(NOT_EQUAL, (deopt != NULL) ? deopt : &fail);
+  __ movl(field_cid, value_cid);
+  __ movl(nullability, value_cid);
+  __ jmp(ok);

[Kevin Millikin (Google), 2013/03/13 16:03:36]

In the case that deopt != NULL, then this will emit a jmp to ok and the caller
will immediately Bind(&ok).  Move the jmp into the if below?

+  if (deopt == NULL) {
+    __ Bind(&fail);
+  }
+}
+
+
+static void CompareJump(FlowGraphCompiler* compiler,

[Kevin Millikin (Google), 2013/03/13 16:03:36]

I don't think this one is necessary.

+                        Condition cond,
+                        const Immediate& left,
+                        const Immediate& right,
+                        Label* target) {
+  if ((left.value() == right.value()) && (cond == EQUAL)) {
+    __ jmp(target);
+  } else if ((left.value() != right.value()) && cond == NOT_EQUAL) {
+    __ jmp(target);
+  }
+}
+
+
+static void CompareJump(FlowGraphCompiler* compiler,
+                        Condition cond,
+                        Immediate left,
+                        Register right,
+                        Label* target) {
+  ASSERT((cond == EQUAL) || (cond == NOT_EQUAL));
+  __ cmpl(right, left);
+  __ j(cond, target);
+}
+
+
+template<typename LeftOperandType, typename RightOperandType>
+static void CompareJump(FlowGraphCompiler* compiler,
+                        Condition cond,
+                        LeftOperandType left,
+                        RightOperandType right,
+                        Label* target) {
+  __ cmpl(left, right);
+  __ j(cond, target);
+}
+
+
+template<typename ValueCidOperandType,
+         typename FieldCidOperandType,
+         typename FieldNullabilityOperandType>
+static void EmitCidGuard(FlowGraphCompiler* compiler,
+                         const Field& field,
+                         ValueCidOperandType value_cid,
+                         FieldCidOperandType field_cid,
+                         FieldNullabilityOperandType nullability,
+                         Label* deopt) {
+  Label ok, update;
+  CompareJump(compiler, EQUAL, field_cid, value_cid, &ok);
+  if ((deopt != NULL) && (field.guarded_cid() != kIllegalCid)) {
+    CompareJump(compiler, NOT_EQUAL, nullability, value_cid, deopt);

[Kevin Millikin (Google), 2013/03/13 16:03:36]

I think this case is impossible.  The caller has already handled deopt != NULL
and field_cid != kIllegalCid.

+  } else {
+    CompareJump(compiler, EQUAL, nullability, value_cid, &ok);
+  }
+
+  EmitCidGuardInit(compiler, value_cid, field_cid, nullability, deopt, &ok);
+
+  if (deopt == NULL) {
+    // Switch field's cid to dynamic and notify runtime.
+    CompareJump(compiler, EQUAL, field_cid, Immediate(kDynamicCid), &ok);
+    __ PushObject(field);
+    PushCid(compiler, value_cid);
+    __ CallRuntime(kUpdateFieldCidRuntimeEntry);
+    __ Drop(2);
+  }
+
+  __ Bind(&ok);
+}
+
+
+template<typename FieldCidOperandType, typename FieldNullabilityOperandType>
+static void EmitCidGuard(FlowGraphCompiler* compiler,
+                         const Field& field,
+                         intptr_t value_cid,
+                         Register value_cid_reg,
+                         FieldCidOperandType field_cid,
+                         FieldNullabilityOperandType nullability,
+                         Label* deopt) {
+  if (value_cid == kDynamicCid) {
+    EmitCidGuard(compiler, field, value_cid_reg, field_cid, nullability, deopt);
+  } else {
+    EmitCidGuard(compiler,
+                 field,
+                 Immediate(value_cid),
+                 field_cid,
+                 nullability,
+                 deopt);
+  }
+}
+
+
+static void EmitFieldGuard(FlowGraphCompiler* compiler,
+                           Register value_reg,
+                           const Field& field,
+                           intptr_t value_cid,
+                           Register value_cid_reg,
+                           intptr_t field_cid,
+                           intptr_t nullability,
+                           Register field_reg,
+                           Label* deopt) {
+  if (value_cid == kDynamicCid) {
+    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);
+  }
+
+  if (field_cid == kIllegalCid) {
+    __ LoadObject(field_reg, Field::ZoneHandle(field.raw()));
+    EmitCidGuard(compiler,
+                 field,
+                 value_cid,
+                 value_cid_reg,
+                 FieldAddress(field_reg, Field::guarded_cid_offset()),
+                 FieldAddress(field_reg, Field::is_nullable_offset()),
+                 deopt);
+  } else {
+    EmitCidGuard(compiler,
+                 field,
+                 value_cid,
+                 value_cid_reg,
+                 Immediate(field_cid),
+                 Immediate(nullability),
+                 deopt);
+  }
+}
+
+
+void GuardFieldInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
+  const intptr_t field_cid = field().guarded_cid();
+
+  if (field_cid == kDynamicCid) {
+    ASSERT(!compiler->is_optimizing());
+    return;  // Nothing to emit.
+  }
+
+  const intptr_t value_cid = value()->Type()->ToCid();
+  const intptr_t nullability = field().is_nullable() ? kNullCid : kIllegalCid;
+

[Kevin Millikin (Google), 2013/03/13 16:03:36]

There is an extra blank line here.

+
+  Register value_reg = locs()->in(0).reg();
+
+  Register value_cid_reg = (value_cid == kDynamicCid) ?
+      locs()->temp(0).reg() : kNoRegister;
+
+  Label* deopt = compiler->is_optimizing() && CanDeoptimize() ?

[Kevin Millikin (Google), 2013/03/13 16:03:36]

CanDeoptimize() returns the constant true, or am I missing something?  If I'm
right, maybe this code can be simplified.

+      compiler->AddDeoptStub(deopt_id(), kDeoptStoreInstanceField) : NULL;
+
+  if ((deopt != NULL) && (field_cid != kIllegalCid)) {
+    if (value_cid != kDynamicCid) {
+      ASSERT(field_cid != value_cid);
+      __ jmp(deopt);
+      return;
+    }
+
+    if (field_cid == kSmiCid) {
+      __ testl(value_reg, Immediate(kSmiTagMask));
+    } else {
+      ASSERT(value_cid_reg != kNoRegister);
+      __ testl(value_reg, Immediate(kSmiTagMask));
+      __ j(ZERO, deopt);
+      __ LoadClassId(value_cid_reg, value_reg);
+      __ cmpl(value_cid_reg, Immediate(field_cid));
+    }
+
+    if (field().is_nullable()) {
+      Label ok;
+      __ j(EQUAL, &ok);
+      const Immediate& raw_null =
+          Immediate(reinterpret_cast<intptr_t>(Object::null()));
+      __ cmpl(value_reg, raw_null);
+      __ j(NOT_EQUAL, deopt);
+      __ Bind(&ok);
+    } else {
+      __ j(NOT_EQUAL, deopt);
+    }
+
+    return;
+  }
+
+  Register field_reg = (field_cid == kIllegalCid) ?
+      locs()->temp(value_cid_reg == kNoRegister ? 0 : 1).reg() : kNoRegister;
+
+  EmitFieldGuard(compiler,
+                 value_reg,
+                 field(),
+                 value_cid, value_cid_reg,
+                 field_cid, nullability, field_reg,
+                 deopt);
+}
+
+
 LocationSummary* StoreInstanceFieldInstr::MakeLocationSummary() const {
   const intptr_t kNumInputs = 2;
   const intptr_t num_temps =  0;
@@ -2828,16 +3079,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();