diff options
author | 2024-01-22 08:57:31 +0000 | |
---|---|---|
committer | 2024-01-22 10:49:24 +0000 | |
commit | b5b98b9bb31acb2deffb692c50d0fbc71476663b (patch) | |
tree | 3bda094f8d5ea90ad34e5d357889ce4933f657f5 /compiler/optimizing/graph_checker.cc | |
parent | dbf9d9309f3df9c9ac8a9e30277b31ebb2977f4d (diff) |
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
Diffstat (limited to 'compiler/optimizing/graph_checker.cc')
-rw-r--r-- | compiler/optimizing/graph_checker.cc | 95 |
1 files changed, 95 insertions, 0 deletions
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) { |