From 6faa4b70c377ee25c1e08d9abb8fb69b2ee756b3 Mon Sep 17 00:00:00 2001 From: Tess Strickland Date: Mon, 29 Nov 2021 11:26:24 +0000 Subject: [PATCH] [vm/compiler] Resolve sentinel comparisons when possible. For StrictCompare instructions where one side is the sentinel value and the other side has a CompileType that cannot be the sentinel, propagate a false (if EQ_STRICT) or true (if NE_STRICT) constant for that comparison. TEST=vm/cc/ConstantPropagator_StrictCompare Change-Id: I96e4bde1a02fa841987964491c5fff176fdf82a6 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/220769 Commit-Queue: Tess Strickland Reviewed-by: Daco Harkes Reviewed-by: Martin Kustermann --- .../compiler/backend/constant_propagator.cc | 14 +++- .../backend/constant_propagator_test.cc | 81 +++++++++++++++++++ 2 files changed, 92 insertions(+), 3 deletions(-) diff --git a/runtime/vm/compiler/backend/constant_propagator.cc b/runtime/vm/compiler/backend/constant_propagator.cc index f4f5424c40a..eed551adb5d 100644 --- a/runtime/vm/compiler/backend/constant_propagator.cc +++ b/runtime/vm/compiler/backend/constant_propagator.cc @@ -588,9 +588,17 @@ void ConstantPropagator::VisitStrictCompare(StrictCompareInstr* instr) { const Object& left = left_defn->constant_value(); const Object& right = right_defn->constant_value(); if (IsNonConstant(left) || IsNonConstant(right)) { - // TODO(vegorov): incorporate nullability information into the lattice. - if ((left.IsNull() && instr->right()->Type()->HasDecidableNullability()) || - (right.IsNull() && instr->left()->Type()->HasDecidableNullability())) { + if ((left.ptr() == Object::sentinel().ptr() && + !instr->right()->Type()->can_be_sentinel()) || + (right.ptr() == Object::sentinel().ptr() && + !instr->left()->Type()->can_be_sentinel())) { + // Handle provably false (EQ_STRICT) or true (NE_STRICT) sentinel checks. + SetValue(instr, Bool::Get(instr->kind() != Token::kEQ_STRICT)); + } else if ((left.IsNull() && + instr->right()->Type()->HasDecidableNullability()) || + (right.IsNull() && + instr->left()->Type()->HasDecidableNullability())) { + // TODO(vegorov): incorporate nullability information into the lattice. bool result = left.IsNull() ? instr->right()->Type()->IsNull() : instr->left()->Type()->IsNull(); if (instr->kind() == Token::kNE_STRICT) { diff --git a/runtime/vm/compiler/backend/constant_propagator_test.cc b/runtime/vm/compiler/backend/constant_propagator_test.cc index de148818bcc..1ee3b6d1717 100644 --- a/runtime/vm/compiler/backend/constant_propagator_test.cc +++ b/runtime/vm/compiler/backend/constant_propagator_test.cc @@ -295,4 +295,85 @@ ISOLATE_UNIT_TEST_CASE(ConstantPropagator_Regress35371) { } #endif +void StrictCompareSentinel(Thread* thread, + bool negate, + bool non_sentinel_on_left) { + const char* kScript = R"( + late final int x = 4; + )"; + Zone* const Z = Thread::Current()->zone(); + const auto& root_library = Library::CheckedHandle(Z, LoadTestScript(kScript)); + const auto& toplevel = Class::Handle(Z, root_library.toplevel_class()); + const auto& field_x = Field::Handle( + Z, toplevel.LookupStaticField(String::Handle(Z, String::New("x")))); + + using compiler::BlockBuilder; + CompilerState S(thread, /*is_aot=*/false, /*is_optimizing=*/true); + FlowGraphBuilderHelper H; + + auto b1 = H.flow_graph()->graph_entry()->normal_entry(); + + { + BlockBuilder builder(H.flow_graph(), b1); + auto v_load = builder.AddDefinition(new LoadStaticFieldInstr( + field_x, {}, + /*calls_initializer=*/true, S.GetNextDeoptId())); + auto v_sentinel = H.flow_graph()->GetConstant(Object::sentinel()); + Value* const left_value = + non_sentinel_on_left ? new Value(v_load) : new Value(v_sentinel); + Value* const right_value = + non_sentinel_on_left ? new Value(v_sentinel) : new Value(v_load); + auto v_compare = builder.AddDefinition(new StrictCompareInstr( + {}, negate ? Token::kNE_STRICT : Token::kEQ_STRICT, left_value, + right_value, + /*needs_number_check=*/false, S.GetNextDeoptId())); + builder.AddReturn(new Value(v_compare)); + } + + H.FinishGraph(); + + FlowGraphPrinter::PrintGraph("Before TypePropagator", H.flow_graph()); + FlowGraphTypePropagator::Propagate(H.flow_graph()); + FlowGraphPrinter::PrintGraph("After TypePropagator", H.flow_graph()); + GrowableArray ignored; + ConstantPropagator::Optimize(H.flow_graph()); + FlowGraphPrinter::PrintGraph("After ConstantPropagator", H.flow_graph()); + + ReturnInstr* ret = nullptr; + + ILMatcher cursor(H.flow_graph(), + H.flow_graph()->graph_entry()->normal_entry(), true); + RELEASE_ASSERT(cursor.TryMatch({ + kMatchAndMoveFunctionEntry, + kMatchAndMoveLoadStaticField, + // The StrictCompare instruction should be removed. + {kMatchReturn, &ret}, + })); + + EXPECT_PROPERTY(ret, it.value()->BindsToConstant()); + EXPECT_PROPERTY(&ret->value()->BoundConstant(), it.IsBool()); + EXPECT_PROPERTY(&ret->value()->BoundConstant(), + Bool::Cast(it).value() == negate); +} + +ISOLATE_UNIT_TEST_CASE(ConstantPropagator_StrictCompareEqualsSentinelLeft) { + StrictCompareSentinel(thread, /*negate=*/false, + /*non_sentinel_on_left=*/true); +} + +ISOLATE_UNIT_TEST_CASE(ConstantPropagator_StrictCompareEqualsSentinelRightt) { + StrictCompareSentinel(thread, /*negate=*/false, + /*non_sentinel_on_left=*/false); +} + +ISOLATE_UNIT_TEST_CASE(ConstantPropagator_StrictCompareNotEqualsSentinelLeft) { + StrictCompareSentinel(thread, /*negate=*/true, + /*non_sentinel_on_left=*/true); +} + +ISOLATE_UNIT_TEST_CASE(ConstantPropagator_StrictCompareNotEqualsSentinelRight) { + StrictCompareSentinel(thread, /*negate=*/true, + /*non_sentinel_on_left=*/false); +} + } // namespace dart