Revert of [turbofan] Connect ObjectIsNumber to effect and control chains....

Revert of [turbofan] Connect ObjectIsNumber to effect and control chains. (patchset #1 id:1 of https://codereview.chromium.org/1709093002/ )

Reason for revert:
Tanks benchmarks (e.g., Octane box2d TF).

Original issue's description:
> [turbofan] Connect ObjectIsNumber to effect and control chains.
>
> In theory, we could connect the nodes when doing
> the schedule-in-the-middle pass, but that would require creating two
> versions of the operator (effectful and pure). I believe we do not
> lose anything by wiring the node up eagerly.
>
> Committed: https://crrev.com/2894e80a0a4a51a0d72e72aa48fcd01968f7949f
> Cr-Commit-Position: refs/heads/master@{#34141}

TBR=bmeurer@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

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

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

src/compiler/change-lowering.cc

@@ -596,42 +596,36 @@ Node* ChangeLowering::IsSmi(Node* value) {
       jsgraph()->IntPtrConstant(kSmiTag));
 }
 
-Node* ChangeLowering::LoadHeapObjectMap(Node* object, Node* effect,
-                                        Node* control) {
+Node* ChangeLowering::LoadHeapObjectMap(Node* object, Node* control) {
   return graph()->NewNode(
       machine()->Load(MachineType::AnyTagged()), object,
       jsgraph()->IntPtrConstant(HeapObject::kMapOffset - kHeapObjectTag),
-      effect, control);
+      graph()->start(), control);
 }
 
-Node* ChangeLowering::LoadMapInstanceType(Node* map, Node* effect,
-                                          Node* control) {
+Node* ChangeLowering::LoadMapInstanceType(Node* map) {
   return graph()->NewNode(
       machine()->Load(MachineType::Uint8()), map,
       jsgraph()->IntPtrConstant(Map::kInstanceTypeOffset - kHeapObjectTag),
-      effect, control);
+      graph()->start(), graph()->start());
 }
 
 Reduction ChangeLowering::ObjectIsNumber(Node* node) {
   Node* input = NodeProperties::GetValueInput(node, 0);
-  Node* control = NodeProperties::GetControlInput(node, 0);
-  Node* effect = NodeProperties::GetEffectInput(node, 0);
   // TODO(bmeurer): Optimize somewhat based on input type.
   Node* check = IsSmi(input);
-  Node* branch = graph()->NewNode(common()->Branch(), check, control);
+  Node* branch = graph()->NewNode(common()->Branch(), check, graph()->start());
   Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
   Node* vtrue = jsgraph()->Int32Constant(1);
   Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
-  Node* load_map = LoadHeapObjectMap(input, effect, if_false);
   Node* vfalse = graph()->NewNode(
-      machine()->WordEqual(), load_map,
+      machine()->WordEqual(), LoadHeapObjectMap(input, if_false),
       jsgraph()->HeapConstant(isolate()->factory()->heap_number_map()));
-  Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
-  Node* ephi =
-      graph()->NewNode(common()->EffectPhi(2), effect, load_map, merge);
-  Node* phi = graph()->NewNode(common()->Phi(MachineRepresentation::kBit, 2),
-                               vtrue, vfalse, merge);
-  ReplaceWithValue(node, phi, ephi, merge);
+  Node* control = graph()->NewNode(common()->Merge(2), if_true, if_false);
+  node->ReplaceInput(0, vtrue);
+  node->AppendInput(graph()->zone(), vfalse);
+  node->AppendInput(graph()->zone(), control);
+  NodeProperties::ChangeOp(node, common()->Phi(MachineRepresentation::kBit, 2));
   return Changed(node);
 }
 
@@ -644,12 +638,10 @@ Reduction ChangeLowering::ObjectIsReceiver(Node* node) {
   Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
   Node* vtrue = jsgraph()->Int32Constant(0);
   Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
-  Node* load_map = LoadHeapObjectMap(input, graph()->start(), if_false);
-  Node* load_instance_type =
-      LoadMapInstanceType(load_map, graph()->start(), graph()->start());
-  Node* vfalse = graph()->NewNode(
-      machine()->Uint32LessThanOrEqual(),
-      jsgraph()->Uint32Constant(FIRST_JS_RECEIVER_TYPE), load_instance_type);
+  Node* vfalse =
+      graph()->NewNode(machine()->Uint32LessThanOrEqual(),
+                       jsgraph()->Uint32Constant(FIRST_JS_RECEIVER_TYPE),
+                       LoadMapInstanceType(LoadHeapObjectMap(input, if_false)));
   Node* control = graph()->NewNode(common()->Merge(2), if_true, if_false);
   node->ReplaceInput(0, vtrue);
   node->AppendInput(graph()->zone(), vfalse);

src/compiler/change-lowering.h

@@ -19,10 +19,9 @@ class Linkage;
 class MachineOperatorBuilder;
 class Operator;
 
-class ChangeLowering final : public AdvancedReducer {
+class ChangeLowering final : public Reducer {
  public:
-  ChangeLowering(Editor* editor, JSGraph* jsgraph)
-      : AdvancedReducer(editor), jsgraph_(jsgraph) {}
+  explicit ChangeLowering(JSGraph* jsgraph) : jsgraph_(jsgraph) {}
   ~ChangeLowering() final;
 
   Reduction Reduce(Node* node) final;
@@ -58,8 +57,8 @@ class ChangeLowering final : public AdvancedReducer {
   Reduction Allocate(Node* node);
 
   Node* IsSmi(Node* value);
-  Node* LoadHeapObjectMap(Node* object, Node* effect, Node* control);
-  Node* LoadMapInstanceType(Node* map, Node* effect, Node* control);
+  Node* LoadHeapObjectMap(Node* object, Node* control);
+  Node* LoadMapInstanceType(Node* map);
 
   Reduction ObjectIsNumber(Node* node);
   Reduction ObjectIsReceiver(Node* node);

src/compiler/graph-reducer.cc

@@ -222,8 +222,7 @@ void GraphReducer::ReplaceWithValue(Node* node, Node* value, Node* effect,
         edge.UpdateTo(dead_);
         Revisit(user);
       } else {
-        edge.UpdateTo(control);
-        Revisit(user);
+        UNREACHABLE();
       }
     } else if (NodeProperties::IsEffectEdge(edge)) {
       DCHECK_NOT_NULL(effect);

src/compiler/js-native-context-specialization.cc

@@ -290,9 +290,8 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess(
       } else {
         DCHECK_EQ(AccessMode::kStore, access_mode);
         if (field_type->Is(Type::UntaggedFloat64())) {
-          Node* check = this_control = this_effect =
-              graph()->NewNode(simplified()->ObjectIsNumber(), this_value,
-                               this_effect, this_control);
+          Node* check =
+              graph()->NewNode(simplified()->ObjectIsNumber(), this_value);
           Node* branch = graph()->NewNode(common()->Branch(BranchHint::kTrue),
                                           check, this_control);
           exit_controls.push_back(
@@ -653,9 +652,8 @@ Reduction JSNativeContextSpecialization::ReduceElementAccess(
 
     // Check that the {index} is actually a Number.
     if (!NumberMatcher(this_index).HasValue()) {
-      Node* check = this_control = this_effect =
-          graph()->NewNode(simplified()->ObjectIsNumber(), this_index,
-                           this_effect, this_control);
+      Node* check =
+          graph()->NewNode(simplified()->ObjectIsNumber(), this_index);
       Node* branch = graph()->NewNode(common()->Branch(BranchHint::kTrue),
                                       check, this_control);
       exit_controls.push_back(graph()->NewNode(common()->IfFalse(), branch));
@@ -827,9 +825,8 @@ Reduction JSNativeContextSpecialization::ReduceElementAccess(
         this_value = graph()->NewNode(common()->Guard(type_cache_.kSmi),
                                       this_value, this_control);
       } else if (IsFastDoubleElementsKind(elements_kind)) {
-        Node* check = this_control = this_effect =
-            graph()->NewNode(simplified()->ObjectIsNumber(), this_value,
-                             this_effect, this_control);
+        Node* check =
+            graph()->NewNode(simplified()->ObjectIsNumber(), this_value);
         Node* branch = graph()->NewNode(common()->Branch(BranchHint::kTrue),
                                         check, this_control);
         exit_controls.push_back(graph()->NewNode(common()->IfFalse(), branch));

src/compiler/pipeline.cc

@@ -730,7 +730,7 @@ struct ChangeLoweringPhase {
                                               data->common());
     SimplifiedOperatorReducer simple_reducer(data->jsgraph());
     ValueNumberingReducer value_numbering(temp_zone);
-    ChangeLowering lowering(&graph_reducer, data->jsgraph());
+    ChangeLowering lowering(data->jsgraph());
     MachineOperatorReducer machine_reducer(data->jsgraph());
     CommonOperatorReducer common_reducer(&graph_reducer, data->graph(),
                                          data->common(), data->machine());

src/compiler/simplified-lowering.cc

@@ -1240,7 +1240,6 @@ class RepresentationSelector {
       }
       case IrOpcode::kObjectIsNumber: {
         ProcessInput(node, 0, UseInfo::AnyTagged());
-        ProcessRemainingInputs(node, 1);
         SetOutput(node, NodeOutputInfo::Bool());
         break;
       }

src/compiler/simplified-operator.cc

@@ -186,6 +186,7 @@ const ElementAccess& ElementAccessOf(const Operator* op) {
   V(ChangeFloat64ToTagged, Operator::kNoProperties, 1)   \
   V(ChangeBoolToBit, Operator::kNoProperties, 1)         \
   V(ChangeBitToBool, Operator::kNoProperties, 1)         \
+  V(ObjectIsNumber, Operator::kNoProperties, 1)          \
   V(ObjectIsReceiver, Operator::kNoProperties, 1)        \
   V(ObjectIsSmi, Operator::kNoProperties, 1)
 
@@ -194,8 +195,6 @@ const ElementAccess& ElementAccessOf(const Operator* op) {
   V(StringLessThan, Operator::kNoThrow, 2)  \
   V(StringLessThanOrEqual, Operator::kNoThrow, 2)
 
-#define STATEFUL_OP_LIST(V) V(ObjectIsNumber)
-
 struct SimplifiedOperatorGlobalCache final {
 #define PURE(Name, properties, input_count)                                \
   struct Name##Operator final : public Operator {                          \
@@ -207,16 +206,6 @@ struct SimplifiedOperatorGlobalCache final {
   PURE_OP_LIST(PURE)
 #undef PURE
 
-#define STATEFUL(Name)                                                         \
-  struct Name##Operator final : public Operator {                              \
-    Name##Operator()                                                           \
-        : Operator(IrOpcode::k##Name, Operator::kNoProperties, #Name, 1, 1, 1, \
-                   1, 1, 1) {}                                                 \
-  };                                                                           \
-  Name##Operator k##Name;
-  STATEFUL_OP_LIST(STATEFUL)
-#undef STATEFUL
-
 #define NO_THROW(Name, properties, input_count)                               \
   struct Name##Operator final : public Operator {                             \
     Name##Operator()                                                          \
@@ -263,10 +252,6 @@ PURE_OP_LIST(GET_FROM_CACHE)
 NO_THROW_OP_LIST(GET_FROM_CACHE)
 #undef GET_FROM_CACHE
 
-#define GET_FROM_CACHE(Name) \
-  const Operator* SimplifiedOperatorBuilder::Name() { return &cache_.k##Name; }
-STATEFUL_OP_LIST(GET_FROM_CACHE)
-#undef GET_FROM_CACHE
 
 const Operator* SimplifiedOperatorBuilder::ReferenceEqual(Type* type) {
   return new (zone()) Operator(IrOpcode::kReferenceEqual,

src/compiler/wasm-compiler.cc

@@ -1987,9 +1987,9 @@ Handle<JSFunction> CompileJSToWasmWrapper(
     typer.Run(roots);
 
     // Run generic and change lowering.
-    GraphReducer graph_reducer(&zone, &graph, jsgraph.Dead());
     JSGenericLowering generic(true, &jsgraph);
-    ChangeLowering changes(&graph_reducer, &jsgraph);
+    ChangeLowering changes(&jsgraph);
+    GraphReducer graph_reducer(&zone, &graph, jsgraph.Dead());
     graph_reducer.AddReducer(&changes);
     graph_reducer.AddReducer(&generic);
     graph_reducer.ReduceGraph();
@@ -2072,9 +2072,9 @@ Handle<Code> CompileWasmToJSWrapper(Isolate* isolate, wasm::ModuleEnv* module,
     typer.Run(roots);
 
     // Run generic and change lowering.
-    GraphReducer graph_reducer(&zone, &graph, jsgraph.Dead());
     JSGenericLowering generic(true, &jsgraph);
-    ChangeLowering changes(&graph_reducer, &jsgraph);
+    ChangeLowering changes(&jsgraph);
+    GraphReducer graph_reducer(&zone, &graph, jsgraph.Dead());
     graph_reducer.AddReducer(&changes);
     graph_reducer.AddReducer(&generic);
     graph_reducer.ReduceGraph();

test/cctest/compiler/test-changes-lowering.cc

@@ -129,9 +129,9 @@ class ChangesLoweringTester : public GraphBuilderTester<ReturnType> {
     // Run the graph reducer with changes lowering on a single node.
     Typer typer(this->isolate(), this->graph());
     typer.Run();
-    GraphReducer reducer(this->zone(), this->graph());
-    ChangeLowering change_lowering(&reducer, &jsgraph);
+    ChangeLowering change_lowering(&jsgraph);
     SelectLowering select_lowering(this->graph(), this->common());
+    GraphReducer reducer(this->zone(), this->graph());
     reducer.AddReducer(&change_lowering);
     reducer.AddReducer(&select_lowering);
     reducer.ReduceNode(change);

test/cctest/compiler/test-simplified-lowering.cc

@@ -60,8 +60,8 @@ class SimplifiedLoweringTester : public GraphBuilderTester<ReturnType> {
     typer.Run();
     lowering.LowerAllNodes();
 
+    ChangeLowering lowering(&jsgraph);
     GraphReducer reducer(this->zone(), this->graph());
-    ChangeLowering lowering(&reducer, &jsgraph);
     reducer.AddReducer(&lowering);
     reducer.ReduceGraph();
     Verifier::Run(this->graph());
@@ -726,8 +726,8 @@ class TestingGraph : public HandleAndZoneScope, public GraphAndBuilders {
     SourcePositionTable table(jsgraph.graph());
     SimplifiedLowering(&jsgraph, jsgraph.zone(), &table).LowerAllNodes();
 
+    ChangeLowering lowering(&jsgraph);
     GraphReducer reducer(this->zone(), this->graph());
-    ChangeLowering lowering(&reducer, &jsgraph);
     reducer.AddReducer(&lowering);
     reducer.ReduceGraph();
     Verifier::Run(this->graph());

test/unittests/compiler/change-lowering-unittest.cc

@@ -42,8 +42,7 @@ class ChangeLoweringTest : public TypedGraphTest {
     JSOperatorBuilder javascript(zone());
     JSGraph jsgraph(isolate(), graph(), common(), &javascript, nullptr,
                     &machine);
-    GraphReducer graph_reducer(zone(), graph());
-    ChangeLowering reducer(&graph_reducer, &jsgraph);
+    ChangeLowering reducer(&jsgraph);
     return reducer.Reduce(node);
   }
 

test/unittests/compiler/simplified-operator-unittest.cc

@@ -65,6 +65,7 @@ const PureOperator kPureOperators[] = {
     PURE(ChangeFloat64ToTagged, Operator::kNoProperties, 1),
     PURE(ChangeBoolToBit, Operator::kNoProperties, 1),
     PURE(ChangeBitToBool, Operator::kNoProperties, 1),
+    PURE(ObjectIsNumber, Operator::kNoProperties, 1),
     PURE(ObjectIsReceiver, Operator::kNoProperties, 1),
     PURE(ObjectIsSmi, Operator::kNoProperties, 1)
 #undef PURE