DBC: Various instructions.

runtime/vm/intermediate_language_dbc.cc

 // Copyright (c) 2016, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/globals.h"  // Needed here to get TARGET_ARCH_DBC.
 #if defined(TARGET_ARCH_DBC)
 
 #include "vm/intermediate_language.h"
 
 #include "vm/cpu.h"
@@ skipping 83 matching lines @@
   M(Float64x2OneArg)                                                           \
   M(ExtractNthOutput)                                                          \
   M(BinaryUint32Op)                                                            \
   M(ShiftUint32Op)                                                             \
   M(UnaryUint32Op)                                                             \
   M(UnboxedIntConverter)                                                       \
   M(GrowRegExpStack)                                                           \
   M(BoxInteger32)                                                              \
   M(UnboxInteger32)                                                            \
   M(CheckedSmiOp)                                                              \
-  M(CheckArrayBound)                                                           \
-  M(RelationalOp)                                                              \
-  M(EqualityCompare)                                                           \
-  M(LoadIndexed)
 
 // Location summaries actually are not used by the unoptimizing DBC compiler
 // because we don't allocate any registers.
 static LocationSummary* CreateLocationSummary(
     Zone* zone,
     intptr_t num_inputs,
     Location output = Location::NoLocation(),
     LocationSummary::ContainsCall contains_call = LocationSummary::kNoCall) {
   const intptr_t kNumTemps = 0;
   LocationSummary* locs = new(zone) LocationSummary(
@@ skipping 44 matching lines @@
 
 #define DEFINE_UNIMPLEMENTED(Name)                                             \
   DEFINE_UNIMPLEMENTED_MAKE_LOCATION_SUMMARY(Name)                             \
   DEFINE_UNIMPLEMENTED_EMIT_NATIVE_CODE(Name)                                  \
 
 FOR_EACH_UNIMPLEMENTED_INSTRUCTION(DEFINE_UNIMPLEMENTED)
 
 #undef DEFINE_UNIMPLEMENTED
 
 DEFINE_UNIMPLEMENTED_EMIT_BRANCH_CODE(TestCids)
-DEFINE_UNIMPLEMENTED_EMIT_BRANCH_CODE(RelationalOp)
-DEFINE_UNIMPLEMENTED_EMIT_BRANCH_CODE(EqualityCompare)
 
 
 EMIT_NATIVE_CODE(InstanceOf, 2, Location::SameAsFirstInput(),
                  LocationSummary::kCall) {
   SubtypeTestCache& test_cache = SubtypeTestCache::Handle();
   if (!type().IsVoidType() && type().IsInstantiated()) {
     test_cache = SubtypeTestCache::New();
   }
 
   if (compiler->is_optimizing()) {
@@ skipping 391 matching lines @@
     __ StoreIndexed(locs()->in(kArrayPos).reg(),
                     locs()->in(kIndexPos).reg(),
                     locs()->in(kValuePos).reg());
   } else {
     ASSERT(class_id() == kArrayCid);
     __ StoreIndexedTOS();
   }
 }
 
 
+EMIT_NATIVE_CODE(LoadIndexed, 2, Location::RequiresRegister()) {
+  ASSERT(compiler->is_optimizing());
+  if (class_id() != kArrayCid) {
+#if defined(PRODUCT)
+    compiler->Bailout("LoadIndexed");
+#else  // defined(PRODUCT)
+    compiler->Bailout(ToCString());
+#endif  // defined(PRODUCT)
+  }
+  const Register array = locs()->in(0).reg();
+  const Register index = locs()->in(1).reg();
+  const Register result = locs()->out(0).reg();
+
+  __ LoadIndexed(result, array, index);
+}
+
+
 EMIT_NATIVE_CODE(StringInterpolate,
                  1, Location::RegisterLocation(0),
                  LocationSummary::kCall) {
   if (compiler->is_optimizing()) {
     __ Push(locs()->in(0).reg());
   }
   const intptr_t kArgumentCount = 1;
   const Array& arguments_descriptor = Array::Handle(
       ArgumentsDescriptor::New(kArgumentCount, Object::null_array()));
   __ PushConstant(CallFunction());
@@ skipping 47 matching lines @@
 EMIT_NATIVE_CODE(StringToCharCode,
                  1, Location::RequiresRegister(),
                  LocationSummary::kNoCall) {
   ASSERT(cid_ == kOneByteStringCid);
   const Register str = locs()->in(0).reg();
   const Register result = locs()->out(0).reg();  // Result char code is a smi.
   __ StringToCharCode(result, str);
 }
 
 
-
 EMIT_NATIVE_CODE(AllocateObject,
                  0, Location::RequiresRegister(),
                  LocationSummary::kCall) {
   if (ArgumentCount() == 1) {
     __ PushConstant(cls());
     __ AllocateT();
     compiler->AddCurrentDescriptor(RawPcDescriptors::kOther,
                                    Thread::kNoDeoptId,
                                    token_pos());
   } else {
@@ skipping 78 matching lines @@
   __ MoveSpecial(-exception_var().index()-1,
                  Simulator::kExceptionSpecialIndex);
   __ MoveSpecial(-stacktrace_var().index()-1,
                  Simulator::kStacktraceSpecialIndex);
   __ SetFrame(compiler->StackSize());
 }
 
 
 EMIT_NATIVE_CODE(Throw, 0, Location::NoLocation(), LocationSummary::kCall) {
   __ Throw(0);
-  compiler->RecordSafepoint(locs());
   compiler->AddCurrentDescriptor(RawPcDescriptors::kOther,
                                  deopt_id(),
                                  token_pos());
+  compiler->RecordAfterCall(this);
   __ Trap();
 }
 
 
 EMIT_NATIVE_CODE(ReThrow, 0, Location::NoLocation(), LocationSummary::kCall) {
   compiler->SetNeedsStacktrace(catch_try_index());
   __ Throw(1);
-  compiler->RecordSafepoint(locs());
   compiler->AddCurrentDescriptor(RawPcDescriptors::kOther,
                                  deopt_id(),
                                  token_pos());
+  compiler->RecordAfterCall(this);
   __ Trap();
 }
 
 EMIT_NATIVE_CODE(InstantiateType,
                  1, Location::RequiresRegister(),
                  LocationSummary::kCall) {
   if (compiler->is_optimizing()) {
     __ Push(locs()->in(0).reg());
   }
   __ InstantiateType(__ AddConstant(type()));
@@ skipping 380 matching lines @@
       break;
     }
     case Token::kBIT_NOT:
       __ BitNot(locs()->out(0).reg(), locs()->in(0).reg());
       break;
     default:
       UNREACHABLE();
   }
 }
 
+
+static Token::Kind FlipCondition(Token::Kind kind) {
+  switch (kind) {
+    case Token::kEQ: return Token::kNE;
+    case Token::kNE: return Token::kEQ;
+    case Token::kLT: return Token::kGTE;
+    case Token::kGT: return Token::kLTE;
+    case Token::kLTE: return Token::kGT;
+    case Token::kGTE: return Token::kLT;
+    default:
+      UNREACHABLE();
+      return Token::kNE;
+  }
+}
+
+
+static Bytecode::Opcode OpcodeForCondition(Token::Kind kind) {
+  switch (kind) {
+    case Token::kEQ: return Bytecode::kIfEqStrict;
+    case Token::kNE: return Bytecode::kIfNeStrict;
+    case Token::kLT: return Bytecode::kIfLt;
+    case Token::kGT: return Bytecode::kIfGt;
+    case Token::kLTE: return Bytecode::kIfLe;
+    case Token::kGTE: return Bytecode::kIfGe;
+    default:
+      UNREACHABLE();
+      return Bytecode::kTrap;
+  }
+}
+
+
+static Condition EmitSmiComparisonOp(FlowGraphCompiler* compiler,
+                                     LocationSummary* locs,
+                                     Token::Kind kind,
+                                     BranchLabels labels) {
+  const Register left = locs->in(0).reg();
+  const Register right = locs->in(1).reg();
+  Token::Kind comparison = kind;
+  Condition condition = NEXT_IS_TRUE;
+  if (labels.fall_through != labels.false_label) {

[Vyacheslav Egorov (Google), 2016/07/01 16:15:35]

I don't get how this works. If label.fall_through is equal to labels.false_label
that means NEXT_IS_FALSE.

[zra, 2016/07/01 17:43:44]

Added some comments here and to EmitBranchOnCondition.

+    // Flip comparision to save a jump.
+    condition = NEXT_IS_FALSE;
+    comparison = FlipCondition(kind);
+  }
+  __ Emit(Bytecode::Encode(OpcodeForCondition(comparison), left, right));
+  return condition;
+}
+
+
+Condition EqualityCompareInstr::EmitComparisonCode(FlowGraphCompiler* compiler,
+                                                   BranchLabels labels) {
+  if (operation_cid() == kSmiCid) {
+    return EmitSmiComparisonOp(compiler, locs(), kind(), labels);
+  } else {
+    ASSERT(operation_cid() == kDoubleCid);
+#if defined(PRODUCT)
+    compiler->Bailout("EqualityCompareInstr::EmitComparisonCode");
+#else  // defined(PRODUCT)
+    compiler->Bailout(ToCString());
+#endif  // defined(PRODUCT)
+    return NEXT_IS_FALSE;
+  }
+}
+
+
+EMIT_NATIVE_CODE(EqualityCompare, 2, Location::RequiresRegister()) {
+  ASSERT(compiler->is_optimizing());
+  ASSERT((kind() == Token::kEQ) || (kind() == Token::kNE));
+  Label is_true, is_false;
+  BranchLabels labels = { &is_true, &is_false, &is_false };

[Vyacheslav Egorov (Google), 2016/07/01 16:15:35]

This does not match the code below. This configuration of labels means that
falltrough is false case.

[zra, 2016/07/01 17:43:45]

The labels don't make sense here because we aren't jumping. They're just picked
so that EmitComparisonCode will return NEXT_IS_TRUE. I'll add a comment.

+  const Register result = locs()->out(0).reg();
+  __ LoadConstant(result, Bool::False());
+  Condition true_condition = EmitComparisonCode(compiler, labels);
+  ASSERT(true_condition == NEXT_IS_TRUE);
+  __ LoadConstant(result, Bool::True());
+}
+
+
+void EqualityCompareInstr::EmitBranchCode(FlowGraphCompiler* compiler,
+                                          BranchInstr* branch) {
+  ASSERT((kind() == Token::kNE) || (kind() == Token::kEQ));
+  BranchLabels labels = compiler->CreateBranchLabels(branch);
+  Condition true_condition = EmitComparisonCode(compiler, labels);
+  EmitBranchOnCondition(compiler, true_condition, labels);
+}
+
+
+Condition RelationalOpInstr::EmitComparisonCode(FlowGraphCompiler* compiler,
+                                                BranchLabels labels) {
+  if (operation_cid() == kSmiCid) {
+    return EmitSmiComparisonOp(compiler, locs(), kind(), labels);
+  } else {
+    ASSERT(operation_cid() == kDoubleCid);
+#if defined(PRODUCT)
+    compiler->Bailout("EqualityCompareInstr::EmitComparisonCode");
+#else  // defined(PRODUCT)
+    compiler->Bailout(ToCString());
+#endif  // defined(PRODUCT)
+    return NEXT_IS_FALSE;
+  }
+}
+
+
+EMIT_NATIVE_CODE(RelationalOp, 2, Location::RequiresRegister()) {
+  ASSERT(compiler->is_optimizing());
+  Label is_true, is_false;
+  BranchLabels labels = { &is_true, &is_false, &is_false };

[Vyacheslav Egorov (Google), 2016/07/01 16:15:35]

The same issue again. You are falling through to a true case while labels say
you are falling through to the false case.

[zra, 2016/07/01 17:43:45]

Acknowledged.

+  const Register result = locs()->out(0).reg();
+  __ LoadConstant(result, Bool::False());
+  Condition true_condition = EmitComparisonCode(compiler, labels);
+  ASSERT(true_condition == NEXT_IS_TRUE);
+  __ LoadConstant(result, Bool::True());
+}
+
+
+void RelationalOpInstr::EmitBranchCode(FlowGraphCompiler* compiler,
+                                       BranchInstr* branch) {
+  BranchLabels labels = compiler->CreateBranchLabels(branch);
+  Condition true_condition = EmitComparisonCode(compiler, labels);
+  EmitBranchOnCondition(compiler, true_condition, labels);
+}
+
+
+EMIT_NATIVE_CODE(CheckArrayBound, 2) {
+  const Register length = locs()->in(kLengthPos).reg();
+  const Register index = locs()->in(kIndexPos).reg();
+  __ IfLe(length, index);

[Vyacheslav Egorov (Google), 2016/07/01 16:15:35]

This is not correct. This does not check for negative index, code in other
back-ends is using *unsigned*.

[zra, 2016/07/01 17:43:45]

Do you have a preference whether we should have unsigned comparison
instructions, or add a flag to IfLe saying signed or unsigned?

[zra, 2016/07/01 20:55:39]

Added unsigned comparison instructions. We can revisit if we're running short on
instructions.

+  compiler->EmitDeopt(deopt_id(),
+                      ICData::kDeoptCheckArrayBound,
+                      (generalized_ ? ICData::kGeneralized : 0) |
+                      (licm_hoisted_ ? ICData::kHoisted : 0));
+}
+
 }  // namespace dart
 
 #endif  // defined TARGET_ARCH_DBC