[turbofan] Consume feedback types for NumberMax and NumberMin.

src/compiler/simplified-lowering.cc

Index: src/compiler/simplified-lowering.cc
diff --git a/src/compiler/simplified-lowering.cc b/src/compiler/simplified-lowering.cc
index 8d1be4d09eefec5dd2e226667925e417a0da0992..fd456f8af4df571f4a9f094313182caeb8cada9b 100644
--- a/src/compiler/simplified-lowering.cc
+++ b/src/compiler/simplified-lowering.cc
@@ -2061,7 +2061,6 @@ class RepresentationSelector {
         return;
       }
       case IrOpcode::kNumberMax: {
-        // TODO(turbofan): We should consider feedback types here as well.
         if (BothInputsAreUnsigned32(node)) {
           VisitWord32TruncatingBinop(node);
           if (lower()) {
@@ -2074,20 +2073,42 @@ class RepresentationSelector {
             lowering->DoMax(node, lowering->machine()->Int32LessThan(),
                             MachineRepresentation::kWord32);
           }
-        } else if (BothInputsAre(node, Type::PlainNumber())) {
-          VisitFloat64Binop(node);
-          if (lower()) {
-            lowering->DoMax(node, lowering->machine()->Float64LessThan(),
-                            MachineRepresentation::kFloat64);
-          }
         } else {
-          VisitFloat64Binop(node);
-          if (lower()) NodeProperties::ChangeOp(node, Float64Op(node));
+          // Below we use feedback types and thus we must always promise
+          // a consistent truncation, namely Truncation::Float64().
+          Type* const lhs_type = TypeOf(node->InputAt(0));
+          Type* const rhs_type = TypeOf(node->InputAt(1));
+          if (lhs_type->Is(Type::Unsigned32()) &&
+              rhs_type->Is(Type::Unsigned32())) {
+            VisitBinop(node, UseInfo::TruncatingFloat64AsWord32(),

[Jarin, 2017/03/07 23:00:51]

Hmm, now that I have been staring at this for longer, I believe we could
actually already look at the feedback type in the cases above (lines 2064, 2070)
because the feedback type can only get smaller, so this can only change the
truncation to a more specific one. Obviously, this reasoning would have to be
well described in the comments. What do you think?

[Benedikt Meurer, 2017/03/08 04:44:07]

Yes, I think it's sufficient to look at the feedback type. But we can just
always pass Float64 truncation, since the Word32 truncation doesn't change
anything if the upper bound is already Signed32 or Unsigned32.

I've changed it to always pass Float64 truncation now (we should really rename
this BTW).

+                       MachineRepresentation::kWord32);
+            if (lower()) {
+              lowering->DoMax(node, lowering->machine()->Uint32LessThan(),
+                              MachineRepresentation::kWord32);
+            }
+          } else if (lhs_type->Is(Type::Signed32()) &&
+                     rhs_type->Is(Type::Signed32())) {
+            VisitBinop(node, UseInfo::TruncatingFloat64AsWord32(),
+                       MachineRepresentation::kWord32);
+            if (lower()) {
+              lowering->DoMax(node, lowering->machine()->Int32LessThan(),
+                              MachineRepresentation::kWord32);
+            }
+          } else if (lhs_type->Is(Type::PlainNumber()) &&
+                     rhs_type->Is(Type::PlainNumber())) {
+            VisitFloat64Binop(node);
+            if (lower()) {
+              lowering->DoMax(node, lowering->machine()->Float64LessThan(),
+                              MachineRepresentation::kFloat64);
+            }
+          } else {
+            VisitFloat64Binop(node);
+            if (lower()) NodeProperties::ChangeOp(node, Float64Op(node));
+          }
         }
         return;
       }
       case IrOpcode::kNumberMin: {
-        // TODO(turbofan): We should consider feedback types here as well.
         if (BothInputsAreUnsigned32(node)) {
           VisitWord32TruncatingBinop(node);
           if (lower()) {
@@ -2100,15 +2121,38 @@ class RepresentationSelector {
             lowering->DoMin(node, lowering->machine()->Int32LessThan(),
                             MachineRepresentation::kWord32);
           }
-        } else if (BothInputsAre(node, Type::PlainNumber())) {
-          VisitFloat64Binop(node);
-          if (lower()) {
-            lowering->DoMin(node, lowering->machine()->Float64LessThan(),
-                            MachineRepresentation::kFloat64);
-          }
         } else {
-          VisitFloat64Binop(node);
-          if (lower()) NodeProperties::ChangeOp(node, Float64Op(node));
+          // Below we use feedback types and thus we must always promise
+          // a consistent truncation, namely Truncation::Float64().
+          Type* const lhs_type = TypeOf(node->InputAt(0));
+          Type* const rhs_type = TypeOf(node->InputAt(1));
+          if (lhs_type->Is(Type::Unsigned32()) &&
+              rhs_type->Is(Type::Unsigned32())) {
+            VisitBinop(node, UseInfo::TruncatingFloat64AsWord32(),
+                       MachineRepresentation::kWord32);
+            if (lower()) {
+              lowering->DoMin(node, lowering->machine()->Uint32LessThan(),
+                              MachineRepresentation::kWord32);
+            }
+          } else if (lhs_type->Is(Type::Signed32()) &&
+                     rhs_type->Is(Type::Signed32())) {
+            VisitBinop(node, UseInfo::TruncatingFloat64AsWord32(),
+                       MachineRepresentation::kWord32);
+            if (lower()) {
+              lowering->DoMin(node, lowering->machine()->Int32LessThan(),
+                              MachineRepresentation::kWord32);
+            }
+          } else if (lhs_type->Is(Type::PlainNumber()) &&
+                     rhs_type->Is(Type::PlainNumber())) {
+            VisitFloat64Binop(node);
+            if (lower()) {
+              lowering->DoMin(node, lowering->machine()->Float64LessThan(),
+                              MachineRepresentation::kFloat64);
+            }
+          } else {
+            VisitFloat64Binop(node);
+            if (lower()) NodeProperties::ChangeOp(node, Float64Op(node));
+          }
         }
         return;
       }