From eb87b88a385b1287476ff19a52ca51ad0414bd1b Mon Sep 17 00:00:00 2001
From: Stephan Herhut <herhut@chromium.org>
Date: Thu, 28 Jun 2018 16:56:28 +0200
Subject: [PATCH] [wasm] Use esp/rsp register directly in stack checks

This change helps mostly to reduce register pressure. By default, we
compile LoadStackPointer into a move from the stack register into some
general purpose register and then later use that register. This might
trigger a spill to free up space, which is particularly costly in
loops.

Change-Id: I886233f890b7833f873fc24773f621add7cf0588
Reviewed-on: https://chromium-review.googlesource.com/1104351
Commit-Queue: Stephan Herhut <herhut@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54126}
---
 .../ia32/instruction-selector-ia32.cc         | 12 ++++++++
 src/compiler/node-matchers.h                  | 28 +++++++++++++++++++
 src/compiler/wasm-compiler.cc                 |  4 +++
 src/compiler/x64/instruction-selector-x64.cc  | 12 ++++++++
 4 files changed, 56 insertions(+)

diff --git a/src/compiler/ia32/instruction-selector-ia32.cc b/src/compiler/ia32/instruction-selector-ia32.cc
index e2a0509f0c..b286517349 100644
--- a/src/compiler/ia32/instruction-selector-ia32.cc
+++ b/src/compiler/ia32/instruction-selector-ia32.cc
@@ -1254,6 +1254,18 @@ void VisitWordCompare(InstructionSelector* selector, Node* node,
     selector->EmitWithContinuation(opcode, cont);
     return;
   }
+  WasmStackCheckMatcher<Int32BinopMatcher, IrOpcode::kUint32LessThan> wasm_m(
+      node);
+  if (wasm_m.Matched()) {
+    // This is a wasm stack check. By structure, we know that we can use the
+    // stack pointer directly, as wasm code does not modify the stack at points
+    // where stack checks are performed.
+    Node* left = node->InputAt(0);
+    LocationOperand esp(InstructionOperand::EXPLICIT, LocationOperand::REGISTER,
+                        InstructionSequence::DefaultRepresentation(),
+                        RegisterCode::kRegCode_esp);
+    return VisitCompareWithMemoryOperand(selector, kIA32Cmp, left, esp, cont);
+  }
   VisitWordCompare(selector, node, kIA32Cmp, cont);
 }
 
diff --git a/src/compiler/node-matchers.h b/src/compiler/node-matchers.h
index 1a8e07f86f..2dd0d472b0 100644
--- a/src/compiler/node-matchers.h
+++ b/src/compiler/node-matchers.h
@@ -743,6 +743,34 @@ struct V8_EXPORT_PRIVATE DiamondMatcher
   Node* if_false_;
 };
 
+template <class BinopMatcher, IrOpcode::Value expected_opcode>
+struct WasmStackCheckMatcher {
+  explicit WasmStackCheckMatcher(Node* compare) : compare_(compare) {}
+
+  bool Matched() {
+    if (compare_->opcode() != expected_opcode) return false;
+    BinopMatcher m(compare_);
+    return MatchedInternal(m.left(), m.right());
+  }
+
+ private:
+  bool MatchedInternal(const typename BinopMatcher::LeftMatcher& l,
+                       const typename BinopMatcher::RightMatcher& r) {
+    // In wasm, the stack check is performed by loading the value given by
+    // the address of a field stored in the instance object. That object is
+    // passed as a parameter.
+    if (l.IsLoad() && r.IsLoadStackPointer()) {
+      LoadMatcher<LoadMatcher<NodeMatcher>> mleft(l.node());
+      if (mleft.object().IsLoad() && mleft.index().Is(0) &&
+          mleft.object().object().IsParameter()) {
+        return true;
+      }
+    }
+    return false;
+  }
+  Node* compare_;
+};
+
 template <class BinopMatcher, IrOpcode::Value expected_opcode>
 struct StackCheckMatcher {
   StackCheckMatcher(Isolate* isolate, Node* compare)
diff --git a/src/compiler/wasm-compiler.cc b/src/compiler/wasm-compiler.cc
index 4f464ed0df..26247a9c31 100644
--- a/src/compiler/wasm-compiler.cc
+++ b/src/compiler/wasm-compiler.cc
@@ -245,6 +245,10 @@ void WasmGraphBuilder::StackCheck(wasm::WasmCodePosition position,
   if (effect == nullptr) effect = effect_;
   if (control == nullptr) control = control_;
 
+  // This instruction sequence is matched in the instruction selector to
+  // load the stack pointer directly on some platforms. Hence, when modifying
+  // please also fix WasmStackCheckMatcher in node-matchers.h
+
   Node* limit_address = graph()->NewNode(
       mcgraph()->machine()->Load(MachineType::Pointer()), instance_node_.get(),
       mcgraph()->Int32Constant(WASM_INSTANCE_OBJECT_OFFSET(StackLimitAddress)),
diff --git a/src/compiler/x64/instruction-selector-x64.cc b/src/compiler/x64/instruction-selector-x64.cc
index 6d41f1ebb9..975d7b7c88 100644
--- a/src/compiler/x64/instruction-selector-x64.cc
+++ b/src/compiler/x64/instruction-selector-x64.cc
@@ -1724,6 +1724,18 @@ void VisitWord64Compare(InstructionSelector* selector, Node* node,
     selector->EmitWithContinuation(opcode, cont);
     return;
   }
+  WasmStackCheckMatcher<Int64BinopMatcher, IrOpcode::kUint64LessThan> wasm_m(
+      node);
+  if (wasm_m.Matched()) {
+    // This is a wasm stack check. By structure, we know that we can use the
+    // stack pointer directly, as wasm code does not modify the stack at points
+    // where stack checks are performed.
+    Node* left = node->InputAt(0);
+    LocationOperand rsp(InstructionOperand::EXPLICIT, LocationOperand::REGISTER,
+                        InstructionSequence::DefaultRepresentation(),
+                        RegisterCode::kRegCode_rsp);
+    return VisitCompareWithMemoryOperand(selector, kX64Cmp, left, rsp, cont);
+  }
   VisitWordCompare(selector, node, kX64Cmp, cont);
 }