[vm/compiler] Decide to optimize switch statements using static type of the tested value

TEST=runtime/tests/vm/dart/regress_52214_test.dart
Fixes https://github.com/dart-lang/sdk/issues/52214

Change-Id: Iaddd00e79ee814feda998f14750ec8980309ae74
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/301480
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>

runtime/tests/vm/dart/regress_52214_test.dart

@@ -0,0 +1,85 @@
+// Copyright (c) 2023, 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.
+
+// Verifies that switch statement with non-integer expression and
+// int case constants is handled correctly.
+// Regression test for https://github.com/dart-lang/sdk/issues/52214.
+
+import 'package:expect/expect.dart';
+
+String test1() {
+  num switcher = 3.0;
+  switch (switcher) {
+    case 3:
+      return 'in case(3)';
+    default:
+      return 'in default';
+  }
+}
+
+String test2(Object switcher) {
+  switch (switcher) {
+    case 3:
+      return 'in case(3)';
+    default:
+      return 'in default';
+  }
+}
+
+String test3(dynamic switcher) {
+  switch (switcher) {
+    case 3:
+      return 'in case(3)';
+    default:
+      return 'in default';
+  }
+}
+
+String test4(switcher) {
+  switch (switcher) {
+    case 1:
+      return 'in case(1)';
+    case 2:
+      return 'in case(2)';
+    case 3:
+      return 'in case(3)';
+    case 4:
+      return 'in case(4)';
+    case 5:
+      return 'in case(5)';
+    case 6:
+      return 'in case(6)';
+    case 7:
+      return 'in case(7)';
+    case 8:
+      return 'in case(8)';
+    case 9:
+      return 'in case(9)';
+    case 10:
+      return 'in case(10)';
+    case 11:
+      return 'in case(11)';
+    case 12:
+      return 'in case(12)';
+    case 13:
+      return 'in case(13)';
+    case 14:
+      return 'in case(14)';
+    case 15:
+      return 'in case(15)';
+    case 16:
+      return 'in case(16)';
+    default:
+      return 'in default';
+  }
+}
+
+void main() {
+  Expect.equals('in case(3)', test1());
+  Expect.equals('in case(3)', test2(3.0));
+  Expect.equals('in case(3)', test3(3.0));
+  Expect.equals('in case(5)', test4(5.0));
+  Expect.equals('in case(7)', test4(7.0));
+  Expect.equals('in default', test4(0.0));
+}

runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc

@@ -4875,14 +4875,18 @@ Fragment StreamingFlowGraphBuilder::BuildSwitchStatement(
   SwitchBlock block(flow_graph_builder_, case_count);
 
   Fragment instructions = BuildExpression();  // read condition.
-  SkipOptionalDartType();                     // skip expression type
+  const AbstractType* expression_type = &Object::dynamic_type();
+  if (ReadTag() == kSomething) {
+    expression_type = &T.BuildType();  // read expression type.
+  }
   instructions +=
       StoreLocal(TokenPosition::kNoSource, scopes()->switch_variable);
   instructions += Drop();
 
   case_count = ReadListLength();  // read number of cases.
 
-  SwitchHelper helper(Z, pos, is_exhaustive, &block, case_count);
+  SwitchHelper helper(Z, pos, is_exhaustive, *expression_type, &block,
+                      case_count);
 
   // Build the case bodies and collect the expressions into the helper
   // for the next step.
@@ -5076,8 +5080,7 @@ Fragment StreamingFlowGraphBuilder::BuildOptimizedSwitchPrelude(
   TargetEntryInstr* then_entry;
   TargetEntryInstr* otherwise_entry;
 
-  const AbstractType& expression_type =
-      AbstractType::ZoneHandle(Z, helper->expression_class().RareType());
+  const AbstractType& expression_type = helper->expression_type();
   ASSERT(dart::SimpleInstanceOfType(expression_type));
 
   Fragment instructions;

runtime/vm/compiler/frontend/kernel_to_il.cc

@@ -5119,6 +5119,36 @@ Fragment FlowGraphBuilder::BuildDoubleHashCode() {
   return body;
 }
 
+SwitchHelper::SwitchHelper(Zone* zone,
+                           TokenPosition position,
+                           bool is_exhaustive,
+                           const AbstractType& expression_type,
+                           SwitchBlock* switch_block,
+                           intptr_t case_count)
+    : zone_(zone),
+      position_(position),
+      is_exhaustive_(is_exhaustive),
+      expression_type_(expression_type),
+      switch_block_(switch_block),
+      case_count_(case_count),
+      case_bodies_(case_count),
+      case_expression_counts_(case_count),
+      expressions_(case_count),
+      sorted_expressions_(case_count) {
+  case_expression_counts_.FillWith(0, 0, case_count);
+
+  if (expression_type.nullability() == Nullability::kNonNullable) {
+    if (expression_type.IsIntType() || expression_type.IsSmiType()) {
+      is_optimizable_ = true;
+    } else if (expression_type.HasTypeClass() &&
+               Class::Handle(zone_, expression_type.type_class())
+                   .is_enum_class()) {
+      is_optimizable_ = true;
+      is_enum_switch_ = true;
+    }
+  }
+}
+
 int64_t SwitchHelper::ExpressionRange() const {
   const int64_t min = expression_min().AsInt64Value();
   const int64_t max = expression_max().AsInt64Value();
@@ -5162,7 +5192,7 @@ SwitchDispatch SwitchHelper::SelectDispatchStrategy() {
   // binary search to avoid code size explosion.
   const double kJumpTableMaxHolesRatio = 1.0;
 
-  if (!is_optimizable()) {
+  if (!is_optimizable() || expressions().is_empty()) {
     // The switch is not optimizable, so we can only use linear scan.
     return kSwitchDispatchLinearScan;
   }
@@ -5306,15 +5336,10 @@ void SwitchHelper::AddExpression(intptr_t case_index,
 
   expressions_.Add(SwitchExpression(case_index, position, value));
 
-  if (is_optimizable_ || expression_class_ == nullptr) {
-    // Check the type of the expression for use in an optimized switch.
-    const Class& value_class = Class::ZoneHandle(zone_, value.clazz());
-    if (expression_class_ == nullptr) {
-      expression_class_ = &value_class;
-      // Only integer and enum expressions can be used in an optimized switch.
-      is_optimizable_ = value.IsInteger() || value_class.is_enum_class();
-    } else if (value_class.ptr() != expression_class_->ptr()) {
-      // At least one expression has a different type than the others.
+  if (is_optimizable_) {
+    // Check the type of the case expression for use in an optimized switch.
+    if (!value.IsInstanceOf(expression_type_, Object::null_type_arguments(),
+                            Object::null_type_arguments())) {
       is_optimizable_ = false;
     }
   }

runtime/vm/compiler/frontend/kernel_to_il.h

@@ -1038,22 +1038,13 @@ class SwitchHelper {
   SwitchHelper(Zone* zone,
                TokenPosition position,
                bool is_exhaustive,
+               const AbstractType& expression_type,
                SwitchBlock* switch_block,
-               intptr_t case_count)
-      : zone_(zone),
-        position_(position),
-        is_exhaustive_(is_exhaustive),
-        switch_block_(switch_block),
-        case_count_(case_count),
-        case_bodies_(case_count),
-        case_expression_counts_(case_count),
-        expressions_(case_count),
-        sorted_expressions_(case_count) {
-    case_expression_counts_.FillWith(0, 0, case_count);
-  }
+               intptr_t case_count);
 
-  // A switch statement is optimizable if all expression are of the same type
-  // and have integer representations.
+  // A switch statement is optimizable if static type of the scrutinee
+  // expression is a non-nullable int or enum, and all case expressions
+  // are instances of the scrutinee static type.
   bool is_optimizable() const { return is_optimizable_; }
   const TokenPosition& position() const { return position_; }
   bool is_exhaustive() const { return is_exhaustive_; }
@@ -1082,11 +1073,8 @@ class SwitchHelper {
     return sorted_expressions_;
   }
 
-  // The common class of all expressions. The statement must be optimizable.
-  const Class& expression_class() const {
-    ASSERT(expression_class_ != nullptr);
-    return *expression_class_;
-  }
+  // Static type of the scrutinee expression.
+  const AbstractType& expression_type() const { return expression_type_; }
 
   const Integer& expression_min() const {
     ASSERT(expression_min_ != nullptr);
@@ -1099,7 +1087,7 @@ class SwitchHelper {
 
   bool has_default() const { return default_case_ >= 0; }
 
-  bool is_enum_switch() const { return expression_class().is_enum_class(); }
+  bool is_enum_switch() const { return is_enum_switch_; }
 
   // Returns size of [min..max] range, or kMaxInt64 on overflow.
   int64_t ExpressionRange() const;
@@ -1120,8 +1108,10 @@ class SwitchHelper {
 
   Zone* zone_;
   bool is_optimizable_ = false;
+  bool is_enum_switch_ = false;
   const TokenPosition position_;
   const bool is_exhaustive_;
+  const AbstractType& expression_type_;
   SwitchBlock* const switch_block_;
   const intptr_t case_count_;
   intptr_t default_case_ = -1;
@@ -1129,7 +1119,6 @@ class SwitchHelper {
   GrowableArray<intptr_t> case_expression_counts_;
   GrowableArray<SwitchExpression> expressions_;
   GrowableArray<SwitchExpression*> sorted_expressions_;
-  const Class* expression_class_ = nullptr;
   const Integer* expression_min_ = nullptr;
   const Integer* expression_max_ = nullptr;
 };