[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 <sstrickl@google.com> Reviewed-by: Daco Harkes <dacoharkes@google.com> Reviewed-by: Martin Kustermann <kustermann@google.com>
2021-11-29 11:26:24 +00:00
parent bd4a27ff81
commit 6faa4b70c3
2 changed files with 92 additions and 3 deletions
@@ -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) {
@@ -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<BlockEntryInstr*> 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