[turbofan] Avoid unnecessary write barriers and improve code generation.

src/compiler/x64/code-generator-x64.cc

 // Copyright 2013 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/compiler/code-generator.h"
 
 #include "src/compiler/code-generator-impl.h"
 #include "src/compiler/gap-resolver.h"
 #include "src/compiler/node-matchers.h"
 #include "src/compiler/osr.h"
@@ skipping 173 matching lines @@
     __ Movsd(MemOperand(rsp, 0), input_);
     __ SlowTruncateToI(result_, rsp, 0);
     __ addp(rsp, Immediate(kDoubleSize));
   }
 
  private:
   Register const result_;
   XMMRegister const input_;
 };
 
+
+class OutOfLineRecordWrite final : public OutOfLineCode {
+ public:
+  OutOfLineRecordWrite(CodeGenerator* gen, Register object, Operand operand,
+                       Register value, Register scratch0, Register scratch1,
+                       RecordWriteMode mode)
+      : OutOfLineCode(gen),
+        object_(object),
+        operand_(operand),
+        value_(value),
+        scratch0_(scratch0),
+        scratch1_(scratch1),
+        mode_(mode) {}
+
+  void Generate() final {
+    if (mode_ > RecordWriteMode::kValueIsPointer) {
+      __ JumpIfSmi(value_, exit());
+    }
+    if (mode_ > RecordWriteMode::kValueIsMap) {
+      __ CheckPageFlag(value_, scratch0_,

[ulan, 2016/02/19 15:02:32]

Eliding kPointersToHereAreInterestingMask check for maps is not correct.

There are two issues:
1. Incremental marker dynamically sets and clears this flag for map space pages,
so we don't know the value of the flag at code generation time.

2. Even assuming that there is no incremental marking, the intention of the code
was probably to not emit the write barrier because we know that the flag is 0
for maps? But this code actually forces the write barrier because it removes the
check. So we end up with old -> map slots in old -> new remembered set.

Context: https://bugs.chromium.org/p/chromium/issues/detail?id=587004#c13

+                       MemoryChunk::kPointersToHereAreInterestingMask, zero,
+                       exit());
+    }
+    SaveFPRegsMode const save_fp_mode =
+        frame()->DidAllocateDoubleRegisters() ? kSaveFPRegs : kDontSaveFPRegs;
+    RecordWriteStub stub(isolate(), object_, scratch0_, scratch1_,
+                         EMIT_REMEMBERED_SET, save_fp_mode);
+    __ leap(scratch1_, operand_);
+    __ CallStub(&stub);
+  }
+
+ private:
+  Register const object_;
+  Operand const operand_;
+  Register const value_;
+  Register const scratch0_;
+  Register const scratch1_;
+  RecordWriteMode const mode_;
+};
+
 }  // namespace
 
 
 #define ASSEMBLE_UNOP(asm_instr)         \
   do {                                   \
     if (instr->Output()->IsRegister()) { \
       __ asm_instr(i.OutputRegister());  \
     } else {                             \
       __ asm_instr(i.OutputOperand());   \
     }                                    \
@@ skipping 443 matching lines @@
     case kArchTruncateDoubleToI: {
       auto result = i.OutputRegister();
       auto input = i.InputDoubleRegister(0);
       auto ool = new (zone()) OutOfLineTruncateDoubleToI(this, result, input);
       __ Cvttsd2siq(result, input);
       __ cmpq(result, Immediate(1));
       __ j(overflow, ool->entry());
       __ bind(ool->exit());
       break;
     }
+    case kArchStoreWithWriteBarrier: {
+      RecordWriteMode mode =
+          static_cast<RecordWriteMode>(MiscField::decode(instr->opcode()));
+      Register object = i.InputRegister(0);
+      size_t index = 0;
+      Operand operand = i.MemoryOperand(&index);
+      Register value = i.InputRegister(index);
+      Register scratch0 = i.TempRegister(0);
+      Register scratch1 = i.TempRegister(1);
+      auto ool = new (zone()) OutOfLineRecordWrite(this, object, operand, value,
+                                                   scratch0, scratch1, mode);
+      __ movp(operand, value);
+      __ CheckPageFlag(object, scratch0,
+                       MemoryChunk::kPointersFromHereAreInterestingMask,
+                       not_zero, ool->entry());
+      __ bind(ool->exit());
+      break;
+    }
     case kX64Add32:
       ASSEMBLE_BINOP(addl);
       break;
     case kX64Add:
       ASSEMBLE_BINOP(addq);
       break;
     case kX64Sub32:
       ASSEMBLE_BINOP(subl);
       break;
     case kX64Sub:
@@ skipping 625 matching lines @@
       break;
     case kX64Poke: {
       int const slot = MiscField::decode(instr->opcode());
       if (HasImmediateInput(instr, 0)) {
         __ movq(Operand(rsp, slot * kPointerSize), i.InputImmediate(0));
       } else {
         __ movq(Operand(rsp, slot * kPointerSize), i.InputRegister(0));
       }
       break;
     }
-    case kX64StoreWriteBarrier: {
-      Register object = i.InputRegister(0);
-      Register value = i.InputRegister(2);
-      SaveFPRegsMode mode =
-          frame()->DidAllocateDoubleRegisters() ? kSaveFPRegs : kDontSaveFPRegs;
-      if (HasImmediateInput(instr, 1)) {
-        int index = i.InputInt32(1);
-        Register scratch = i.TempRegister(1);
-        __ movq(Operand(object, index), value);
-        __ RecordWriteContextSlot(object, index, value, scratch, mode);
-      } else {
-        Register index = i.InputRegister(1);
-        __ movq(Operand(object, index, times_1, 0), value);
-        __ leaq(index, Operand(object, index, times_1, 0));
-        __ RecordWrite(object, index, value, mode);
-      }
-      break;
-    }
     case kCheckedLoadInt8:
       ASSEMBLE_CHECKED_LOAD_INTEGER(movsxbl);
       break;
     case kCheckedLoadUint8:
       ASSEMBLE_CHECKED_LOAD_INTEGER(movzxbl);
       break;
     case kCheckedLoadInt16:
       ASSEMBLE_CHECKED_LOAD_INTEGER(movsxwl);
       break;
     case kCheckedLoadUint16:
@@ skipping 515 matching lines @@
     int padding_size = last_lazy_deopt_pc_ + space_needed - current_pc;
     __ Nop(padding_size);
   }
 }
 
 #undef __
 
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8