Reviewers: Benedikt Meurer,

Description:
Improve the x64 Load and Store access instruction selection to include immediate
offsets.

Currently GetEffectiveAddressMemoryOperand() uses
BaseWithIndexAndDisplacement64Match. However this does not recognize
the ChangeUint32ToUint64 operation which is applied to the index. It
looks like a lot of work to rework BaseWithIndexAndDisplacement64Match,
and BaseWithIndexAndDisplacement32Match seems ok as-is, so it seems more
appropriate to add x64 backend specific code to match the patterns
which is done inline in GetEffectiveAddressMemoryOperand().

Some of the code in VisitChangeUint32ToUint64() has been refactored
into OpZeroExtends() and is needed in the pattern matching.

The BaseWithIndexAndDisplacementMatch code has some OwnedBy()
tests. It's not immediately clear if these are to maintain some
invariant or if they are just a heuristic? It seems generally better
to dehoist the index offsets aggressively so the OwnedBy() test is
omitted in this patch. This could use some reviewer scrutiny?

A TODO note is included to explore the scaled-index patterns matched
by BaseWithIndexAndDisplacement64Match. Such patterns are less likely
to be proven within bounds and thus not taken by these paths anyway,
so they are left for future work.

This patch gives a useful x0.92 improvement in run time for an asm.js
zlib benchmark when using index masking to avoid bounds checks.

BUG=

Please review this at https://codereview.chromium.org/860283004/

Base URL: https://chromium.googlesource.com/v8/v8.git@master

Affected files (+60, -38 lines):
  M src/compiler/x64/instruction-selector-x64.cc


Index: src/compiler/x64/instruction-selector-x64.cc
diff --git a/src/compiler/x64/instruction-selector-x64.cc b/src/compiler/x64/instruction-selector-x64.cc index 0e6e220306a8c921d01c8e10a00cf6f524022000..248b8a65451c7d9d29022be242358353bd68b188 100644
--- a/src/compiler/x64/instruction-selector-x64.cc
+++ b/src/compiler/x64/instruction-selector-x64.cc
@@ -10,6 +10,37 @@ namespace v8 {
 namespace internal {
 namespace compiler {

+// Operations that zero extend their 32 bit result to 64 bits.
+static bool OpZeroExtends(Node *value) {
+  switch (value->opcode()) {
+    case IrOpcode::kWord32And:
+    case IrOpcode::kWord32Or:
+    case IrOpcode::kWord32Xor:
+    case IrOpcode::kWord32Shl:
+    case IrOpcode::kWord32Shr:
+    case IrOpcode::kWord32Sar:
+    case IrOpcode::kWord32Ror:
+    case IrOpcode::kWord32Equal:
+    case IrOpcode::kInt32Add:
+    case IrOpcode::kInt32Sub:
+    case IrOpcode::kInt32Mul:
+    case IrOpcode::kInt32MulHigh:
+    case IrOpcode::kInt32Div:
+    case IrOpcode::kInt32LessThan:
+    case IrOpcode::kInt32LessThanOrEqual:
+    case IrOpcode::kInt32Mod:
+    case IrOpcode::kUint32Div:
+    case IrOpcode::kUint32LessThan:
+    case IrOpcode::kUint32LessThanOrEqual:
+    case IrOpcode::kUint32Mod:
+    case IrOpcode::kUint32MulHigh:
+      return true;
+    default:
+      return false;
+  }
+}
+
+
 // Adds X64-specific methods for generating operands.
 class X64OperandGenerator FINAL : public OperandGenerator {
  public:
@@ -87,16 +118,31 @@ class X64OperandGenerator FINAL : public OperandGenerator {
   AddressingMode GetEffectiveAddressMemoryOperand(Node* operand,
InstructionOperand* inputs[],
                                                   size_t* input_count) {
-    BaseWithIndexAndDisplacement64Matcher m(operand, true);
-    DCHECK(m.matches());
-    if ((m.displacement() == NULL || CanBeImmediate(m.displacement()))) {
-      return GenerateMemoryOperandInputs(m.index(), m.scale(), m.base(),
- m.displacement(), inputs, input_count);
-    } else {
-      inputs[(*input_count)++] = UseRegister(operand->InputAt(0));
-      inputs[(*input_count)++] = UseRegister(operand->InputAt(1));
-      return kMode_MR1;
+    inputs[(*input_count)++] = UseRegister(operand->InputAt(0));
+    Node *index = operand->InputAt(1);
+
+    if (CanBeImmediate(index)) {
+      inputs[(*input_count)++] = UseImmediate(index);
+      return kMode_MRI;
     }
+
+    if (index->opcode() == IrOpcode::kChangeUint32ToUint64) {
+      Node *index32 = index->InputAt(0);
+      // Match an index plus a constant offset.
+      if (index32->opcode() == IrOpcode::kInt32Add) {
+        Int32BinopMatcher madd(index32);
+        if (madd.right().HasValue() && OpZeroExtends(madd.left().node()) &&
+            CanBeImmediate(madd.right().node())) {
+          inputs[(*input_count)++] = UseRegister(madd.left().node());
+          inputs[(*input_count)++] = UseImmediate(madd.right().node());
+          return kMode_MR1I;
+        }
+      }
+      // TODO(turbofan): Match cases that include a shifted index.
+    }
+
+    inputs[(*input_count)++] = UseRegister(index);
+    return kMode_MR1;
   }

   bool CanBeBetterLeftOperand(Node* node) const {
@@ -762,35 +808,11 @@ void InstructionSelector::VisitChangeInt32ToInt64(Node* node) {
 void InstructionSelector::VisitChangeUint32ToUint64(Node* node) {
   X64OperandGenerator g(this);
   Node* value = node->InputAt(0);
-  switch (value->opcode()) {
-    case IrOpcode::kWord32And:
-    case IrOpcode::kWord32Or:
-    case IrOpcode::kWord32Xor:
-    case IrOpcode::kWord32Shl:
-    case IrOpcode::kWord32Shr:
-    case IrOpcode::kWord32Sar:
-    case IrOpcode::kWord32Ror:
-    case IrOpcode::kWord32Equal:
-    case IrOpcode::kInt32Add:
-    case IrOpcode::kInt32Sub:
-    case IrOpcode::kInt32Mul:
-    case IrOpcode::kInt32MulHigh:
-    case IrOpcode::kInt32Div:
-    case IrOpcode::kInt32LessThan:
-    case IrOpcode::kInt32LessThanOrEqual:
-    case IrOpcode::kInt32Mod:
-    case IrOpcode::kUint32Div:
-    case IrOpcode::kUint32LessThan:
-    case IrOpcode::kUint32LessThanOrEqual:
-    case IrOpcode::kUint32Mod:
-    case IrOpcode::kUint32MulHigh: {
- // These 32-bit operations implicitly zero-extend to 64-bit on x64, so the
-      // zero-extension is a no-op.
-      Emit(kArchNop, g.DefineSameAsFirst(node), g.Use(value));
-      return;
-    }
-    default:
-      break;
+  if (OpZeroExtends(value)) {
+ // These 32-bit operations implicitly zero-extend to 64-bit on x64, so the
+    // zero-extension is a no-op.
+    Emit(kArchNop, g.DefineSameAsFirst(node), g.Use(value));
+    return;
   }
   Emit(kX64Movl, g.DefineAsRegister(node), g.Use(value));
 }


--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to