Fix DBC double comparisons for NaN.

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 436 matching lines @@
     // to be in the subsequent instruction, which it skips if the test fails.
     __ Jump(labels.true_label);
     if (labels.fall_through != labels.false_label) {
       // The preceeding Jump instruction will be skipped if the test fails.
       // If we aren't falling through to the false case, then we have to do
       // a Jump to it here.
       __ Jump(labels.false_label);
     }
   } else {
     ASSERT(true_condition == NEXT_IS_FALSE);
-    // NEXT_IS_FALSE indicates that the preceeing test has been flipped and
+    // NEXT_IS_FALSE indicates that the preceeding test has been flipped and
     // expects the false case to be in the subsequent instruction, which it
     // skips if the test succeeds.
     __ Jump(labels.false_label);
     if (labels.fall_through != labels.true_label) {
       // The preceeding Jump instruction will be skipped if the test succeeds.
       // If we aren't falling through to the true case, then we have to do
       // a Jump to it here.
       __ Jump(labels.true_label);
     }
   }
 }
 
 
 Condition StrictCompareInstr::EmitComparisonCode(FlowGraphCompiler* compiler,
                                                  BranchLabels labels) {
   ASSERT((kind() == Token::kNE_STRICT) ||
          (kind() == Token::kEQ_STRICT));
 
   Token::Kind comparison;
   Condition condition;
   if (labels.fall_through == labels.false_label) {
     condition = NEXT_IS_TRUE;
     comparison = kind();
   } else {
-    // Flip comparision to save a jump.
+    // Flip comparison to save a jump.
     condition = NEXT_IS_FALSE;
     comparison = (kind() == Token::kEQ_STRICT) ? Token::kNE_STRICT
                                                : Token::kEQ_STRICT;
   }
 
   if (!compiler->is_optimizing()) {
     const Bytecode::Opcode eq_op = needs_number_check() ?
         Bytecode::kIfEqStrictNumTOS : Bytecode::kIfEqStrictTOS;
     const Bytecode::Opcode ne_op = needs_number_check() ?
         Bytecode::kIfNeStrictNumTOS : Bytecode::kIfNeStrictTOS;
@@ skipping 1288 matching lines @@
 }
 
 
 static Condition EmitDoubleComparisonOp(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;
+  // For double comparisons we can't flip the condition like with smi

[zra, 2016/10/07 02:14:43]

Can we flip for all but == and/or != ?

[Florian Schneider, 2016/10/07 16:49:33]

As discussed offline, we could do inline assembly to handle the nan-case like in
other architectures. Or we could reorder blocks so that the fall-through is
always the false-label.

I'll do some performance measurements to figure out the impact of this change.

+  // comparisons because of NaN which will compare false for all except !=
+  // operations.
+  // TODO(fschneider): Change the block order instead in DBC so that the
+  // false block in always the fall-through block.
   Condition condition = NEXT_IS_TRUE;
-  if (labels.fall_through != labels.false_label) {
-    // If we aren't falling through to the false label, we can save a Jump
-    // instruction in the case that the true case is the fall through by
-    // flipping the sense of the test such that the instruction following the
-    // test is the Jump to the false label.
-    condition = NEXT_IS_FALSE;
-    comparison = FlipCondition(kind);
-  }
   __ Emit(Bytecode::Encode(OpcodeForDoubleCondition(comparison), left, right));
   return condition;
 }
 
 
 Condition EqualityCompareInstr::EmitComparisonCode(FlowGraphCompiler* compiler,
                                                    BranchLabels labels) {
   if (operation_cid() == kSmiCid) {
     return EmitSmiComparisonOp(compiler, locs(), kind(), labels);
   } else {
@@ skipping 73 matching lines @@
   __ IfULe(length, index);
   compiler->EmitDeopt(deopt_id(),
                       ICData::kDeoptCheckArrayBound,
                       (generalized_ ? ICData::kGeneralized : 0) |
                       (licm_hoisted_ ? ICData::kHoisted : 0));
 }
 
 }  // namespace dart
 
 #endif  // defined TARGET_ARCH_DBC