From 1be176f5a78750e2f0e32470f8c83e3d1643954d Mon Sep 17 00:00:00 2001 From: Santiago Aboy Solanes Date: Tue, 23 Jan 2024 13:55:50 +0000 Subject: Revert^5 "Disable write-barrier elimination pass" This reverts commit 31b949bc4a76e5c6d00a8e18c346f123b5321a1c. Reason for revert: PS1 is reland as-is PS2 has two fixes: * Missed poisoning heap references in a code path * Removed incorrect DCHECK Change-Id: I81b317ddc704dbd8a173f5d5c624dbc69e2d9e60 Test: art/test/testrunner/testrunner.py --host --64 --optimizing -b Test: art/test/testrunner/testrunner.py --target --64 --optimizing -b Both commnands with `export ART_HEAP_POISONING=true` Bug: 301833859 Bug: 310755375 Bug: 260843353 --- compiler/optimizing/nodes.h | 46 ++++++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 21 deletions(-) (limited to 'compiler/optimizing/nodes.h') diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h index 0efe8f4335..8dd89fa4e4 100644 --- a/compiler/optimizing/nodes.h +++ b/compiler/optimizing/nodes.h @@ -2903,6 +2903,10 @@ class HBackwardInstructionIterator : public ValueObject { next_ = Done() ? nullptr : instruction_->GetPrevious(); } + explicit HBackwardInstructionIterator(HInstruction* instruction) : instruction_(instruction) { + next_ = Done() ? nullptr : instruction_->GetPrevious(); + } + bool Done() const { return instruction_ == nullptr; } HInstruction* Current() const { return instruction_; } void Advance() { @@ -6369,19 +6373,13 @@ class HInstanceFieldGet final : public HExpression<1> { }; enum class WriteBarrierKind { - // Emit the write barrier, with a runtime optimization which checks if the value that it is being - // set is null. - kEmitWithNullCheck, - // Emit the write barrier, without the runtime null check optimization. This could be set because: - // A) It is a write barrier for an ArraySet (which does the optimization with the type check, so - // it never does the optimization at the write barrier stage) - // B) We know that the input can't be null - // C) This write barrier is actually several write barriers coalesced into one. Potentially we - // could ask if every value is null for a runtime optimization at the cost of compile time / code - // size. At the time of writing it was deemed not worth the effort. - kEmitNoNullCheck, + // Emit the write barrier. This write barrier is not being relied on so e.g. codegen can decide to + // skip it if the value stored is null. This is the default behavior. + kEmitNotBeingReliedOn, + // Emit the write barrier. This write barrier is being relied on and must be emitted. + kEmitBeingReliedOn, // Skip emitting the write barrier. This could be set because: - // A) The write barrier is not needed (e.g. it is not a reference, or the value is the null + // A) The write barrier is not needed (i.e. it is not a reference, or the value is the null // constant) // B) This write barrier was coalesced into another one so there's no need to emit it. kDontEmit, @@ -6412,7 +6410,7 @@ class HInstanceFieldSet final : public HExpression<2> { declaring_class_def_index, dex_file) { SetPackedFlag(true); - SetPackedField(WriteBarrierKind::kEmitWithNullCheck); + SetPackedField(WriteBarrierKind::kEmitNotBeingReliedOn); SetRawInputAt(0, object); SetRawInputAt(1, value); } @@ -6433,8 +6431,11 @@ class HInstanceFieldSet final : public HExpression<2> { void ClearValueCanBeNull() { SetPackedFlag(false); } WriteBarrierKind GetWriteBarrierKind() { return GetPackedField(); } void SetWriteBarrierKind(WriteBarrierKind kind) { - DCHECK(kind != WriteBarrierKind::kEmitWithNullCheck) + DCHECK(kind != WriteBarrierKind::kEmitNotBeingReliedOn) << "We shouldn't go back to the original value."; + DCHECK_IMPLIES(kind == WriteBarrierKind::kDontEmit, + GetWriteBarrierKind() != WriteBarrierKind::kEmitBeingReliedOn) + << "If a write barrier was relied on by other write barriers, we cannot skip emitting it."; SetPackedField(kind); } @@ -6576,8 +6577,7 @@ class HArraySet final : public HExpression<3> { SetPackedFlag(value->GetType() == DataType::Type::kReference); SetPackedFlag(true); SetPackedFlag(false); - // ArraySets never do the null check optimization at the write barrier stage. - SetPackedField(WriteBarrierKind::kEmitNoNullCheck); + SetPackedField(WriteBarrierKind::kEmitNotBeingReliedOn); SetRawInputAt(0, array); SetRawInputAt(1, index); SetRawInputAt(2, value); @@ -6653,10 +6653,11 @@ class HArraySet final : public HExpression<3> { WriteBarrierKind GetWriteBarrierKind() { return GetPackedField(); } void SetWriteBarrierKind(WriteBarrierKind kind) { - DCHECK(kind != WriteBarrierKind::kEmitNoNullCheck) + DCHECK(kind != WriteBarrierKind::kEmitNotBeingReliedOn) << "We shouldn't go back to the original value."; - DCHECK(kind != WriteBarrierKind::kEmitWithNullCheck) - << "We never do the null check optimization for ArraySets."; + DCHECK_IMPLIES(kind == WriteBarrierKind::kDontEmit, + GetWriteBarrierKind() != WriteBarrierKind::kEmitBeingReliedOn) + << "If a write barrier was relied on by other write barriers, we cannot skip emitting it."; SetPackedField(kind); } @@ -7516,7 +7517,7 @@ class HStaticFieldSet final : public HExpression<2> { declaring_class_def_index, dex_file) { SetPackedFlag(true); - SetPackedField(WriteBarrierKind::kEmitWithNullCheck); + SetPackedField(WriteBarrierKind::kEmitNotBeingReliedOn); SetRawInputAt(0, cls); SetRawInputAt(1, value); } @@ -7534,8 +7535,11 @@ class HStaticFieldSet final : public HExpression<2> { WriteBarrierKind GetWriteBarrierKind() { return GetPackedField(); } void SetWriteBarrierKind(WriteBarrierKind kind) { - DCHECK(kind != WriteBarrierKind::kEmitWithNullCheck) + DCHECK(kind != WriteBarrierKind::kEmitNotBeingReliedOn) << "We shouldn't go back to the original value."; + DCHECK_IMPLIES(kind == WriteBarrierKind::kDontEmit, + GetWriteBarrierKind() != WriteBarrierKind::kEmitBeingReliedOn) + << "If a write barrier was relied on by other write barriers, we cannot skip emitting it."; SetPackedField(kind); } -- cgit v1.2.3-59-g8ed1b