[turbofan] Remove phase ordering problem in JSToNumber lowering.

Previously we had to run SimplifiedLowering, ChangeLowering and
JSGenericLowering independently and exactly in this order to
achieve great performance for the common case of JSToNumber (i.e.
input is already a Smi or a HeapNumber). This phase ordering
problem already causes trouble with not being able to run the
generic lowering phase earlier, but also blocks proper plain
primitive ToNumber optimizations. So this properly integrates
JSToNumber into the truncation analysis and optimizes according
to the truncation (either Word32 or Float64).

R=jarin@chromium.org

Review URL: https://codereview.chromium.org/1901803002

Cr-Commit-Position: refs/heads/master@{#35643}

src/compiler/change-lowering.cc

@@ -31,6 +31,8 @@ Reduction ChangeLowering::Reduce(Node* node) {
       return ChangeFloat64ToTagged(node->InputAt(0), control);
     case IrOpcode::kChangeInt32ToTagged:
       return ChangeInt32ToTagged(node->InputAt(0), control);
+    case IrOpcode::kChangeSmiToInt32:
+      return ChangeSmiToInt32(node->InputAt(0));
     case IrOpcode::kChangeTaggedToFloat64:
       return ChangeTaggedToFloat64(node->InputAt(0), control);
     case IrOpcode::kChangeTaggedToInt32:
@@ -121,11 +123,10 @@ Node* ChangeLowering::ChangeInt32ToSmi(Node* value) {
 
 
 Node* ChangeLowering::ChangeSmiToFloat64(Node* value) {
-  return ChangeInt32ToFloat64(ChangeSmiToInt32(value));
+  return ChangeInt32ToFloat64(ChangeSmiToWord32(value));
 }
 
-
-Node* ChangeLowering::ChangeSmiToInt32(Node* value) {
+Node* ChangeLowering::ChangeSmiToWord32(Node* value) {
   value = graph()->NewNode(machine()->WordSar(), value, SmiShiftBitsConstant());
   if (machine()->Is64()) {
     value = graph()->NewNode(machine()->TruncateInt64ToInt32(), value);
@@ -277,11 +278,14 @@ Reduction ChangeLowering::ChangeInt32ToTagged(Node* value, Node* control) {
   return Replace(phi);
 }
 
+Reduction ChangeLowering::ChangeSmiToInt32(Node* value) {
+  return Replace(ChangeSmiToWord32(value));
+}
 
 Reduction ChangeLowering::ChangeTaggedToUI32(Node* value, Node* control,
                                              Signedness signedness) {
   if (NodeProperties::GetType(value)->Is(Type::TaggedSigned())) {
-    return Replace(ChangeSmiToInt32(value));
+    return ChangeSmiToInt32(value);
   }
 
   const Operator* op = (signedness == kSigned)
@@ -324,7 +328,7 @@ Reduction ChangeLowering::ChangeTaggedToUI32(Node* value, Node* control,
   }
 
   Node* if_smi = graph()->NewNode(common()->IfFalse(), branch);
-  Node* vfrom_smi = ChangeSmiToInt32(value);
+  Node* vfrom_smi = ChangeSmiToWord32(value);
 
   Node* merge = graph()->NewNode(common()->Merge(2), if_not_smi, if_smi);
   Node* phi = graph()->NewNode(common()->Phi(MachineRepresentation::kWord32, 2),
@@ -334,85 +338,7 @@ Reduction ChangeLowering::ChangeTaggedToUI32(Node* value, Node* control,
 }
 
 
-namespace {
-
-bool CanCover(Node* value, IrOpcode::Value opcode) {
-  if (value->opcode() != opcode) return false;
-  bool first = true;
-  for (Edge const edge : value->use_edges()) {
-    if (NodeProperties::IsControlEdge(edge)) continue;
-    if (NodeProperties::IsEffectEdge(edge)) continue;
-    DCHECK(NodeProperties::IsValueEdge(edge));
-    if (!first) return false;
-    first = false;
-  }
-  return true;
-}
-
-}  // namespace
-
-
 Reduction ChangeLowering::ChangeTaggedToFloat64(Node* value, Node* control) {
-  if (CanCover(value, IrOpcode::kJSToNumber)) {
-    // ChangeTaggedToFloat64(JSToNumber(x)) =>
-    //   if IsSmi(x) then ChangeSmiToFloat64(x)
-    //   else let y = JSToNumber(x) in
-    //     if IsSmi(y) then ChangeSmiToFloat64(y)
-    //     else LoadHeapNumberValue(y)
-    Node* const object = NodeProperties::GetValueInput(value, 0);
-    Node* const context = NodeProperties::GetContextInput(value);
-    Node* const frame_state = NodeProperties::GetFrameStateInput(value, 0);
-    Node* const effect = NodeProperties::GetEffectInput(value);
-    Node* const control = NodeProperties::GetControlInput(value);
-
-    const Operator* merge_op = common()->Merge(2);
-    const Operator* ephi_op = common()->EffectPhi(2);
-    const Operator* phi_op = common()->Phi(MachineRepresentation::kFloat64, 2);
-
-    Node* check1 = TestNotSmi(object);
-    Node* branch1 =
-        graph()->NewNode(common()->Branch(BranchHint::kFalse), check1, control);
-
-    Node* if_true1 = graph()->NewNode(common()->IfTrue(), branch1);
-    Node* vtrue1 = graph()->NewNode(value->op(), object, context, frame_state,
-                                    effect, if_true1);
-    Node* etrue1 = vtrue1;
-
-    Node* check2 = TestNotSmi(vtrue1);
-    Node* branch2 = graph()->NewNode(common()->Branch(), check2, if_true1);
-
-    Node* if_true2 = graph()->NewNode(common()->IfTrue(), branch2);
-    Node* vtrue2 = LoadHeapNumberValue(vtrue1, if_true2);
-
-    Node* if_false2 = graph()->NewNode(common()->IfFalse(), branch2);
-    Node* vfalse2 = ChangeSmiToFloat64(vtrue1);
-
-    if_true1 = graph()->NewNode(merge_op, if_true2, if_false2);
-    vtrue1 = graph()->NewNode(phi_op, vtrue2, vfalse2, if_true1);
-
-    Node* if_false1 = graph()->NewNode(common()->IfFalse(), branch1);
-    Node* vfalse1 = ChangeSmiToFloat64(object);
-    Node* efalse1 = effect;
-
-    Node* merge1 = graph()->NewNode(merge_op, if_true1, if_false1);
-    Node* ephi1 = graph()->NewNode(ephi_op, etrue1, efalse1, merge1);
-    Node* phi1 = graph()->NewNode(phi_op, vtrue1, vfalse1, merge1);
-
-    // Wire the new diamond into the graph, {JSToNumber} can still throw.
-    NodeProperties::ReplaceUses(value, phi1, ephi1, etrue1, etrue1);
-
-    // TODO(mstarzinger): This iteration cuts out the IfSuccess projection from
-    // the node and places it inside the diamond. Come up with a helper method!
-    for (Node* use : etrue1->uses()) {
-      if (use->opcode() == IrOpcode::kIfSuccess) {
-        use->ReplaceUses(merge1);
-        NodeProperties::ReplaceControlInput(branch2, use);
-      }
-    }
-
-    return Replace(phi1);
-  }
-
   Node* check = TestNotSmi(value);
   Node* branch =
       graph()->NewNode(common()->Branch(BranchHint::kFalse), check, control);

src/compiler/change-lowering.h

@@ -35,7 +35,7 @@ class ChangeLowering final : public Reducer {
   Node* ChangeInt32ToFloat64(Node* value);
   Node* ChangeInt32ToSmi(Node* value);
   Node* ChangeSmiToFloat64(Node* value);
-  Node* ChangeSmiToInt32(Node* value);
+  Node* ChangeSmiToWord32(Node* value);
   Node* ChangeUint32ToFloat64(Node* value);
   Node* ChangeUint32ToSmi(Node* value);
   Node* LoadHeapNumberValue(Node* value, Node* control);
@@ -45,6 +45,7 @@ class ChangeLowering final : public Reducer {
   Reduction ChangeBoolToBit(Node* value);
   Reduction ChangeFloat64ToTagged(Node* value, Node* control);
   Reduction ChangeInt32ToTagged(Node* value, Node* control);
+  Reduction ChangeSmiToInt32(Node* value);
   Reduction ChangeTaggedToFloat64(Node* value, Node* control);
   Reduction ChangeTaggedToUI32(Node* value, Node* control,
                                Signedness signedness);

src/compiler/opcodes.h

@@ -196,6 +196,7 @@
   V(NumberToUint32)                \
   V(NumberIsHoleNaN)               \
   V(StringToNumber)                \
+  V(ChangeSmiToInt32)              \
   V(ChangeTaggedToInt32)           \
   V(ChangeTaggedToUint32)          \
   V(ChangeTaggedToFloat64)         \

src/compiler/simplified-lowering.cc

@@ -8,6 +8,7 @@
 
 #include "src/base/bits.h"
 #include "src/code-factory.h"
+#include "src/compiler/access-builder.h"
 #include "src/compiler/common-operator.h"
 #include "src/compiler/diamond.h"
 #include "src/compiler/linkage.h"
@@ -761,19 +762,23 @@ class RepresentationSelector {
       case IrOpcode::kCall:
         return VisitCall(node, lowering);
 
-//------------------------------------------------------------------
-// JavaScript operators.
-//------------------------------------------------------------------
-// For now, we assume that all JS operators were too complex to lower
-// to Simplified and that they will always require tagged value inputs
-// and produce tagged value outputs.
-// TODO(turbofan): it might be possible to lower some JSOperators here,
-// but that responsibility really lies in the typed lowering phase.
-#define DEFINE_JS_CASE(x) case IrOpcode::k##x:
-        JS_OP_LIST(DEFINE_JS_CASE)
-#undef DEFINE_JS_CASE
+      //------------------------------------------------------------------
+      // JavaScript operators.
+      //------------------------------------------------------------------
+      case IrOpcode::kJSToNumber: {
         VisitInputs(node);
-        return SetOutput(node, MachineRepresentation::kTagged);
+        // TODO(bmeurer): Optimize somewhat based on input type?
+        if (truncation.TruncatesToWord32()) {
+          SetOutput(node, MachineRepresentation::kWord32);
+          if (lower()) lowering->DoJSToNumberTruncatesToWord32(node, this);
+        } else if (truncation.TruncatesToFloat64()) {
+          SetOutput(node, MachineRepresentation::kFloat64);
+          if (lower()) lowering->DoJSToNumberTruncatesToFloat64(node, this);
+        } else {
+          SetOutput(node, MachineRepresentation::kTagged);
+        }
+        break;
+      }
 
       //------------------------------------------------------------------
       // Simplified operators.
@@ -1463,6 +1468,165 @@ void SimplifiedLowering::LowerAllNodes() {
   selector.Run(this);
 }
 
+void SimplifiedLowering::DoJSToNumberTruncatesToFloat64(
+    Node* node, RepresentationSelector* selector) {
+  DCHECK_EQ(IrOpcode::kJSToNumber, node->opcode());
+  Node* value = node->InputAt(0);
+  Node* context = node->InputAt(1);
+  Node* frame_state = node->InputAt(2);
+  Node* effect = node->InputAt(3);
+  Node* control = node->InputAt(4);
+  Node* throwing;
+
+  Node* check0 = graph()->NewNode(simplified()->ObjectIsSmi(), value);
+  Node* branch0 =
+      graph()->NewNode(common()->Branch(BranchHint::kTrue), check0, control);
+
+  Node* if_true0 = graph()->NewNode(common()->IfTrue(), branch0);
+  Node* etrue0 = effect;
+  Node* vtrue0;
+  {
+    vtrue0 = graph()->NewNode(simplified()->ChangeSmiToInt32(), value);
+    vtrue0 = graph()->NewNode(machine()->ChangeInt32ToFloat64(), vtrue0);
+  }
+
+  Node* if_false0 = graph()->NewNode(common()->IfFalse(), branch0);
+  Node* efalse0 = effect;
+  Node* vfalse0;
+  {
+    throwing = vfalse0 = efalse0 =
+        graph()->NewNode(ToNumberOperator(), ToNumberCode(), value, context,
+                         frame_state, efalse0, if_false0);
+    if_false0 = graph()->NewNode(common()->IfSuccess(), throwing);
+
+    Node* check1 = graph()->NewNode(simplified()->ObjectIsSmi(), vfalse0);
+    Node* branch1 = graph()->NewNode(common()->Branch(), check1, if_false0);
+
+    Node* if_true1 = graph()->NewNode(common()->IfTrue(), branch1);
+    Node* etrue1 = efalse0;
+    Node* vtrue1;
+    {
+      vtrue1 = graph()->NewNode(simplified()->ChangeSmiToInt32(), vfalse0);
+      vtrue1 = graph()->NewNode(machine()->ChangeInt32ToFloat64(), vtrue1);
+    }
+
+    Node* if_false1 = graph()->NewNode(common()->IfFalse(), branch1);
+    Node* efalse1 = efalse0;
+    Node* vfalse1;
+    {
+      vfalse1 = efalse1 = graph()->NewNode(
+          simplified()->LoadField(AccessBuilder::ForHeapNumberValue()), efalse0,
+          efalse1, if_false1);
+    }
+
+    if_false0 = graph()->NewNode(common()->Merge(2), if_true1, if_false1);
+    efalse0 =
+        graph()->NewNode(common()->EffectPhi(2), etrue1, efalse1, if_false0);
+    vfalse0 =
+        graph()->NewNode(common()->Phi(MachineRepresentation::kFloat64, 2),
+                         vtrue1, vfalse1, if_false0);
+  }
+
+  control = graph()->NewNode(common()->Merge(2), if_true0, if_false0);
+  effect = graph()->NewNode(common()->EffectPhi(2), etrue0, efalse0, control);
+  value = graph()->NewNode(common()->Phi(MachineRepresentation::kFloat64, 2),
+                           vtrue0, vfalse0, control);
+
+  // Replace effect and control uses appropriately.
+  for (Edge edge : node->use_edges()) {
+    if (NodeProperties::IsControlEdge(edge)) {
+      if (edge.from()->opcode() == IrOpcode::kIfSuccess) {
+        edge.from()->ReplaceUses(control);
+        edge.from()->Kill();
+      } else if (edge.from()->opcode() == IrOpcode::kIfException) {
+        edge.UpdateTo(throwing);
+      } else {
+        UNREACHABLE();
+      }
+    } else if (NodeProperties::IsEffectEdge(edge)) {
+      edge.UpdateTo(effect);
+    }
+  }
+
+  selector->DeferReplacement(node, value);
+}
+
+void SimplifiedLowering::DoJSToNumberTruncatesToWord32(
+    Node* node, RepresentationSelector* selector) {
+  DCHECK_EQ(IrOpcode::kJSToNumber, node->opcode());
+  Node* value = node->InputAt(0);
+  Node* context = node->InputAt(1);
+  Node* frame_state = node->InputAt(2);
+  Node* effect = node->InputAt(3);
+  Node* control = node->InputAt(4);
+  Node* throwing;
+
+  Node* check0 = graph()->NewNode(simplified()->ObjectIsSmi(), value);
+  Node* branch0 =
+      graph()->NewNode(common()->Branch(BranchHint::kTrue), check0, control);
+
+  Node* if_true0 = graph()->NewNode(common()->IfTrue(), branch0);
+  Node* etrue0 = effect;
+  Node* vtrue0 = graph()->NewNode(simplified()->ChangeSmiToInt32(), value);
+
+  Node* if_false0 = graph()->NewNode(common()->IfFalse(), branch0);
+  Node* efalse0 = effect;
+  Node* vfalse0;
+  {
+    throwing = vfalse0 = efalse0 =
+        graph()->NewNode(ToNumberOperator(), ToNumberCode(), value, context,
+                         frame_state, efalse0, if_false0);
+    if_false0 = graph()->NewNode(common()->IfSuccess(), throwing);
+
+    Node* check1 = graph()->NewNode(simplified()->ObjectIsSmi(), vfalse0);
+    Node* branch1 = graph()->NewNode(common()->Branch(), check1, if_false0);
+
+    Node* if_true1 = graph()->NewNode(common()->IfTrue(), branch1);
+    Node* etrue1 = efalse0;
+    Node* vtrue1 = graph()->NewNode(simplified()->ChangeSmiToInt32(), vfalse0);
+
+    Node* if_false1 = graph()->NewNode(common()->IfFalse(), branch1);
+    Node* efalse1 = efalse0;
+    Node* vfalse1;
+    {
+      vfalse1 = efalse1 = graph()->NewNode(
+          simplified()->LoadField(AccessBuilder::ForHeapNumberValue()), efalse0,
+          efalse1, if_false1);
+      vfalse1 = graph()->NewNode(
+          machine()->TruncateFloat64ToInt32(TruncationMode::kJavaScript),
+          vfalse1);
+    }
+
+    if_false0 = graph()->NewNode(common()->Merge(2), if_true1, if_false1);
+    efalse0 =
+        graph()->NewNode(common()->EffectPhi(2), etrue1, efalse1, if_false0);
+    vfalse0 = graph()->NewNode(common()->Phi(MachineRepresentation::kWord32, 2),
+                               vtrue1, vfalse1, if_false0);
+  }
+
+  control = graph()->NewNode(common()->Merge(2), if_true0, if_false0);
+  effect = graph()->NewNode(common()->EffectPhi(2), etrue0, efalse0, control);
+  value = graph()->NewNode(common()->Phi(MachineRepresentation::kWord32, 2),
+                           vtrue0, vfalse0, control);
+
+  // Replace effect and control uses appropriately.
+  for (Edge edge : node->use_edges()) {
+    if (NodeProperties::IsControlEdge(edge)) {
+      if (edge.from()->opcode() == IrOpcode::kIfSuccess) {
+        edge.from()->ReplaceUses(control);
+        edge.from()->Kill();
+      } else if (edge.from()->opcode() == IrOpcode::kIfException) {
+        edge.UpdateTo(throwing);
+      } else {
+        UNREACHABLE();
+      }
+    } else if (NodeProperties::IsEffectEdge(edge)) {
+      edge.UpdateTo(effect);
+    }
+  }
+
+  selector->DeferReplacement(node, value);
+}
 
 void SimplifiedLowering::DoLoadBuffer(Node* node,
                                       MachineRepresentation output_rep,
@@ -2183,6 +2347,26 @@ void SimplifiedLowering::DoShift(Node* node, Operator const* op,
   NodeProperties::ChangeOp(node, op);
 }
 
+Node* SimplifiedLowering::ToNumberCode() {
+  if (!to_number_code_.is_set()) {
+    Callable callable = CodeFactory::ToNumber(isolate());
+    to_number_code_.set(jsgraph()->HeapConstant(callable.code()));
+  }
+  return to_number_code_.get();
+}
+
+Operator const* SimplifiedLowering::ToNumberOperator() {
+  if (!to_number_operator_.is_set()) {
+    Callable callable = CodeFactory::ToNumber(isolate());
+    CallDescriptor::Flags flags = CallDescriptor::kNeedsFrameState;
+    CallDescriptor* desc = Linkage::GetStubCallDescriptor(
+        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

src/compiler/simplified-lowering.h

@@ -21,6 +21,7 @@ namespace compiler {
 
 // Forward declarations.
 class RepresentationChanger;
+class RepresentationSelector;
 class SourcePositionTable;
 
 class SimplifiedLowering final {
@@ -31,6 +32,10 @@ class SimplifiedLowering final {
 
   void LowerAllNodes();
 
+  void DoJSToNumberTruncatesToFloat64(Node* node,
+                                      RepresentationSelector* selector);
+  void DoJSToNumberTruncatesToWord32(Node* node,
+                                     RepresentationSelector* selector);
   // TODO(turbofan): The representation can be removed once the result of the
   // representation analysis is stored in the node bounds.
   void DoLoadBuffer(Node* node, MachineRepresentation rep,
@@ -42,6 +47,8 @@ class SimplifiedLowering final {
   JSGraph* const jsgraph_;
   Zone* const zone_;
   TypeCache const& type_cache_;
+  SetOncePointer<Node> to_number_code_;
+  SetOncePointer<Operator const> to_number_operator_;
 
   // TODO(danno): SimplifiedLowering shouldn't know anything about the source
   // positions table, but must for now since there currently is no other way to
@@ -59,6 +66,9 @@ class SimplifiedLowering final {
   Node* Uint32Div(Node* const node);
   Node* Uint32Mod(Node* const node);
 
+  Node* ToNumberCode();
+  Operator const* ToNumberOperator();
+
   friend class RepresentationSelector;
 
   Isolate* isolate() { return jsgraph_->isolate(); }
@@ -67,6 +77,7 @@ class SimplifiedLowering final {
   Graph* graph() { return jsgraph()->graph(); }
   CommonOperatorBuilder* common() { return jsgraph()->common(); }
   MachineOperatorBuilder* machine() { return jsgraph()->machine(); }
+  SimplifiedOperatorBuilder* simplified() { return jsgraph()->simplified(); }
 };
 
 }  // namespace compiler

src/compiler/simplified-operator.cc

@@ -182,6 +182,7 @@ const ElementAccess& ElementAccessOf(const Operator* op) {
   V(NumberToUint32, Operator::kNoProperties, 1)          \
   V(NumberIsHoleNaN, Operator::kNoProperties, 1)         \
   V(StringToNumber, Operator::kNoProperties, 1)          \
+  V(ChangeSmiToInt32, Operator::kNoProperties, 1)        \
   V(ChangeTaggedToInt32, Operator::kNoProperties, 1)     \
   V(ChangeTaggedToUint32, Operator::kNoProperties, 1)    \
   V(ChangeTaggedToFloat64, Operator::kNoProperties, 1)   \

src/compiler/simplified-operator.h

@@ -159,6 +159,7 @@ class SimplifiedOperatorBuilder final : public ZoneObject {
   const Operator* StringLessThanOrEqual();
   const Operator* StringToNumber();
 
+  const Operator* ChangeSmiToInt32();
   const Operator* ChangeTaggedToInt32();
   const Operator* ChangeTaggedToUint32();
   const Operator* ChangeTaggedToFloat64();

src/compiler/typer.cc

@@ -1845,6 +1845,11 @@ Type* ChangeRepresentation(Type* type, Type* rep, Zone* zone) {
 
 }  // namespace
 
+Type* Typer::Visitor::TypeChangeSmiToInt32(Node* node) {
+  Type* arg = Operand(node, 0);
+  // TODO(neis): DCHECK(arg->Is(Type::Signed32()));
+  return ChangeRepresentation(arg, Type::UntaggedIntegral32(), zone());
+}
 
 Type* Typer::Visitor::TypeChangeTaggedToInt32(Node* node) {
   Type* arg = Operand(node, 0);

src/compiler/verifier.cc

@@ -760,6 +760,15 @@ void Verifier::Visitor::Check(Node* node) {
       CheckUpperIs(node, Type::TaggedPointer());
       break;
 
+    case IrOpcode::kChangeSmiToInt32: {
+      // Signed32 /\ Tagged -> Signed32 /\ UntaggedInt32
+      // TODO(neis): Activate once ChangeRepresentation works in typer.
+      // Type* from = Type::Intersect(Type::Signed32(), Type::Tagged());
+      // Type* to = Type::Intersect(Type::Signed32(), Type::UntaggedInt32());
+      // CheckValueInputIs(node, 0, from));
+      // CheckUpperIs(node, to));
+      break;
+    }
     case IrOpcode::kChangeTaggedToInt32: {
       // Signed32 /\ Tagged -> Signed32 /\ UntaggedInt32
       // TODO(neis): Activate once ChangeRepresentation works in typer.