From b5b98b9bb31acb2deffb692c50d0fbc71476663b Mon Sep 17 00:00:00 2001
From: Santiago Aboy Solanes <solanes@google.com>
Date: Mon, 22 Jan 2024 08:57:31 +0000
Subject: Revert^3 "Disable write-barrier elimination pass"

This reverts commit 9f8df195b7ff2ce47eec4e9b193ff3214ebed19c.

Reason for revert: Fix for x86_64 with heap poison enabled

This case uses a temp with index `1` in the regular FieldSet case.
This is done like this due to GenerateVarHandleSet also calling
HandleFieldSet. The bug was that we were allocating only one
temp in the regular FieldSet case and therefore not having the
temp with index `1` available.

PS1 is the revert as-is.
PS2 contains the fix.

Test: art/test/testrunner/testrunner.py --host --64 --optimizing -b
Test: Same command with heap poison enabled too
Bug: 301833859
Bug: 310755375
Bug: 260843353
Change-Id: Ie2740b4c443158c4e72810ce1d8268353c5f0055
---
 compiler/optimizing/graph_checker.cc | 95 ++++++++++++++++++++++++++++++++++++
 1 file changed, 95 insertions(+)

(limited to 'compiler/optimizing/graph_checker.cc')

diff --git a/compiler/optimizing/graph_checker.cc b/compiler/optimizing/graph_checker.cc
index 8f2f25355d..36ac7bc82c 100644
--- a/compiler/optimizing/graph_checker.cc
+++ b/compiler/optimizing/graph_checker.cc
@@ -31,6 +31,7 @@
 #include "mirror/class.h"
 #include "nodes.h"
 #include "obj_ptr-inl.h"
+#include "optimizing/data_type.h"
 #include "scoped_thread_state_change-inl.h"
 #include "subtype_check.h"
 
@@ -1280,6 +1281,26 @@ void GraphChecker::VisitNeg(HNeg* instruction) {
   }
 }
 
+HInstruction* HuntForOriginalReference(HInstruction* ref) {
+  // An original reference can be transformed by instructions like:
+  //   i0 NewArray
+  //   i1 HInstruction(i0)  <-- NullCheck, BoundType, IntermediateAddress.
+  //   i2 ArraySet(i1, index, value)
+  DCHECK(ref != nullptr);
+  while (ref->IsNullCheck() || ref->IsBoundType() || ref->IsIntermediateAddress()) {
+    ref = ref->InputAt(0);
+  }
+  return ref;
+}
+
+bool IsRemovedWriteBarrier(DataType::Type type,
+                           WriteBarrierKind write_barrier_kind,
+                           HInstruction* value) {
+  return write_barrier_kind == WriteBarrierKind::kDontEmit &&
+         type == DataType::Type::kReference &&
+         !value->IsNullConstant();
+}
+
 void GraphChecker::VisitArraySet(HArraySet* instruction) {
   VisitInstruction(instruction);
 
@@ -1293,6 +1314,80 @@ void GraphChecker::VisitArraySet(HArraySet* instruction) {
                      StrBool(instruction->NeedsTypeCheck()),
                      StrBool(instruction->GetSideEffects().Includes(SideEffects::CanTriggerGC()))));
   }
+
+  if (IsRemovedWriteBarrier(instruction->GetComponentType(),
+                            instruction->GetWriteBarrierKind(),
+                            instruction->GetValue())) {
+    CheckWriteBarrier(instruction, [](HInstruction* it_instr) {
+      return it_instr->AsArraySet()->GetWriteBarrierKind();
+    });
+  }
+}
+
+void GraphChecker::VisitInstanceFieldSet(HInstanceFieldSet* instruction) {
+  VisitInstruction(instruction);
+  if (IsRemovedWriteBarrier(instruction->GetFieldType(),
+                            instruction->GetWriteBarrierKind(),
+                            instruction->GetValue())) {
+    CheckWriteBarrier(instruction, [](HInstruction* it_instr) {
+      return it_instr->AsInstanceFieldSet()->GetWriteBarrierKind();
+    });
+  }
+}
+
+void GraphChecker::VisitStaticFieldSet(HStaticFieldSet* instruction) {
+  VisitInstruction(instruction);
+  if (IsRemovedWriteBarrier(instruction->GetFieldType(),
+                            instruction->GetWriteBarrierKind(),
+                            instruction->GetValue())) {
+    CheckWriteBarrier(instruction, [](HInstruction* it_instr) {
+      return it_instr->AsStaticFieldSet()->GetWriteBarrierKind();
+    });
+  }
+}
+
+template <typename GetWriteBarrierKind>
+void GraphChecker::CheckWriteBarrier(HInstruction* instruction,
+                                     GetWriteBarrierKind&& get_write_barrier_kind) {
+  DCHECK(instruction->IsStaticFieldSet() ||
+         instruction->IsInstanceFieldSet() ||
+         instruction->IsArraySet());
+
+  // For removed write barriers, we expect that the write barrier they are relying on is:
+  // A) In the same block, and
+  // B) There's no instruction between them that can trigger a GC.
+  HInstruction* object = HuntForOriginalReference(instruction->InputAt(0));
+  bool found = false;
+  for (HBackwardInstructionIterator it(instruction); !it.Done(); it.Advance()) {
+    if (instruction->GetKind() == it.Current()->GetKind() &&
+        object == HuntForOriginalReference(it.Current()->InputAt(0)) &&
+        get_write_barrier_kind(it.Current()) == WriteBarrierKind::kEmitBeingReliedOn) {
+      // Found the write barrier we are relying on.
+      found = true;
+      break;
+    }
+
+    // We check the `SideEffects::CanTriggerGC` after failing to find the write barrier since having
+    // a write barrier that's relying on an ArraySet that can trigger GC is fine because the card
+    // table is marked after the GC happens.
+    if (it.Current()->GetSideEffects().Includes(SideEffects::CanTriggerGC())) {
+      AddError(
+          StringPrintf("%s %d from block %d was expecting a write barrier and it didn't find "
+                       "any. %s %d can trigger GC",
+                       instruction->DebugName(),
+                       instruction->GetId(),
+                       instruction->GetBlock()->GetBlockId(),
+                       it.Current()->DebugName(),
+                       it.Current()->GetId()));
+    }
+  }
+
+  if (!found) {
+    AddError(StringPrintf("%s %d in block %d didn't find a write barrier to latch onto",
+                          instruction->DebugName(),
+                          instruction->GetId(),
+                          instruction->GetBlock()->GetBlockId()));
+  }
 }
 
 void GraphChecker::VisitBinaryOperation(HBinaryOperation* op) {
-- 
cgit v1.2.3-59-g8ed1b