From e013aaa511fcd4cf1f593e801845d2c30220896e Mon Sep 17 00:00:00 2001 From: Florian Schneider Date: Fri, 7 Oct 2016 15:30:21 -0700 Subject: [PATCH] Fix DBC double comparisons for NaN. Because NaN is not handled in a separate branch (like on other architetures - e.g. branch-on-parity-flag on ia32), and every comparison with NaN returns false, except "!=" we can't flip the branch condition in the same way for double compares like we do for integer compares. We could re-order blocks to always make the false-successor the fall-through, but for now I just disable flipping the branch condition for double compares. BUG=#27481 R=zra@google.com Review URL: https://codereview.chromium.org/2400103002 . --- runtime/vm/intermediate_language_dbc.cc | 17 +++++++---------- tests/lib/lib.status | 3 --- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/runtime/vm/intermediate_language_dbc.cc b/runtime/vm/intermediate_language_dbc.cc index 49cc77ccacc..72085c04881 100644 --- a/runtime/vm/intermediate_language_dbc.cc +++ b/runtime/vm/intermediate_language_dbc.cc @@ -454,7 +454,7 @@ static void EmitBranchOnCondition(FlowGraphCompiler* compiler, } } 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); @@ -479,7 +479,7 @@ Condition StrictCompareInstr::EmitComparisonCode(FlowGraphCompiler* compiler, 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; @@ -1798,15 +1798,12 @@ static Condition EmitDoubleComparisonOp(FlowGraphCompiler* compiler, 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 + // 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; } diff --git a/tests/lib/lib.status b/tests/lib/lib.status index a8b3b58056d..6be0ee903c7 100644 --- a/tests/lib/lib.status +++ b/tests/lib/lib.status @@ -396,9 +396,6 @@ convert/utf85_test: Pass, Timeout # version. mirrors/accessor_cache_overflow_test: Skip -[ $arch == simdbc64 ] -math/double_pow_test: Fail # Issue #27481 - [ $hot_reload || $hot_reload_rollback ] async/multiple_timer_test: Pass, Fail # Timing related async/stream_transformer_test: Pass, Fail # Closure identity