[maglev] Add a write barrier to StoreField

StoreField wasn't emitting a write barrier after performing the store,
leading to the usual set of hard-to-debug issues. Now it does.

The write barrier requires some of its registers to be in fixed
locations, and others to be clobberable. Thsi patch extends the
temporaries mechanism to allow requesting a specific temporary, in this
case for the slot address scratch register.

Bug: v8:7700
Change-Id: I506856071e0f44feafb98c2685ef1b3362b0e41e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3613388
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80263}

src/maglev/maglev-ir.cc

@@ -7,6 +7,7 @@
 #include "src/base/bits.h"
 #include "src/base/logging.h"
 #include "src/codegen/interface-descriptors-inl.h"
+#include "src/codegen/interface-descriptors.h"
 #include "src/codegen/macro-assembler-inl.h"
 #include "src/codegen/register.h"
 #include "src/compiler/backend/instruction.h"
@@ -600,8 +601,14 @@ void LoadDoubleField::PrintParams(std::ostream& os,
 
 void StoreField::AllocateVreg(MaglevVregAllocationState* vreg_state,
                               const ProcessingState& state) {
-  UseRegister(object_input());
+  UseFixed(object_input(), WriteBarrierDescriptor::ObjectRegister());
   UseRegister(value_input());
+  // We need the slot address to be free, and an additional scratch register
+  // for the value.
+  // TODO(leszeks): Add input clobbering to remove the need for this
+  // unconditional value scratch register.
+  RequireSpecificTemporary(WriteBarrierDescriptor::SlotAddressRegister());
+  set_temporaries_needed(2);
 }
 void StoreField::GenerateCode(MaglevCodeGenState* code_gen_state,
                               const ProcessingState& state) {
@@ -609,10 +616,19 @@ void StoreField::GenerateCode(MaglevCodeGenState* code_gen_state,
   Register value = ToRegister(value_input());
 
   if (StoreHandler::IsInobjectBits::decode(this->handler())) {
-    Operand operand = FieldOperand(
-        object,
-        StoreHandler::FieldIndexBits::decode(this->handler()) * kTaggedSize);
-    __ StoreTaggedField(operand, value);
+    RegList temps = temporaries();
+    DCHECK(temporaries().has(WriteBarrierDescriptor::SlotAddressRegister()));
+    temps.clear(WriteBarrierDescriptor::SlotAddressRegister());
+    int offset =
+        StoreHandler::FieldIndexBits::decode(this->handler()) * kTaggedSize;
+    __ StoreTaggedField(FieldOperand(object, offset), value);
+    // TODO(leszeks): Add input clobbering to remove the need for this
+    // unconditional value scratch register.
+    Register value_scratch = temps.PopFirst();
+    __ movq(value_scratch, value);
+    __ RecordWriteField(object, offset, value_scratch,
+                        WriteBarrierDescriptor::SlotAddressRegister(),
+                        SaveFPRegsMode::kSave);
   } else {
     // TODO(victorgomes): Out-of-object properties.
     UNSUPPORTED("StoreField out-of-object property");

src/maglev/maglev-ir.h

@@ -516,33 +516,11 @@ class NodeBase : public ZoneObject {
     id_ = id;
   }
 
-  int num_temporaries_needed() const {
-#ifdef DEBUG
-    if (kTemporariesState == kUnset) {
-      DCHECK_EQ(num_temporaries_needed_, 0);
-    } else {
-      DCHECK_EQ(kTemporariesState, kNeedsTemporaries);
-    }
-#endif  // DEBUG
-    return num_temporaries_needed_;
-  }
+  int num_temporaries_needed() const { return num_temporaries_needed_; }
 
-  RegList temporaries() const {
-    DCHECK_EQ(kTemporariesState, kHasTemporaries);
-    return temporaries_;
-  }
+  RegList temporaries() const { return temporaries_; }
 
-  void assign_temporaries(RegList list) {
-#ifdef DEBUG
-    if (kTemporariesState == kUnset) {
-      DCHECK_EQ(num_temporaries_needed_, 0);
-    } else {
-      DCHECK_EQ(kTemporariesState, kNeedsTemporaries);
-    }
-    kTemporariesState = kHasTemporaries;
-#endif  // DEBUG
-    temporaries_ = list;
-  }
+  void assign_temporaries(RegList list) { temporaries_ = list; }
 
   void Print(std::ostream& os, MaglevGraphLabeller*) const;
 
@@ -604,14 +582,19 @@ class NodeBase : public ZoneObject {
     bit_field_ = InputCountField::update(bit_field_, input_count() - 1);
   }
 
+  // Specify that there need to be a certain number of registers free (i.e.
+  // useable as scratch registers) on entry into this node.
+  //
+  // Includes any registers requested by RequireSpecificTemporary.
   void set_temporaries_needed(int value) {
-#ifdef DEBUG
-    DCHECK_EQ(kTemporariesState, kUnset);
-    kTemporariesState = kNeedsTemporaries;
-#endif  // DEBUG
+    DCHECK_EQ(num_temporaries_needed_, 0);
     num_temporaries_needed_ = value;
   }
 
+  // Require that a specific register is free (and therefore clobberable) by the
+  // entry into this node.
+  void RequireSpecificTemporary(Register reg) { temporaries_.set(reg); }
+
   EagerDeoptInfo* eager_deopt_info_address() {
     DCHECK(properties().can_eager_deopt());
     DCHECK(!properties().can_lazy_deopt());
@@ -656,18 +639,8 @@ class NodeBase : public ZoneObject {
 
  private:
   NodeIdT id_ = kInvalidNodeId;
-
-  union {
-    int num_temporaries_needed_ = 0;
-    RegList temporaries_;
-  };
-#ifdef DEBUG
-  enum {
-    kUnset,
-    kNeedsTemporaries,
-    kHasTemporaries
-  } kTemporariesState = kUnset;
-#endif  // DEBUG
+  uint8_t num_temporaries_needed_ = 0;
+  RegList temporaries_;
 
   NodeBase() = delete;
   NodeBase(const NodeBase&) = delete;

src/maglev/maglev-regalloc.cc

@@ -861,6 +861,15 @@ compiler::InstructionOperand RegisterFrameState<RegisterT>::TryAllocateRegister(
 
 void StraightForwardRegisterAllocator::AssignTemporaries(NodeBase* node) {
   // TODO(victorgomes): Support double registers as temporaries.
+  RegList initial_temporaries = node->temporaries();
+
+  // Make sure that any initially set temporaries are definitely free.
+  for (Register reg : initial_temporaries) {
+    if (general_registers_.free().has(reg)) continue;
+    DropRegisterValue(general_registers_, reg);
+    general_registers_.AddToFree(reg);
+  }
+
   int num_temporaries_needed = node->num_temporaries_needed();
   int num_free_registers = general_registers_.free().Count();
 
@@ -870,6 +879,8 @@ void StraightForwardRegisterAllocator::AssignTemporaries(NodeBase* node) {
   }
 
   DCHECK_GE(general_registers_.free().Count(), num_temporaries_needed);
+  DCHECK_EQ(general_registers_.free() | initial_temporaries,
+            general_registers_.free());
   node->assign_temporaries(general_registers_.free());
 }