Skip to content

Commit

Permalink
[vm/compiler] Follow redefinitions in Value::NeedsWriteBarrier()
Browse files Browse the repository at this point in the history
This is needed in order to account for redefinitions with less precise
type, such as AssertAssignable of Smi to a generic T.

x64, bytecode compiler:
CryptoDecrypt(RunTime): 8516.8 -> 8190.6 us

Change-Id: I8c5b8eb3efcc7ae1ccf204f5081772081be29c2f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/112755
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
  • Loading branch information
alexmarkov authored and commit-bot@chromium.org committed Aug 13, 2019
1 parent 2f02836 commit 20407e2
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 9 deletions.
29 changes: 20 additions & 9 deletions runtime/vm/compiler/backend/il.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1271,17 +1271,28 @@ void FlowGraphVisitor::VisitBlocks() {
}

bool Value::NeedsWriteBarrier() {
if (Type()->IsNull() || (Type()->ToNullableCid() == kSmiCid) ||
(Type()->ToNullableCid() == kBoolCid)) {
return false;
}
Value* value = this;
do {
if (value->Type()->IsNull() ||
(value->Type()->ToNullableCid() == kSmiCid) ||
(value->Type()->ToNullableCid() == kBoolCid)) {
return false;
}

// Strictly speaking, the incremental barrier can only be skipped for
// immediate objects (Smis) or permanent objects (vm-isolate heap or
// image pages). Here we choose to skip the barrier for any constant on
// the assumption it will remain reachable through the object pool.
// Strictly speaking, the incremental barrier can only be skipped for
// immediate objects (Smis) or permanent objects (vm-isolate heap or
// image pages). Here we choose to skip the barrier for any constant on
// the assumption it will remain reachable through the object pool.
if (value->BindsToConstant()) {
return false;
}

return !BindsToConstant();
// Follow the chain of redefinitions as redefined value could have a more
// accurate type (for example, AssertAssignable of Smi to a generic T).
value = value->definition()->RedefinedValue();
} while (value != nullptr);

return true;
}

void JoinEntryInstr::AddPredecessor(BlockEntryInstr* predecessor) {
Expand Down
45 changes: 45 additions & 0 deletions runtime/vm/compiler/backend/il_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.

#include "vm/compiler/backend/il.h"
#include "vm/compiler/backend/il_test_helper.h"
#include "vm/unit_test.h"

namespace dart {
Expand Down Expand Up @@ -40,4 +41,48 @@ ISOLATE_UNIT_TEST_CASE(OptimizationTests) {
EXPECT(!c3->Equals(c1));
}

ISOLATE_UNIT_TEST_CASE(IRTest_EliminateWriteBarrier) {
const char* kScript =
R"(
class Container<T> {
operator []=(var index, var value) {
return data[index] = value;
}
List<T> data = new List<T>()..length = 10;
}
Container<int> x = new Container<int>();
foo() {
for (int i = 0; i < 10; ++i) {
x[i] = i;
}
}
)";

const auto& root_library = Library::Handle(LoadTestScript(kScript));
const auto& function = Function::Handle(GetFunction(root_library, "foo"));

Invoke(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_indexed = nullptr;

ILMatcher cursor(flow_graph, entry, true);
RELEASE_ASSERT(cursor.TryMatch({
kMoveGlob,
kMatchAndMoveBranchTrue,
kMoveGlob,
{kMatchStoreIndexed, &store_indexed},
}));

EXPECT(!store_indexed->value()->NeedsWriteBarrier());
}

} // namespace dart

0 comments on commit 20407e2

Please sign in to comment.