mirror of
https://github.com/dart-lang/sdk
synced 2024-10-07 09:11:55 +00:00
[vm/compiler] Fix incorrect write-barrier elimination
If there is a GC-triggering instruction between array allocation and store into array we cannot omit the barrier. This worked for stores where the receiver is a direct array allocation, but not if it's a phi. Closes https://github.com/dart-lang/sdk/issues/43786 Closes https://github.com/dart-lang/sdk/issues/43770 Change-Id: I28de29cf85842a9d3ae3189bb8e6426969fe4279 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/167570 Commit-Queue: Martin Kustermann <kustermann@google.com> Reviewed-by: Alexander Markov <alexmarkov@google.com> Reviewed-by: Ryan Macnak <rmacnak@google.com> Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
This commit is contained in:
parent
1f3c76ccdd
commit
32dfefbea1
|
@ -192,6 +192,8 @@ void WriteBarrierElimination::SaveResults() {
|
||||||
void WriteBarrierElimination::IndexDefinitions(Zone* zone) {
|
void WriteBarrierElimination::IndexDefinitions(Zone* zone) {
|
||||||
BitmapBuilder array_allocations;
|
BitmapBuilder array_allocations;
|
||||||
|
|
||||||
|
GrowableArray<Definition*> create_array_worklist;
|
||||||
|
|
||||||
for (intptr_t i = 0; i < block_order_->length(); ++i) {
|
for (intptr_t i = 0; i < block_order_->length(); ++i) {
|
||||||
BlockEntryInstr* const block = block_order_->At(i);
|
BlockEntryInstr* const block = block_order_->At(i);
|
||||||
if (auto join_block = block->AsJoinEntry()) {
|
if (auto join_block = block->AsJoinEntry()) {
|
||||||
|
@ -209,8 +211,12 @@ void WriteBarrierElimination::IndexDefinitions(Zone* zone) {
|
||||||
for (ForwardInstructionIterator it(block); !it.Done(); it.Advance()) {
|
for (ForwardInstructionIterator it(block); !it.Done(); it.Advance()) {
|
||||||
if (Definition* current = it.Current()->AsDefinition()) {
|
if (Definition* current = it.Current()->AsDefinition()) {
|
||||||
if (IsUsable(current)) {
|
if (IsUsable(current)) {
|
||||||
array_allocations.Set(definition_count_, current->IsCreateArray());
|
const bool is_create_array = current->IsCreateArray();
|
||||||
|
array_allocations.Set(definition_count_, is_create_array);
|
||||||
definition_indices_.Insert({current, definition_count_++});
|
definition_indices_.Insert({current, definition_count_++});
|
||||||
|
if (is_create_array) {
|
||||||
|
create_array_worklist.Add(current);
|
||||||
|
}
|
||||||
#if defined(DEBUG)
|
#if defined(DEBUG)
|
||||||
if (tracing_) {
|
if (tracing_) {
|
||||||
THR_Print("Definition (%" Pd ") has index %" Pd ".\n",
|
THR_Print("Definition (%" Pd ") has index %" Pd ".\n",
|
||||||
|
@ -222,6 +228,20 @@ void WriteBarrierElimination::IndexDefinitions(Zone* zone) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
while (!create_array_worklist.is_empty()) {
|
||||||
|
auto instr = create_array_worklist.RemoveLast();
|
||||||
|
for (Value::Iterator it(instr->input_use_list()); !it.Done();
|
||||||
|
it.Advance()) {
|
||||||
|
if (auto phi_use = it.Current()->instruction()->AsPhi()) {
|
||||||
|
const intptr_t index = Index(phi_use);
|
||||||
|
if (!array_allocations.Get(index)) {
|
||||||
|
array_allocations.Set(index, /*can_be_create_array=*/true);
|
||||||
|
create_array_worklist.Add(phi_use);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
vector_ = new (zone) BitVector(zone, definition_count_);
|
vector_ = new (zone) BitVector(zone, definition_count_);
|
||||||
vector_->SetAll();
|
vector_->SetAll();
|
||||||
array_allocations_mask_ = new (zone) BitVector(zone, definition_count_);
|
array_allocations_mask_ = new (zone) BitVector(zone, definition_count_);
|
||||||
|
|
|
@ -12,7 +12,8 @@ namespace dart {
|
||||||
DEBUG_ONLY(DECLARE_FLAG(bool, trace_write_barrier_elimination);)
|
DEBUG_ONLY(DECLARE_FLAG(bool, trace_write_barrier_elimination);)
|
||||||
|
|
||||||
ISOLATE_UNIT_TEST_CASE(IRTest_WriteBarrierElimination_JoinSuccessors) {
|
ISOLATE_UNIT_TEST_CASE(IRTest_WriteBarrierElimination_JoinSuccessors) {
|
||||||
DEBUG_ONLY(FLAG_trace_write_barrier_elimination = true);
|
DEBUG_ONLY(
|
||||||
|
SetFlagScope<bool> sfs(&FLAG_trace_write_barrier_elimination, true));
|
||||||
const char* nullable_tag = TestCase::NullableTag();
|
const char* nullable_tag = TestCase::NullableTag();
|
||||||
const char* null_assert_tag = TestCase::NullAssertTag();
|
const char* null_assert_tag = TestCase::NullAssertTag();
|
||||||
|
|
||||||
|
@ -82,7 +83,8 @@ ISOLATE_UNIT_TEST_CASE(IRTest_WriteBarrierElimination_JoinSuccessors) {
|
||||||
}
|
}
|
||||||
|
|
||||||
ISOLATE_UNIT_TEST_CASE(IRTest_WriteBarrierElimination_AtLeastOnce) {
|
ISOLATE_UNIT_TEST_CASE(IRTest_WriteBarrierElimination_AtLeastOnce) {
|
||||||
DEBUG_ONLY(FLAG_trace_write_barrier_elimination = true);
|
DEBUG_ONLY(
|
||||||
|
SetFlagScope<bool> sfs(&FLAG_trace_write_barrier_elimination, true));
|
||||||
// Ensure that we process every block at least once during the analysis
|
// Ensure that we process every block at least once during the analysis
|
||||||
// phase so that the out-sets will be initialized. If we don't process
|
// phase so that the out-sets will be initialized. If we don't process
|
||||||
// each block at least once, the store "c.next = n" will be marked
|
// each block at least once, the store "c.next = n" will be marked
|
||||||
|
@ -137,7 +139,8 @@ ISOLATE_UNIT_TEST_CASE(IRTest_WriteBarrierElimination_AtLeastOnce) {
|
||||||
}
|
}
|
||||||
|
|
||||||
ISOLATE_UNIT_TEST_CASE(IRTest_WriteBarrierElimination_Arrays) {
|
ISOLATE_UNIT_TEST_CASE(IRTest_WriteBarrierElimination_Arrays) {
|
||||||
DEBUG_ONLY(FLAG_trace_write_barrier_elimination = true);
|
DEBUG_ONLY(
|
||||||
|
SetFlagScope<bool> sfs(&FLAG_trace_write_barrier_elimination, true));
|
||||||
const char* nullable_tag = TestCase::NullableTag();
|
const char* nullable_tag = TestCase::NullableTag();
|
||||||
|
|
||||||
// Test that array allocations are not considered usable after a
|
// Test that array allocations are not considered usable after a
|
||||||
|
@ -204,4 +207,49 @@ ISOLATE_UNIT_TEST_CASE(IRTest_WriteBarrierElimination_Arrays) {
|
||||||
EXPECT(store_into_array->ShouldEmitStoreBarrier() == true);
|
EXPECT(store_into_array->ShouldEmitStoreBarrier() == true);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
ISOLATE_UNIT_TEST_CASE(IRTest_WriteBarrierElimination_Regress43786) {
|
||||||
|
DEBUG_ONLY(
|
||||||
|
SetFlagScope<bool> sfs(&FLAG_trace_write_barrier_elimination, true));
|
||||||
|
const char* kScript = R"(
|
||||||
|
foo() {
|
||||||
|
final root = List<dynamic>.filled(128, null);
|
||||||
|
List<dynamic> last = root;
|
||||||
|
for (int i = 0; i < 10 * 1024; ++i) {
|
||||||
|
final nc = List(128);
|
||||||
|
last[0] = nc;
|
||||||
|
last = nc;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
main() { foo(); }
|
||||||
|
)";
|
||||||
|
|
||||||
|
const auto& root_library = Library::Handle(LoadTestScript(kScript));
|
||||||
|
|
||||||
|
Invoke(root_library, "main");
|
||||||
|
|
||||||
|
const auto& function = Function::Handle(GetFunction(root_library, "foo"));
|
||||||
|
TestPipeline pipeline(function, CompilerPass::kJIT);
|
||||||
|
FlowGraph* flow_graph = pipeline.RunPasses({});
|
||||||
|
|
||||||
|
auto entry = flow_graph->graph_entry()->normal_entry();
|
||||||
|
EXPECT(entry != nullptr);
|
||||||
|
|
||||||
|
StoreIndexedInstr* store_into_phi = nullptr;
|
||||||
|
|
||||||
|
ILMatcher cursor(flow_graph, entry);
|
||||||
|
RELEASE_ASSERT(cursor.TryMatch(
|
||||||
|
{
|
||||||
|
kMatchAndMoveCreateArray,
|
||||||
|
kMatchAndMoveGoto,
|
||||||
|
kMatchAndMoveBranchTrue,
|
||||||
|
kMatchAndMoveCreateArray,
|
||||||
|
{kMatchAndMoveStoreIndexed, &store_into_phi},
|
||||||
|
},
|
||||||
|
kMoveGlob));
|
||||||
|
|
||||||
|
EXPECT(store_into_phi->array()->definition()->IsPhi());
|
||||||
|
EXPECT(store_into_phi->ShouldEmitStoreBarrier());
|
||||||
|
}
|
||||||
|
|
||||||
} // namespace dart
|
} // namespace dart
|
||||||
|
|
Loading…
Reference in a new issue