[vm, compiler] Implement MathMinMaxInstr with min/max when available.

TEST=vm/dart/min_max_test
Change-Id: I727a421d8c38261effdf3b09dbd4fd9330693f82
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/394800
Reviewed-by: Alexander Markov <[email protected]>
Commit-Queue: Ryan Macnak <[email protected]>

Author: rmacnak-google

runtime/tests/vm/dart/min_max_test.dart

@@ -0,0 +1,256 @@
+// Copyright (c) 2024, 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.
+// Dart test for testing Math.min and Math.max.
+
+// VMOptions=--optimization-counter-threshold=50 --no-background-compilation
+
+import "package:expect/expect.dart";
+import "dart:math" as math;
+
+var inf = double.infinity;
+var nan = double.nan;
+
+@pragma("vm:never-inline")
+int smi_min(int a, int b) => math.min(a, b);
+
+@pragma("vm:never-inline")
+int smi_max(int a, int b) => math.max(a, b);
+
+@pragma("vm:never-inline")
+int mint_min(int a, int b) => math.min(a, b);
+
+@pragma("vm:never-inline")
+int mint_max(int a, int b) => math.max(a, b);
+
+@pragma("vm:never-inline")
+double double_min(double a, double b) => math.min(a, b);
+
+@pragma("vm:never-inline")
+double double_max(double a, double b) => math.max(a, b);
+
+testSmiMin() {
+  Expect.equals(-2, smi_min(-2, -2));
+  Expect.equals(-2, smi_min(-1, -2));
+  Expect.equals(-2, smi_min(0, -2));
+  Expect.equals(-2, smi_min(1, -2));
+  Expect.equals(-2, smi_min(2, -2));
+
+  Expect.equals(-2, smi_min(-2, -1));
+  Expect.equals(-1, smi_min(-1, -1));
+  Expect.equals(-1, smi_min(0, -1));
+  Expect.equals(-1, smi_min(1, -1));
+  Expect.equals(-1, smi_min(2, -1));
+
+  Expect.equals(-2, smi_min(-2, 0));
+  Expect.equals(-1, smi_min(-1, 0));
+  Expect.equals(0, smi_min(0, 0));
+  Expect.equals(0, smi_min(1, 0));
+  Expect.equals(0, smi_min(2, 0));
+
+  Expect.equals(-2, smi_min(-2, 1));
+  Expect.equals(-1, smi_min(-1, 1));
+  Expect.equals(0, smi_min(0, 1));
+  Expect.equals(1, smi_min(1, 1));
+  Expect.equals(1, smi_min(2, 1));
+
+  Expect.equals(-2, smi_min(-2, 2));
+  Expect.equals(-1, smi_min(-1, 2));
+  Expect.equals(0, smi_min(0, 2));
+  Expect.equals(1, smi_min(1, 2));
+  Expect.equals(2, smi_min(2, 2));
+}
+
+testSmiMax() {
+  Expect.equals(-2, smi_max(-2, -2));
+  Expect.equals(-1, smi_max(-1, -2));
+  Expect.equals(0, smi_max(0, -2));
+  Expect.equals(1, smi_max(1, -2));
+  Expect.equals(2, smi_max(2, -2));
+
+  Expect.equals(-1, smi_max(-2, -1));
+  Expect.equals(-1, smi_max(-1, -1));
+  Expect.equals(0, smi_max(0, -1));
+  Expect.equals(1, smi_max(1, -1));
+  Expect.equals(2, smi_max(2, -1));
+
+  Expect.equals(0, smi_max(-2, 0));
+  Expect.equals(0, smi_max(-1, 0));
+  Expect.equals(0, smi_max(0, 0));
+  Expect.equals(1, smi_max(1, 0));
+  Expect.equals(2, smi_max(2, 0));
+
+  Expect.equals(1, smi_max(-2, 1));
+  Expect.equals(1, smi_max(-1, 1));
+  Expect.equals(1, smi_max(0, 1));
+  Expect.equals(1, smi_max(1, 1));
+  Expect.equals(2, smi_max(2, 1));
+
+  Expect.equals(2, smi_max(-2, 2));
+  Expect.equals(2, smi_max(-1, 2));
+  Expect.equals(2, smi_max(0, 2));
+  Expect.equals(2, smi_max(1, 2));
+  Expect.equals(2, smi_max(2, 2));
+}
+
+testMintMin() {
+  Expect.equals(0x8000000000000000, mint_min(0x8000000000000000, 0x8000000000000000));
+  Expect.equals(0x8000000000000000, mint_min(-1, 0x8000000000000000));
+  Expect.equals(0x8000000000000000, mint_min(0, 0x8000000000000000));
+  Expect.equals(0x8000000000000000, mint_min(1, 0x8000000000000000));
+  Expect.equals(0x8000000000000000, mint_min(0x7FFFFFFFFFFFFFFF, 0x8000000000000000));
+
+  Expect.equals(0x8000000000000000, mint_min(0x8000000000000000, -1));
+  Expect.equals(-1, mint_min(-1, -1));
+  Expect.equals(-1, mint_min(0, -1));
+  Expect.equals(-1, mint_min(1, -1));
+  Expect.equals(-1, mint_min(0x7FFFFFFFFFFFFFFF, -1));
+
+  Expect.equals(0x8000000000000000, mint_min(0x8000000000000000, 0));
+  Expect.equals(-1, mint_min(-1, 0));
+  Expect.equals(0, mint_min(0, 0));
+  Expect.equals(0, mint_min(1, 0));
+  Expect.equals(0, mint_min(0x7FFFFFFFFFFFFFFF, 0));
+
+  Expect.equals(0x8000000000000000, mint_min(0x8000000000000000, 1));
+  Expect.equals(-1, mint_min(-1, 1));
+  Expect.equals(0, mint_min(0, 1));
+  Expect.equals(1, mint_min(1, 1));
+  Expect.equals(1, mint_min(0x7FFFFFFFFFFFFFFF, 1));
+
+  Expect.equals(0x8000000000000000, mint_min(0x8000000000000000, 0x7FFFFFFFFFFFFFFF));
+  Expect.equals(-1, mint_min(-1, 0x7FFFFFFFFFFFFFFF));
+  Expect.equals(0, mint_min(0, 0x7FFFFFFFFFFFFFFF));
+  Expect.equals(1, mint_min(1, 0x7FFFFFFFFFFFFFFF));
+  Expect.equals(0x7FFFFFFFFFFFFFFF, mint_min(0x7FFFFFFFFFFFFFFF, 0x7FFFFFFFFFFFFFFF));
+}
+
+testMintMax() {
+  Expect.equals(0x8000000000000000, mint_max(0x8000000000000000, 0x8000000000000000));
+  Expect.equals(-1, mint_max(-1, 0x8000000000000000));
+  Expect.equals(0, mint_max(0, 0x8000000000000000));
+  Expect.equals(1, mint_max(1, 0x8000000000000000));
+  Expect.equals(0x7FFFFFFFFFFFFFFF, mint_max(0x7FFFFFFFFFFFFFFF, 0x8000000000000000));
+
+  Expect.equals(-1, mint_max(0x8000000000000000, -1));
+  Expect.equals(-1, mint_max(-1, -1));
+  Expect.equals(0, mint_max(0, -1));
+  Expect.equals(1, mint_max(1, -1));
+  Expect.equals(0x7FFFFFFFFFFFFFFF, mint_max(0x7FFFFFFFFFFFFFFF, -1));
+
+  Expect.equals(0, mint_max(0x8000000000000000, 0));
+  Expect.equals(0, mint_max(-1, 0));
+  Expect.equals(0, mint_max(0, 0));
+  Expect.equals(1, mint_max(1, 0));
+  Expect.equals(0x7FFFFFFFFFFFFFFF, mint_max(0x7FFFFFFFFFFFFFFF, 0));
+
+  Expect.equals(1, mint_max(0x8000000000000000, 1));
+  Expect.equals(1, mint_max(-1, 1));
+  Expect.equals(1, mint_max(0, 1));
+  Expect.equals(1, mint_max(1, 1));
+  Expect.equals(0x7FFFFFFFFFFFFFFF, mint_max(0x7FFFFFFFFFFFFFFF, 1));
+
+  Expect.equals(0x7FFFFFFFFFFFFFFF, mint_max(0x8000000000000000, 0x7FFFFFFFFFFFFFFF));
+  Expect.equals(0x7FFFFFFFFFFFFFFF, mint_max(-1, 0x7FFFFFFFFFFFFFFF));
+  Expect.equals(0x7FFFFFFFFFFFFFFF, mint_max(0, 0x7FFFFFFFFFFFFFFF));
+  Expect.equals(0x7FFFFFFFFFFFFFFF, mint_max(1, 0x7FFFFFFFFFFFFFFF));
+  Expect.equals(0x7FFFFFFFFFFFFFFF, mint_max(0x7FFFFFFFFFFFFFFF, 0x7FFFFFFFFFFFFFFF));
+}
+
+testDoubleMin() {
+  Expect.equals(-inf, double_min(-inf, -inf));
+  Expect.equals(-inf, double_min(-1.0, -inf));
+  Expect.equals(-inf, double_min(0.0, -inf));
+  Expect.equals(-inf, double_min(1.0, -inf));
+  Expect.equals(-inf, double_min(inf, -inf));
+
+  Expect.equals(-inf, double_min(-inf, -1.0));
+  Expect.equals(-1.0, double_min(-1.0, -1.0));
+  Expect.equals(-1.0, double_min(0.0, -1.0));
+  Expect.equals(-1.0, double_min(1.0, -1.0));
+  Expect.equals(-1.0, double_min(inf, -1.0));
+
+  Expect.equals(-inf, double_min(-inf, 0.0));
+  Expect.equals(-1.0, double_min(-1.0, 0.0));
+  Expect.equals(0.0, double_min(0.0, 0.0));
+  Expect.equals(0.0, double_min(1.0, 0.0));
+  Expect.equals(0.0, double_min(inf, 0.0));
+
+  Expect.equals(-inf, double_min(-inf, 1.0));
+  Expect.equals(-1.0, double_min(-1.0, 1.0));
+  Expect.equals(0.0, double_min(0.0, 1.0));
+  Expect.equals(1.0, double_min(1.0, 1.0));
+  Expect.equals(1.0, double_min(inf, 1.0));
+
+  Expect.equals(-inf, double_min(-inf, inf));
+  Expect.equals(-1.0, double_min(-1.0, inf));
+  Expect.equals(0.0, double_min(0.0, inf));
+  Expect.equals(1.0, double_min(1.0, inf));
+  Expect.equals(inf, double_min(inf, inf));
+
+  Expect.identical(-0.0, double_min(0.0, -0.0));
+  Expect.identical(-0.0, double_min(-0.0, 0.0));
+
+  Expect.isTrue(double_min(nan, nan).isNaN);
+  Expect.isTrue(double_min(nan, -inf).isNaN);
+  Expect.isTrue(double_min(nan, 1.0).isNaN);
+  Expect.isTrue(double_min(nan, inf).isNaN);
+  Expect.isTrue(double_min(nan, nan).isNaN);
+  Expect.isTrue(double_min(-inf, nan).isNaN);
+  Expect.isTrue(double_min(1.0, nan).isNaN);
+  Expect.isTrue(double_min(inf, nan).isNaN);
+}
+
+testDoubleMax() {
+  Expect.equals(-inf, double_max(-inf, -inf));
+  Expect.equals(-1.0, double_max(-1.0, -inf));
+  Expect.equals(0.0, double_max(0.0, -inf));
+  Expect.equals(1.0, double_max(1.0, -inf));
+  Expect.equals(inf, double_max(inf, -inf));
+
+  Expect.equals(-1.0, double_max(-inf, -1.0));
+  Expect.equals(-1.0, double_max(-1.0, -1.0));
+  Expect.equals(0.0, double_max(0.0, -1.0));
+  Expect.equals(1.0, double_max(1.0, -1.0));
+  Expect.equals(inf, double_max(inf, -1.0));
+
+  Expect.equals(0.0, double_max(-inf, 0.0));
+  Expect.equals(0.0, double_max(-1.0, 0.0));
+  Expect.equals(0.0, double_max(0.0, 0.0));
+  Expect.equals(1.0, double_max(1.0, 0.0));
+  Expect.equals(inf, double_max(inf, 0.0));
+
+  Expect.equals(1.0, double_max(-inf, 1.0));
+  Expect.equals(1.0, double_max(-1.0, 1.0));
+  Expect.equals(1.0, double_max(0.0, 1.0));
+  Expect.equals(1.0, double_max(1.0, 1.0));
+  Expect.equals(inf, double_max(inf, 1.0));
+
+  Expect.equals(inf, double_max(-inf, inf));
+  Expect.equals(inf, double_max(-1.0, inf));
+  Expect.equals(inf, double_max(0.0, inf));
+  Expect.equals(inf, double_max(1.0, inf));
+  Expect.equals(inf, double_max(inf, inf));
+
+  Expect.identical(0.0, double_max(0.0, -0.0));
+  Expect.identical(0.0, double_max(-0.0, 0.0));
+
+  Expect.isTrue(double_max(nan, nan).isNaN);
+  Expect.isTrue(double_max(nan, -inf).isNaN);
+  Expect.isTrue(double_max(nan, 1.0).isNaN);
+  Expect.isTrue(double_max(nan, inf).isNaN);
+  Expect.isTrue(double_max(-inf, nan).isNaN);
+  Expect.isTrue(double_max(1.0, nan).isNaN);
+  Expect.isTrue(double_max(inf, nan).isNaN);
+}
+
+main() {
+  for (int i = 0; i < 100; i++) {
+    testSmiMin();
+    testSmiMax();
+    testMintMin();
+    testMintMax();
+    testDoubleMin();
+    testDoubleMax();
+  }
+}

runtime/vm/compiler/backend/il.cc

@@ -1103,7 +1103,16 @@ bool MathMinMaxInstr::AttributesEqual(const Instruction& other) const {
   auto const other_op = other.AsMathMinMax();
   ASSERT(other_op != nullptr);
   return (op_kind() == other_op->op_kind()) &&
-         (result_cid() == other_op->result_cid());
+         (representation() == other_op->representation());
+}
+
+Definition* MathMinMaxInstr::Canonicalize(FlowGraph* flow_graph) {
+  if (!HasUses()) return nullptr;
+  if (left()->definition()->OriginalDefinition() ==
+      right()->definition()->OriginalDefinition()) {
+    return left()->definition();
+  }
+  return this;
 }
 
 bool BinaryIntegerOpInstr::AttributesEqual(const Instruction& other) const {

runtime/vm/compiler/backend/il.h

@@ -8805,11 +8805,12 @@ class MathMinMaxInstr : public TemplateDefinition<2, NoThrow, Pure> {
                   Value* left_value,
                   Value* right_value,
                   intptr_t deopt_id,
-                  intptr_t result_cid)
+                  Representation representation)
       : TemplateDefinition(deopt_id),
         op_kind_(op_kind),
-        result_cid_(result_cid) {
-    ASSERT((result_cid == kSmiCid) || (result_cid == kDoubleCid));
+        representation_(representation) {
+    ASSERT((representation == kUnboxedInt64) ||
+           (representation == kUnboxedDouble));
     SetInputAt(0, left_value);
     SetInputAt(1, right_value);
   }
@@ -8819,24 +8820,11 @@ class MathMinMaxInstr : public TemplateDefinition<2, NoThrow, Pure> {
   Value* left() const { return inputs_[0]; }
   Value* right() const { return inputs_[1]; }
 
-  intptr_t result_cid() const { return result_cid_; }
-
   virtual bool ComputeCanDeoptimize() const { return false; }
 
-  virtual Representation representation() const {
-    if (result_cid() == kSmiCid) {
-      return kTagged;
-    }
-    ASSERT(result_cid() == kDoubleCid);
-    return kUnboxedDouble;
-  }
-
+  virtual Representation representation() const { return representation_; }
   virtual Representation RequiredInputRepresentation(intptr_t idx) const {
-    if (result_cid() == kSmiCid) {
-      return kTagged;
-    }
-    ASSERT(result_cid() == kDoubleCid);
-    return kUnboxedDouble;
+    return representation_;
   }
 
   virtual intptr_t DeoptimizationTarget() const {
@@ -8848,10 +8836,11 @@ class MathMinMaxInstr : public TemplateDefinition<2, NoThrow, Pure> {
   DECLARE_INSTRUCTION(MathMinMax)
   virtual CompileType ComputeType() const;
   virtual bool AttributesEqual(const Instruction& other) const;
+  virtual Definition* Canonicalize(FlowGraph* flow_graph);
 
 #define FIELD_LIST(F)                                                          \
   F(const MethodRecognizer::Kind, op_kind_)                                    \
-  F(const intptr_t, result_cid_)
+  F(const Representation, representation_)
 
   DECLARE_INSTRUCTION_SERIALIZABLE_FIELDS(MathMinMaxInstr,
                                           TemplateDefinition,

runtime/vm/compiler/backend/il_arm.cc

@@ -5279,7 +5279,7 @@ void CaseInsensitiveCompareInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
 
 LocationSummary* MathMinMaxInstr::MakeLocationSummary(Zone* zone,
                                                       bool opt) const {
-  if (result_cid() == kDoubleCid) {
+  if (representation() == kUnboxedDouble) {
     const intptr_t kNumInputs = 2;
     const intptr_t kNumTemps = 1;
     LocationSummary* summary = new (zone)
@@ -5291,23 +5291,15 @@ LocationSummary* MathMinMaxInstr::MakeLocationSummary(Zone* zone,
     summary->set_temp(0, Location::RequiresRegister());
     return summary;
   }
-  ASSERT(result_cid() == kSmiCid);
-  const intptr_t kNumInputs = 2;
-  const intptr_t kNumTemps = 0;
-  LocationSummary* summary = new (zone)
-      LocationSummary(zone, kNumInputs, kNumTemps, LocationSummary::kNoCall);
-  summary->set_in(0, Location::RequiresRegister());
-  summary->set_in(1, Location::RequiresRegister());
-  // Reuse the left register so that code can be made shorter.
-  summary->set_out(0, Location::SameAsFirstInput());
-  return summary;
+  UNREACHABLE();
+  return nullptr;
 }
 
 void MathMinMaxInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
   ASSERT((op_kind() == MethodRecognizer::kMathMin) ||
          (op_kind() == MethodRecognizer::kMathMax));
   const bool is_min = (op_kind() == MethodRecognizer::kMathMin);
-  if (result_cid() == kDoubleCid) {
+  if (representation() == kUnboxedDouble) {
     compiler::Label done, returns_nan, are_equal;
     const DRegister left = EvenDRegisterOf(locs()->in(0).fpu_reg());
     const DRegister right = EvenDRegisterOf(locs()->in(1).fpu_reg());
@@ -5348,17 +5340,7 @@ void MathMinMaxInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
     return;
   }
 
-  ASSERT(result_cid() == kSmiCid);
-  const Register left = locs()->in(0).reg();
-  const Register right = locs()->in(1).reg();
-  const Register result = locs()->out(0).reg();
-  __ cmp(left, compiler::Operand(right));
-  ASSERT(result == left);
-  if (is_min) {
-    __ mov(result, compiler::Operand(right), GT);
-  } else {
-    __ mov(result, compiler::Operand(right), LT);
-  }
+  UNREACHABLE();
 }
 
 LocationSummary* UnarySmiOpInstr::MakeLocationSummary(Zone* zone,