[compiler] Remove StackPointerGreaterThan from pure op list

This operator implicitly reads the stack pointer register and is thus
not pure. This CL removes it from the list of pure operators and
inserts it into the effect chain at its use sites.

Drive-by: Alpha-sort the list in CanAllocate.

Bug: v8:9534
Change-Id: Icf96fb3e308600dbacec3dbfb7386a4f0d0bdc39
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1875098
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Auto-Submit: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64502}

src/compiler/js-generic-lowering.cc

@@ -858,7 +858,8 @@ void JSGenericLowering::LowerJSStackCheck(Node* node) {
                            ExternalReference::address_of_jslimit(isolate())),
                        jsgraph()->IntPtrConstant(0), effect, control);
 
-  Node* check = graph()->NewNode(machine()->StackPointerGreaterThan(), limit);
+  Node* check = effect =
+      graph()->NewNode(machine()->StackPointerGreaterThan(), limit, effect);
   Node* branch =
       graph()->NewNode(common()->Branch(BranchHint::kTrue), check, control);
 

src/compiler/machine-operator.cc

@@ -404,8 +404,7 @@ MachineType AtomicOpType(Operator const* op) {
   V(S1x8AllTrue, Operator::kNoProperties, 1, 0, 1)                            \
   V(S1x16AnyTrue, Operator::kNoProperties, 1, 0, 1)                           \
   V(S1x16AllTrue, Operator::kNoProperties, 1, 0, 1)                           \
-  V(S8x16Swizzle, Operator::kNoProperties, 2, 0, 1)                           \
-  V(StackPointerGreaterThan, Operator::kNoProperties, 1, 0, 1)
+  V(S8x16Swizzle, Operator::kNoProperties, 2, 0, 1)
 
 // The format is:
 // V(Name, properties, value_input_count, control_input_count, output_count)
@@ -898,6 +897,13 @@ struct MachineOperatorGlobalCache {
                    "UnsafePointerAdd", 2, 1, 1, 1, 1, 0) {}
   };
   UnsafePointerAddOperator kUnsafePointerAdd;
+
+  struct StackPointerGreaterThanOperator final : public Operator {
+    StackPointerGreaterThanOperator()
+        : Operator(IrOpcode::kStackPointerGreaterThan, Operator::kEliminatable,
+                   "StackPointerGreaterThan", 1, 1, 0, 1, 1, 0) {}
+  };
+  StackPointerGreaterThanOperator kStackPointerGreaterThan;
 };
 
 struct CommentOperator : public Operator1<const char*> {
@@ -1064,6 +1070,10 @@ const Operator* MachineOperatorBuilder::UnsafePointerAdd() {
   return &cache_.kUnsafePointerAdd;
 }
 
+const Operator* MachineOperatorBuilder::StackPointerGreaterThan() {
+  return &cache_.kStackPointerGreaterThan;
+}
+
 const Operator* MachineOperatorBuilder::BitcastWordToTagged() {
   return &cache_.kBitcastWordToTagged;
 }

src/compiler/memory-optimizer.cc

@@ -21,16 +21,16 @@ namespace {
 
 bool CanAllocate(const Node* node) {
   switch (node->opcode()) {
+    case IrOpcode::kAbortCSAAssert:
     case IrOpcode::kBitcastTaggedToWord:
     case IrOpcode::kBitcastWordToTagged:
-    case IrOpcode::kChangeCompressedToTagged:
-    case IrOpcode::kChangeCompressedSignedToTaggedSigned:
     case IrOpcode::kChangeCompressedPointerToTaggedPointer:
-    case IrOpcode::kChangeTaggedToCompressed:
-    case IrOpcode::kChangeTaggedSignedToCompressedSigned:
+    case IrOpcode::kChangeCompressedSignedToTaggedSigned:
+    case IrOpcode::kChangeCompressedToTagged:
     case IrOpcode::kChangeTaggedPointerToCompressedPointer:
+    case IrOpcode::kChangeTaggedSignedToCompressedSigned:
+    case IrOpcode::kChangeTaggedToCompressed:
     case IrOpcode::kComment:
-    case IrOpcode::kAbortCSAAssert:
     case IrOpcode::kDebugBreak:
     case IrOpcode::kDeoptimizeIf:
     case IrOpcode::kDeoptimizeUnless:
@@ -44,6 +44,8 @@ bool CanAllocate(const Node* node) {
     case IrOpcode::kProtectedLoad:
     case IrOpcode::kProtectedStore:
     case IrOpcode::kRetain:
+    case IrOpcode::kStackPointerGreaterThan:
+    case IrOpcode::kStaticAssert:
     // TODO(tebbi): Store nodes might do a bump-pointer allocation.
     //              We should introduce a special bump-pointer store node to
     //              differentiate that.
@@ -54,9 +56,8 @@ bool CanAllocate(const Node* node) {
     case IrOpcode::kTaggedPoisonOnSpeculation:
     case IrOpcode::kUnalignedLoad:
     case IrOpcode::kUnalignedStore:
-    case IrOpcode::kUnsafePointerAdd:
     case IrOpcode::kUnreachable:
-    case IrOpcode::kStaticAssert:
+    case IrOpcode::kUnsafePointerAdd:
     case IrOpcode::kWord32AtomicAdd:
     case IrOpcode::kWord32AtomicAnd:
     case IrOpcode::kWord32AtomicCompareExchange:

src/compiler/wasm-compiler.cc

@@ -328,8 +328,9 @@ void WasmGraphBuilder::StackCheck(wasm::WasmCodePosition position,
       mcgraph()->IntPtrConstant(0), limit_address, *control);
   *effect = limit;
 
-  Node* check =
-      graph()->NewNode(mcgraph()->machine()->StackPointerGreaterThan(), limit);
+  Node* check = graph()->NewNode(
+      mcgraph()->machine()->StackPointerGreaterThan(), limit, *effect);
+  *effect = check;
 
   Diamond stack_check(graph(), mcgraph()->common(), check, BranchHint::kTrue);
   stack_check.Chain(*control);