Fix a bug in optimized ARM code.

runtime/vm/flow_graph_compiler_arm.cc

Index: runtime/vm/flow_graph_compiler_arm.cc
===================================================================
--- runtime/vm/flow_graph_compiler_arm.cc	(revision 23151)
+++ runtime/vm/flow_graph_compiler_arm.cc	(working copy)
@@ -1290,7 +1290,65 @@
     intptr_t deopt_id,
     intptr_t token_pos,
     LocationSummary* locs) {
-  UNIMPLEMENTED();
+  MegamorphicCacheTable* table = Isolate::Current()->megamorphic_cache_table();
+  const String& name = String::Handle(ic_data.target_name());
+  const MegamorphicCache& cache =
+      MegamorphicCache::ZoneHandle(table->Lookup(name, arguments_descriptor));
+  Label not_smi, load_cache;
+  __ LoadFromOffset(kLoadWord, R0, SP, (argument_count - 1) * kWordSize);
+  __ tst(R0, ShifterOperand(kSmiTagMask));
+  __ b(&not_smi, NE);
+  __ mov(R0, ShifterOperand(Smi::RawValue(kSmiCid)));
+  __ b(&load_cache);
+
+  __ Bind(&not_smi);
+  __ LoadClassId(R0, R0);
+  __ SmiTag(R0);
+
+  // R0: class ID of the receiver (smi).
+  __ Bind(&load_cache);
+  __ LoadObject(R1, cache);
+  __ ldr(R2, FieldAddress(R1, MegamorphicCache::buckets_offset()));
+  __ ldr(R1, FieldAddress(R1, MegamorphicCache::mask_offset()));
+  // R2: cache buckets array.
+  // R1: mask.
+  __ mov(R3, ShifterOperand(R0));
+
+  Label loop, update, call_target_function;
+  __ b(&loop);
+
+  __ Bind(&update);
+  __ add(R3, R3, ShifterOperand(Smi::RawValue(1)));
+  __ Bind(&loop);
+  __ and_(R3, R3, ShifterOperand(R1));
+  const intptr_t base = Array::data_offset();
+  // R3 is smi tagged, but table entries are two words, so LSL 2.
+  __ add(IP, R2, ShifterOperand(R3, LSL, 2));
+  __ ldr(R4, FieldAddress(IP, base));
+
+  ASSERT(kIllegalCid == 0);
+  __ tst(R4, ShifterOperand(R4));
+  __ b(&call_target_function, EQ);
+  __ cmp(R4, ShifterOperand(R0));
+  __ b(&update, NE);
+
+  __ Bind(&call_target_function);
+  // Call the target found in the cache.  For a class id match, this is a
+  // proper target for the given name and arguments descriptor.  If the
+  // illegal class id was found, the target is a cache miss handler that can
+  // be invoked as a normal Dart function.
+  __ add(IP, R2, ShifterOperand(R3, LSL, 2));
+  __ ldr(R0, FieldAddress(IP, base + kWordSize));
+  __ ldr(R0, FieldAddress(R0, Function::code_offset()));
+  __ ldr(R0, FieldAddress(R0, Code::instructions_offset()));
+  __ LoadObject(R5, ic_data);
+  __ LoadObject(R4, arguments_descriptor);
+  __ AddImmediate(R0, Instructions::HeaderSize() - kHeapObjectTag);
+  __ blx(R0);
+  AddCurrentDescriptor(PcDescriptors::kOther, Isolate::kNoDeoptId, token_pos);
+  RecordSafepoint(locs);
+  AddDeoptIndexAtCall(Isolate::ToDeoptAfter(deopt_id), token_pos);
+  __ Drop(argument_count);

[zra, 2013/05/24 17:11:41]

Since the arguments are already pushed on the stack before entry to this
function, a comment about the stack layout might be helpful.

[regis, 2013/05/24 18:52:09]

Is that unexpected? The same is done for other call emitting functions. See
EmitInstanceCall above and EmitStaticCall below, for example.

 }
 
 
@@ -1449,16 +1507,44 @@
 
 
 Condition FlowGraphCompiler::FlipCondition(Condition condition) {

[zra, 2013/05/24 17:11:41]

It looks like this is only used in intermediate_language_*.cc, Maybe we could
move it over there, and then it could sit right next to NegateCondition.

[regis, 2013/05/24 18:52:09]

Done. There are other static functions transforming a condition. They are
usually declared close to their use, so I did the same for FlipCondition.

-  UNIMPLEMENTED();
-  return condition;
+  switch (condition) {
+    case EQ: return EQ;
+    case NE: return NE;
+    case LT: return GT;
+    case LE: return GE;
+    case GT: return LT;
+    case GE: return LE;
+    case CC: return HI;
+    case LS: return CS;
+    case HI: return CC;
+    case CS: return LS;
+    default:
+      UNIMPLEMENTED();
+      return EQ;
+  }
 }
 
 
 bool FlowGraphCompiler::EvaluateCondition(Condition condition,

[zra, 2013/05/24 17:11:41]

I couldn't find where this function is called. It doesn't look like the versions
for the other architectures are used either.

[regis, 2013/05/24 18:52:09]

Done.

                                           intptr_t left,
                                           intptr_t right) {
-  UNIMPLEMENTED();
-  return false;
+  const uintptr_t unsigned_left = static_cast<uintptr_t>(left);
+  const uintptr_t unsigned_right = static_cast<uintptr_t>(right);
+  switch (condition) {
+    case EQ: return left == right;
+    case NE: return left != right;
+    case LT: return left < right;
+    case LE: return left <= right;
+    case GT: return left > right;
+    case GE: return left >= right;
+    case CC: return unsigned_left < unsigned_right;
+    case LS: return unsigned_left <= unsigned_right;
+    case HI: return unsigned_left > unsigned_right;
+    case CS: return unsigned_left >= unsigned_right;
+    default:
+      UNIMPLEMENTED();
+      return false;
+  }
 }