[turbofan] Fix NumberMin and NumberMax typings

For some input types containing -0 but not +0, the result type of
NumberMin and NumberMax would unnecessarily include +0. However, for
some larger inputs, the result type would not include the spurious +0,
thus breaking monotonicity.

The CL fixes this and addresses a TODO as well.

Bug: chromium:1063661
Change-Id: Icd56d6102fbea12a2d96aa063a803b1052c714b8
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2116199
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66854}

src/compiler/operation-typer.cc

@@ -1040,23 +1040,31 @@ Type OperationTyper::NumberMax(Type lhs, Type rhs) {
   if (lhs.Maybe(Type::NaN()) || rhs.Maybe(Type::NaN())) {
     type = Type::Union(type, Type::NaN(), zone());
   }
-  lhs = Type::Intersect(lhs, Type::OrderedNumber(), zone());
-  DCHECK(!lhs.IsNone());
-  rhs = Type::Intersect(rhs, Type::OrderedNumber(), zone());
-  DCHECK(!rhs.IsNone());
-  if (lhs.Is(cache_->kIntegerOrMinusZero) &&
-      rhs.Is(cache_->kIntegerOrMinusZero)) {
-    // TODO(turbofan): This could still be improved in ruling out -0 when
-    // one of the inputs' min is 0.
-    double max = std::max(lhs.Max(), rhs.Max());
-    double min = std::max(lhs.Min(), rhs.Min());
+
+  if (!lhs.Is(cache_->kIntegerOrMinusZeroOrNaN) ||
+      !rhs.Is(cache_->kIntegerOrMinusZeroOrNaN)) {
+    return Type::Union(type, Type::Union(lhs, rhs, zone()), zone());
+  }
+
+  bool const lhs_maybe_minus_zero = lhs.Maybe(Type::MinusZero());
+  bool const rhs_maybe_minus_zero = rhs.Maybe(Type::MinusZero());
+  lhs = Type::Intersect(lhs, cache_->kInteger, zone());
+  rhs = Type::Intersect(rhs, cache_->kInteger, zone());
+
+  bool maybe_minus_zero = lhs_maybe_minus_zero || rhs_maybe_minus_zero;
+  if (!lhs.IsNone() || !rhs.IsNone()) {
+    double min = std::max(lhs.IsNone() ? -V8_INFINITY : lhs.Min(),
+                          rhs.IsNone() ? -V8_INFINITY : rhs.Min());
+    double max = std::max(lhs.IsNone() ? -V8_INFINITY : lhs.Max(),
+                          rhs.IsNone() ? -V8_INFINITY : rhs.Max());
     type = Type::Union(type, Type::Range(min, max, zone()), zone());
-    if (min <= 0.0 && 0.0 <= max &&
-        (lhs.Maybe(Type::MinusZero()) || rhs.Maybe(Type::MinusZero()))) {
-      type = Type::Union(type, Type::MinusZero(), zone());
-    }
-  } else {
-    type = Type::Union(type, Type::Union(lhs, rhs, zone()), zone());
+    maybe_minus_zero =
+        maybe_minus_zero && (min < 0.0 || (min == 0.0 && lhs_maybe_minus_zero &&
+                                           rhs_maybe_minus_zero));
+  }
+
+  if (maybe_minus_zero) {
+    type = Type::Union(type, Type::MinusZero(), zone());
   }
   return type;
 }
@@ -1072,21 +1080,28 @@ Type OperationTyper::NumberMin(Type lhs, Type rhs) {
   if (lhs.Maybe(Type::NaN()) || rhs.Maybe(Type::NaN())) {
     type = Type::Union(type, Type::NaN(), zone());
   }
-  lhs = Type::Intersect(lhs, Type::OrderedNumber(), zone());
-  DCHECK(!lhs.IsNone());
-  rhs = Type::Intersect(rhs, Type::OrderedNumber(), zone());
-  DCHECK(!rhs.IsNone());
-  if (lhs.Is(cache_->kIntegerOrMinusZero) &&
-      rhs.Is(cache_->kIntegerOrMinusZero)) {
-    double max = std::min(lhs.Max(), rhs.Max());
-    double min = std::min(lhs.Min(), rhs.Min());
+
+  if (!lhs.Is(cache_->kIntegerOrMinusZeroOrNaN) ||
+      !rhs.Is(cache_->kIntegerOrMinusZeroOrNaN)) {
+    return Type::Union(type, Type::Union(lhs, rhs, zone()), zone());
+  }
+
+  bool maybe_minus_zero =
+      lhs.Maybe(Type::MinusZero()) || rhs.Maybe(Type::MinusZero());
+  lhs = Type::Intersect(lhs, cache_->kInteger, zone());
+  rhs = Type::Intersect(rhs, cache_->kInteger, zone());
+
+  if (!lhs.IsNone() || !rhs.IsNone()) {
+    double min = std::min(lhs.IsNone() ? +V8_INFINITY : lhs.Min(),
+                          rhs.IsNone() ? +V8_INFINITY : rhs.Min());
+    double max = std::min(lhs.IsNone() ? +V8_INFINITY : lhs.Max(),
+                          rhs.IsNone() ? +V8_INFINITY : rhs.Max());
     type = Type::Union(type, Type::Range(min, max, zone()), zone());
-    if (min <= 0.0 && 0.0 <= max &&
-        (lhs.Maybe(Type::MinusZero()) || rhs.Maybe(Type::MinusZero()))) {
-      type = Type::Union(type, Type::MinusZero(), zone());
-    }
-  } else {
-    type = Type::Union(type, Type::Union(lhs, rhs, zone()), zone());
+    maybe_minus_zero = maybe_minus_zero && max >= 0.0;
+  }
+
+  if (maybe_minus_zero) {
+    type = Type::Union(type, Type::MinusZero(), zone());
   }
   return type;
 }

test/mjsunit/compiler/regress-1063661.js

+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --interrupt-budget=1024
+
+function main() {
+  const v1 = [];
+  for (let v11 = 0; v11 < 7; v11++) {
+    for (let v16 = 0; v16 != 100; v16++) {}
+    for (let v18 = -0.0; v18 < 7; v18 = v18 || 13.37) {
+      const v21 = Math.max(-339,v18);
+      v1.fill();
+      undefined % v21;
+    }
+  }
+}
+main();

test/mjsunit/mjsunit.status

@@ -1149,4 +1149,12 @@
   'wasm/many-modules': [SKIP],
 }],  # variant == stress_js_bg_compile_wasm_code_gc
 
+##############################################################################
+['variant == assert_types', {
+  # Type assertions can lead to differences in representation selection,
+  # which in turn can lead to different deopt behavior.
+  'compiler/number-abs': [SKIP],
+  'compiler/number-toboolean': [SKIP],
+}],  # variant == assert_types
+
 ]

test/unittests/compiler/typer-unittest.cc

@@ -585,6 +585,98 @@ SIMPLIFIED_NUMBER_BINOP_LIST(TEST_MONOTONICITY)
 SIMPLIFIED_SPECULATIVE_NUMBER_BINOP_LIST(TEST_MONOTONICITY)
 #undef TEST_MONOTONICITY
 
+TEST_F(TyperTest, Manual_Operation_NumberMax) {
+  BinaryTyper t = [&](Type type1, Type type2) {
+    return operation_typer_.NumberMax(type1, type2);
+  };
+
+  Type zero = Type::Constant(0, zone());
+  Type zero_or_minuszero = Type::Union(zero, Type::MinusZero(), zone());
+  Type dot_five = Type::Constant(0.5, zone());
+
+  Type a = t(Type::MinusZero(), Type::MinusZero());
+  CHECK(Type::MinusZero().Is(a));
+
+  Type b = t(Type::MinusZero(), zero_or_minuszero);
+  CHECK(Type::MinusZero().Is(b));
+  CHECK(zero.Is(b));
+  CHECK(a.Is(b));
+
+  Type c = t(zero_or_minuszero, Type::MinusZero());
+  CHECK(Type::MinusZero().Is(c));
+  CHECK(zero.Is(c));
+  CHECK(a.Is(c));
+
+  Type d = t(zero_or_minuszero, zero_or_minuszero);
+  CHECK(Type::MinusZero().Is(d));
+  CHECK(zero.Is(d));
+  CHECK(b.Is(d));
+  CHECK(c.Is(d));
+
+  Type e =
+      t(Type::MinusZero(), Type::Union(Type::MinusZero(), dot_five, zone()));
+  CHECK(Type::MinusZero().Is(e));
+  CHECK(dot_five.Is(e));
+  CHECK(a.Is(e));
+
+  Type f = t(Type::MinusZero(), zero);
+  CHECK(zero.Is(f));
+  CHECK(f.Is(b));
+  CHECK(f.Is(zero));  // Checks precision, not soundness.
+
+  Type g = t(zero, Type::MinusZero());
+  CHECK(zero.Is(g));
+  CHECK(g.Is(c));
+  CHECK(g.Is(zero));  // Checks precision, not soundness.
+}
+
+TEST_F(TyperTest, Manual_Operation_NumberMin) {
+  BinaryTyper t = [&](Type type1, Type type2) {
+    return operation_typer_.NumberMin(type1, type2);
+  };
+
+  Type zero = Type::Constant(0, zone());
+  Type zero_or_minuszero = Type::Union(zero, Type::MinusZero(), zone());
+  Type one = Type::Constant(1, zone());
+  Type minus_dot_five = Type::Constant(-0.5, zone());
+
+  Type a = t(Type::MinusZero(), Type::MinusZero());
+  CHECK(Type::MinusZero().Is(a));
+
+  Type b = t(Type::MinusZero(), zero_or_minuszero);
+  CHECK(Type::MinusZero().Is(b));
+  CHECK(zero.Is(b));
+  CHECK(a.Is(b));
+
+  Type c = t(zero_or_minuszero, Type::MinusZero());
+  CHECK(Type::MinusZero().Is(c));
+  CHECK(zero.Is(c));
+  CHECK(a.Is(c));
+
+  Type d = t(zero_or_minuszero, zero_or_minuszero);
+  CHECK(Type::MinusZero().Is(d));
+  CHECK(zero.Is(d));
+  CHECK(b.Is(d));
+  CHECK(c.Is(d));
+
+  Type e = t(Type::MinusZero(),
+             Type::Union(Type::MinusZero(), minus_dot_five, zone()));
+  CHECK(Type::MinusZero().Is(e));
+  CHECK(minus_dot_five.Is(e));
+  CHECK(a.Is(e));
+
+  Type f = t(Type::MinusZero(), zero);
+  CHECK(Type::MinusZero().Is(f));
+  CHECK(f.Is(b));
+
+  Type g = t(zero, Type::MinusZero());
+  CHECK(Type::MinusZero().Is(g));
+  CHECK(g.Is(c));
+
+  Type h = t(one, Type::MinusZero());
+  CHECK(Type::MinusZero().Is(h));
+}
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8