[vm, compiler] Reduce spilling needed to call __tsan_read/write.

Make the Dart register allocator spill what is needed instead of spilling all volatile registers.

TEST=tsan
Bug: #61352
Change-Id: Ic50937e305698565a14469181b1e5f408104c656
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/445964
Reviewed-by: Alexander Aprelev <[email protected]>
Commit-Queue: Ryan Macnak <[email protected]>

Author: rmacnak-google

runtime/vm/compiler/backend/constant_propagator.cc

@@ -307,6 +307,8 @@ void ConstantPropagator::VisitDeoptimize(DeoptimizeInstr* instr) {
   // TODO(vegorov) remove all code after DeoptimizeInstr as dead.
 }
 
+void ConstantPropagator::VisitTsanReadWrite(TsanReadWriteInstr* instr) {}
+
 Definition* ConstantPropagator::UnwrapPhi(Definition* defn) {
   if (defn->IsPhi()) {
     JoinEntryInstr* block = defn->AsPhi()->block();

runtime/vm/compiler/backend/flow_graph.cc

@@ -2673,6 +2673,36 @@ void FlowGraph::ExtractNonInternalTypedDataPayloads() {
   }
 }
 
+void FlowGraph::AddTsanInstrumentation() {
+  if (!FLAG_target_thread_sanitizer) return;
+
+  for (BlockIterator block_it = reverse_postorder_iterator(); !block_it.Done();
+       block_it.Advance()) {
+    BlockEntryInstr* block = block_it.Current();
+    for (ForwardInstructionIterator it(block); !it.Done(); it.Advance()) {
+      Instruction* current = it.Current();
+
+      if (LoadFieldInstr* load = current->AsLoadField()) {
+        if (!load->slot().is_no_sanitize_thread() &&
+            load->memory_order() == compiler::Assembler::kRelaxedNonAtomic) {
+          auto* tsan_read = new (Z) TsanReadWriteInstr(
+              TsanReadWriteInstr::Kind::kRead, load->slot(),
+              new (Z) Value(load->instance()->definition()), load->source());
+          InsertBefore(load, tsan_read, /*env=*/nullptr, kEffect);
+        }
+      } else if (StoreFieldInstr* store = current->AsStoreField()) {
+        if (!store->slot().is_no_sanitize_thread() &&
+            store->memory_order() == compiler::Assembler::kRelaxedNonAtomic) {
+          auto* tsan_write = new (Z) TsanReadWriteInstr(
+              TsanReadWriteInstr::Kind::kWrite, store->slot(),
+              new (Z) Value(store->instance()->definition()), store->source());
+          InsertBefore(store, tsan_write, /*env=*/nullptr, kEffect);
+        }
+      }
+    }
+  }
+}
+
 bool FlowGraph::Canonicalize() {
   bool changed = false;
 

runtime/vm/compiler/backend/flow_graph.h

@@ -531,6 +531,8 @@ class FlowGraph : public ZoneAllocated {
   // Once this is done, no intra-block code motion should be performed.
   void ExtractNonInternalTypedDataPayloads();
 
+  void AddTsanInstrumentation();
+
   bool IsReceiver(Definition* def) const;
 
   // Optimize (a << b) & c pattern: if c is a positive Smi or zero, then the

runtime/vm/compiler/backend/il.cc

@@ -4684,16 +4684,6 @@ void LoadFieldInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
   ASSERT(OffsetInBytes() != 0 || slot().has_untagged_instance());
   auto const rep = slot().representation();
 
-#if defined(TARGET_ARCH_ARM64) || defined(TARGET_ARCH_RISCV64) ||              \
-    defined(TARGET_ARCH_X64)
-  if (FLAG_target_thread_sanitizer && !slot().is_no_sanitize_thread() &&
-      memory_order_ == compiler::Assembler::kRelaxedNonAtomic) {
-    intptr_t tag = slot().has_untagged_instance() ? 0 : kHeapObjectTag;
-    __ AddImmediate(TMP, instance_reg, slot().offset_in_bytes() - tag);
-    __ TsanRead(TMP, RepresentationUtils::ValueSize(rep));
-  }
-#endif
-
   if (calls_initializer()) {
     __ LoadFromSlot(locs()->out(0).reg(), instance_reg, slot(), memory_order_);
     EmitNativeCodeForInitializerCall(compiler);
@@ -7293,6 +7283,91 @@ Definition* InvokeMathCFunctionInstr::Canonicalize(FlowGraph* flow_graph) {
   return this;
 }
 
+LocationSummary* TsanReadWriteInstr::MakeLocationSummary(Zone* zone,
+                                                         bool opt) const {
+  const intptr_t kNumTemps = 1;
+  LocationSummary* result = new (zone) LocationSummary(
+      zone, InputCount(), kNumTemps, LocationSummary::kNativeLeafCall);
+  result->set_temp(0, Location::RegisterLocation(CALLEE_SAVED_TEMP));
+  result->set_in(0, Location::RequiresRegister());
+  return result;
+}
+
+const RuntimeEntry& TsanReadWriteInstr::RuntimeEntry() const {
+  intptr_t size = RepresentationUtils::ValueSize(slot().representation());
+  if (kind_ == Kind::kRead) {
+    switch (size) {
+      case 1:
+        return kTsanRead1RuntimeEntry;
+      case 2:
+        return kTsanRead2RuntimeEntry;
+      case 4:
+        return kTsanRead4RuntimeEntry;
+      case 8:
+        return kTsanRead8RuntimeEntry;
+      case 16:
+        return kTsanRead16RuntimeEntry;
+      default:
+        UNREACHABLE();
+    }
+  } else if (kind_ == kWrite) {
+    switch (size) {
+      case 1:
+        return kTsanWrite1RuntimeEntry;
+      case 2:
+        return kTsanWrite2RuntimeEntry;
+      case 4:
+        return kTsanWrite4RuntimeEntry;
+      case 8:
+        return kTsanWrite8RuntimeEntry;
+      case 16:
+        return kTsanWrite16RuntimeEntry;
+      default:
+        UNREACHABLE();
+    }
+  }
+  UNREACHABLE();
+}
+
+void TsanReadWriteInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
+#if defined(TARGET_ARCH_ARM64) || defined(TARGET_ARCH_RISCV64) ||              \
+    defined(TARGET_ARCH_X64)
+  const Register saved_sp = locs()->temp(0).reg();
+  ASSERT(IsAbiPreservedRegister(saved_sp));
+  ASSERT(IsAbiPreservedRegister(THR));
+  ASSERT(IsAbiPreservedRegister(PP));
+  ASSERT(IsAbiPreservedRegister(CODE_REG));
+#if defined(TARGET_ARCH_ARM64)
+  ASSERT(IsAbiPreservedRegister(NULL_REG));
+  ASSERT(IsAbiPreservedRegister(HEAP_BITS));
+  ASSERT(IsAbiPreservedRegister(DISPATCH_TABLE_REG));
+#endif
+  __ MoveRegister(saved_sp, SPREG);
+#if defined(TARGET_ARCH_ARM64)
+  __ andi(CSP, SP, compiler::Immediate(~(OS::ActivationFrameAlignment() - 1)));
+#else
+  __ ReserveAlignedFrameSpace(0);
+#endif
+  const Register instance_reg = locs()->in(0).reg();
+  intptr_t tag = slot().has_untagged_instance() ? 0 : kHeapObjectTag;
+  __ AddImmediate(CallingConventions::ArgumentRegisters[0], instance_reg,
+                  slot().offset_in_bytes() - tag);
+  __ Load(TMP, compiler::Address(THR, RuntimeEntry().OffsetFromThread()));
+  __ Store(TMP,
+           compiler::Address(THR, compiler::target::Thread::vm_tag_offset()));
+  __ CallCFunction(TMP);
+  __ LoadImmediate(TMP, VMTag::kDartTagId);
+  __ Store(TMP,
+           compiler::Address(THR, compiler::target::Thread::vm_tag_offset()));
+  __ MoveRegister(SPREG, saved_sp);
+#if defined(TARGET_ARCH_ARM64)
+  __ SetupCSPFromThread(THR);
+#endif
+#else
+  UNREACHABLE();
+#endif
+}
+
 bool DoubleToIntegerInstr::SupportsFloorAndCeil() {
 #if defined(TARGET_ARCH_X64)
   return CompilerState::Current().is_aot() || FLAG_target_unknown_cpu;
@@ -7821,16 +7896,6 @@ void StoreFieldInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
   ASSERT(OffsetInBytes() != 0 || slot().has_untagged_instance());
   const Representation rep = slot().representation();
 
-#if defined(TARGET_ARCH_ARM64) || defined(TARGET_ARCH_RISCV64) ||              \
-    defined(TARGET_ARCH_X64)
-  if (FLAG_target_thread_sanitizer && !slot().is_no_sanitize_thread() &&
-      memory_order_ == compiler::Assembler::kRelaxedNonAtomic) {
-    intptr_t tag = slot().has_untagged_instance() ? 0 : kHeapObjectTag;
-    __ AddImmediate(TMP, instance_reg, slot().offset_in_bytes() - tag);
-    __ TsanWrite(TMP, RepresentationUtils::ValueSize(rep));
-  }
-#endif
-
 #if defined(TARGET_ARCH_ARM64) || defined(TARGET_ARCH_RISCV32) ||              \
     defined(TARGET_ARCH_RISCV64)
   if (locs()->in(kValuePos).IsConstant() &&

runtime/vm/compiler/backend/il.h

@@ -512,6 +512,7 @@ struct InstrAttrs {
   M(OneByteStringFromCharCode, kNoGC)                                          \
   M(Utf8Scan, kNoGC)                                                           \
   M(InvokeMathCFunction, kNoGC)                                                \
+  M(TsanReadWrite, kNoGC)                                                      \
   M(TruncDivMod, kNoGC)                                                        \
   /*We could be more precise about when these 2 instructions can trigger GC.*/ \
   M(GuardFieldClass, _)                                                        \
@@ -6422,6 +6423,9 @@ class StoreFieldInstr : public TemplateInstruction<2, NoThrow> {
 
   Value* instance() const { return inputs_[kInstancePos]; }
   const Slot& slot() const { return slot_; }
+  compiler::Assembler::MemoryOrder memory_order() const {
+    return memory_order_;
+  }
   Value* value() const { return inputs_[kValuePos]; }
 
   virtual TokenPosition token_pos() const { return token_pos_; }
@@ -8168,6 +8172,9 @@ class LoadFieldInstr : public TemplateLoadField<1> {
 
   Value* instance() const { return inputs_[0]; }
   const Slot& slot() const { return slot_; }
+  compiler::Assembler::MemoryOrder memory_order() const {
+    return memory_order_;
+  }
 
   InnerPointerAccess loads_inner_pointer() const {
     return loads_inner_pointer_;
@@ -10031,6 +10038,64 @@ class InvokeMathCFunctionInstr : public VariadicDefinition {
   DISALLOW_COPY_AND_ASSIGN(InvokeMathCFunctionInstr);
 };
 
+class TsanReadWriteInstr : public TemplateInstruction<1, NoThrow> {
+ public:
+  enum Kind { kRead, kWrite };
+
+  TsanReadWriteInstr(Kind kind,
+                     const Slot& slot,
+                     Value* instance,
+                     const InstructionSource& source)
+      : TemplateInstruction(source, DeoptId::kNone),
+        kind_(kind),
+        slot_(slot),
+        token_pos_(source.token_pos) {
+    SetInputAt(0, instance);
+  }
+
+  Value* instance() const { return inputs_[0]; }
+  const Slot& slot() const { return slot_; }
+
+  virtual TokenPosition token_pos() const { return token_pos_; }
+
+  DECLARE_INSTRUCTION(TsanReadWrite)
+
+  virtual bool ComputeCanDeoptimize() const { return false; }
+
+  virtual Representation RequiredInputRepresentation(intptr_t idx) const {
+    ASSERT_EQUAL(idx, 0);
+    return slot_.has_untagged_instance() ? kUntagged : kTagged;
+  }
+
+  virtual intptr_t DeoptimizationTarget() const { return GetDeoptId(); }
+
+  virtual bool AllowsCSE() const { return false; }
+  virtual bool HasUnknownSideEffects() const { return false; }
+
+  virtual bool AttributesEqual(const Instruction& other) const {
+    return &other == this;
+  }
+
+  virtual bool MayThrow() const { return false; }
+
+  PRINT_OPERANDS_TO_SUPPORT
+
+#define FIELD_LIST(F)                                                          \
+  F(const TsanReadWriteInstr::Kind, kind_)                                     \
+  F(const Slot&, slot_)                                                        \
+  F(const TokenPosition, token_pos_)
+
+  DECLARE_INSTRUCTION_SERIALIZABLE_FIELDS(TsanReadWriteInstr,
+                                          TemplateInstruction,
+                                          FIELD_LIST)
+#undef FIELD_LIST
+
+  const RuntimeEntry& RuntimeEntry() const;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TsanReadWriteInstr);
+};
+
 class ExtractNthOutputInstr : public TemplateDefinition<1, NoThrow, Pure> {
  public:
   // Extract the Nth output register from value.

runtime/vm/compiler/backend/il_printer.cc

@@ -1237,6 +1237,16 @@ void InvokeMathCFunctionInstr::PrintOperandsTo(BaseTextBuffer* f) const {
   Definition::PrintOperandsTo(f);
 }
 
+void TsanReadWriteInstr::PrintOperandsTo(BaseTextBuffer* f) const {
+  instance()->PrintTo(f);
+  f->Printf(" . %s", slot().Name());
+  if (kind_ == kRead) {
+    f->AddString(", read");
+  } else {
+    f->AddString(", write");
+  }
+}
+
 void GraphEntryInstr::PrintBlockHeaderTo(BaseTextBuffer* f) const {
   f->Printf("B%" Pd "[graph]:%" Pd, block_id(), GetDeoptId());
 }

runtime/vm/compiler/compiler_pass.cc

@@ -562,8 +562,10 @@ COMPILER_PASS(TestILSerialization, {
   }
 });
 
-COMPILER_PASS(LoweringAfterCodeMotionDisabled,
-              { flow_graph->ExtractNonInternalTypedDataPayloads(); });
+COMPILER_PASS(LoweringAfterCodeMotionDisabled, {
+  flow_graph->ExtractNonInternalTypedDataPayloads();
+  flow_graph->AddTsanInstrumentation();
+});
 
 COMPILER_PASS(GenerateCode, { state->graph_compiler->CompileGraph(); });