[turbofan] Consume feedback types for NumberMax and NumberMin.

src/compiler/simplified-lowering.cc

 // Copyright 2014 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.
 
 #include "src/compiler/simplified-lowering.h"
 
 #include <limits>
 
 #include "src/address-map.h"
 #include "src/base/bits.h"
@@ skipping 2043 matching lines @@
         if (lower()) NodeProperties::ChangeOp(node, Uint32Op(node));
         return;
       }
       case IrOpcode::kNumberFround: {
         VisitUnop(node, UseInfo::TruncatingFloat64(),
                   MachineRepresentation::kFloat32);
         if (lower()) NodeProperties::ChangeOp(node, Float64Op(node));
         return;
       }
       case IrOpcode::kNumberMax: {
-        // TODO(turbofan): We should consider feedback types here as well.
         if (BothInputsAreUnsigned32(node)) {
           VisitWord32TruncatingBinop(node);
           if (lower()) {
             lowering->DoMax(node, lowering->machine()->Uint32LessThan(),
                             MachineRepresentation::kWord32);
           }
         } else if (BothInputsAreSigned32(node)) {
           VisitWord32TruncatingBinop(node);
           if (lower()) {
             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 {
+          // 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));
           }
-        } 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()) {
             lowering->DoMin(node, lowering->machine()->Uint32LessThan(),
                             MachineRepresentation::kWord32);
           }
         } else if (BothInputsAreSigned32(node)) {
           VisitWord32TruncatingBinop(node);
           if (lower()) {
             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 {
+          // 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));
           }
-        } else {
-          VisitFloat64Binop(node);
-          if (lower()) NodeProperties::ChangeOp(node, Float64Op(node));
         }
         return;
       }
       case IrOpcode::kNumberAtan2:
       case IrOpcode::kNumberPow: {
         VisitBinop(node, UseInfo::TruncatingFloat64(),
                    MachineRepresentation::kFloat64);
         if (lower()) NodeProperties::ChangeOp(node, Float64Op(node));
         return;
       }
@@ skipping 1448 matching lines @@
         isolate(), graph()->zone(), callable.descriptor(), 0, flags,
         Operator::kNoProperties);
     to_number_operator_.set(common()->Call(desc));
   }
   return to_number_operator_.get();
 }
 
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8