[vm/compiler] Improve range analysis over MOD

Rationale:
Improves range analysis over MOD with the goal
of removing more bounds checks in a[i % length]
array accesses.

#37789

Change-Id: I1fb275d53a00400e2343628295fa010ac9b21786
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/112933
Commit-Queue: Aart Bik <ajcbik@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>

runtime/vm/compiler/backend/bce_test.cc

@@ -33,7 +33,8 @@ static intptr_t CountBoundChecks(FlowGraph* flow_graph) {
 
 // Helper method to build CFG, run BCE, and count number of
 // before/after bounds checks.
-static std::pair<intptr_t, intptr_t> ApplyBCE(const char* script_chars) {
+static std::pair<intptr_t, intptr_t> ApplyBCE(const char* script_chars,
+                                              CompilerPass::PipelineMode mode) {
   // Load the script and exercise the code once
   // while exercising the given compiler passes.
   const auto& root_library = Library::Handle(LoadTestScript(script_chars));
@@ -47,13 +48,12 @@ static std::pair<intptr_t, intptr_t> ApplyBCE(const char* script_chars) {
       CompilerPass::kApplyICData,
       CompilerPass::kSelectRepresentations,
       CompilerPass::kCanonicalize,
+      CompilerPass::kConstantPropagation,
       CompilerPass::kCSE,
       CompilerPass::kLICM,
   };
   const auto& function = Function::Handle(GetFunction(root_library, "foo"));
-  // TODO(ajcbik): test CompilerPass::kAOT too, but how to deal
-  //               with FLAG_precompiled_mode and DART_PRECOMPILER?
-  TestPipeline pipeline(function, CompilerPass::kJIT);
+  TestPipeline pipeline(function, mode);
   FlowGraph* flow_graph = pipeline.RunPasses(passes);
   // Count the number of before/after bounds checks.
   const intptr_t num_bc_before = CountBoundChecks(flow_graph);
@@ -63,6 +63,14 @@ static std::pair<intptr_t, intptr_t> ApplyBCE(const char* script_chars) {
   return {num_bc_before, num_bc_after};
 }
 
+static void TestScriptJIT(const char* script_chars,
+                          intptr_t expected_before,
+                          intptr_t expected_after) {
+  auto jit_result = ApplyBCE(script_chars, CompilerPass::kJIT);
+  EXPECT_STREQ(expected_before, jit_result.first);
+  EXPECT_STREQ(expected_after, jit_result.second);
+}
+
 //
 // BCE (bounds-check-elimination) tests.
 //
@@ -79,9 +87,7 @@ ISOLATE_UNIT_TEST_CASE(BCECannotRemove) {
         foo(l);
       }
     )";
-  auto result = ApplyBCE(kScriptChars);
-  EXPECT_STREQ(1, result.first);
-  EXPECT_STREQ(1, result.second);
+  TestScriptJIT(kScriptChars, 1, 1);
 }
 
 ISOLATE_UNIT_TEST_CASE(BCERemoveOne) {
@@ -96,9 +102,7 @@ ISOLATE_UNIT_TEST_CASE(BCERemoveOne) {
         foo(l);
       }
     )";
-  auto result = ApplyBCE(kScriptChars);
-  EXPECT_STREQ(2, result.first);
-  EXPECT_STREQ(1, result.second);
+  TestScriptJIT(kScriptChars, 2, 1);
 }
 
 ISOLATE_UNIT_TEST_CASE(BCESimpleLoop) {
@@ -115,9 +119,21 @@ ISOLATE_UNIT_TEST_CASE(BCESimpleLoop) {
         foo(l);
       }
     )";
-  auto result = ApplyBCE(kScriptChars);
-  EXPECT_STREQ(1, result.first);
-  EXPECT_STREQ(0, result.second);
+  TestScriptJIT(kScriptChars, 1, 0);
+}
+
+ISOLATE_UNIT_TEST_CASE(BCEModulo) {
+  const char* kScriptChars =
+      R"(
+      foo(int i) {
+        var l = new List<int>(3);
+        return l[i % 3] ?? l[i % (-3)];
+      }
+      main() {
+        foo(0);
+      }
+    )";
+  TestScriptJIT(kScriptChars, 2, 0);
 }
 
 // TODO(ajcbik): add more tests

runtime/vm/compiler/backend/range_analysis.cc

@@ -2331,6 +2331,26 @@ void Range::TruncDiv(const Range* left_range,
   *result_max = RangeBoundary::PositiveInfinity();
 }
 
+void Range::Mod(const Range* right_range,
+                RangeBoundary* result_min,
+                RangeBoundary* result_max) {
+  ASSERT(right_range != nullptr);
+  ASSERT(result_min != nullptr);
+  ASSERT(result_max != nullptr);
+  // Each modulo result is positive and bounded by one less than
+  // the maximum of the right-hand-side (it is unlikely that the
+  // left-hand-side further refines this in typical programs).
+  // Note that x % MinInt can be MaxInt and x % 0 always throws.
+  const int64_t kModMin = 0;
+  int64_t mod_max = kMaxInt64;
+  if (Range::ConstantMin(right_range).ConstantValue() != kMinInt64) {
+    const int64_t right_max = ConstantAbsMax(right_range);
+    mod_max = Utils::Maximum(right_max - 1, kModMin);
+  }
+  *result_min = RangeBoundary::FromConstant(kModMin);
+  *result_max = RangeBoundary::FromConstant(mod_max);
+}
+
 // Both the a and b ranges are >= 0.
 bool Range::OnlyPositiveOrZero(const Range& a, const Range& b) {
   return a.OnlyGreaterThanOrEqualTo(0) && b.OnlyGreaterThanOrEqualTo(0);
@@ -2398,6 +2418,10 @@ void Range::BinaryOp(const Token::Kind op,
       Range::TruncDiv(left_range, right_range, &min, &max);
       break;
 
+    case Token::kMOD:
+      Range::Mod(right_range, &min, &max);
+      break;
+
     case Token::kSHL:
       Range::Shl(left_range, right_range, &min, &max);
       break;

runtime/vm/compiler/backend/range_analysis.h

@@ -450,6 +450,10 @@ class Range : public ZoneAllocated {
                        RangeBoundary* min,
                        RangeBoundary* max);
 
+  static void Mod(const Range* right_range,
+                  RangeBoundary* min,
+                  RangeBoundary* max);
+
   static void Shr(const Range* left_range,
                   const Range* right_range,
                   RangeBoundary* min,