AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix write-after-read in clustering

Browse Source 
This fixes the `discard` gm that demonstrates the write-after-read.

Bug: skia:10877
Change-Id: I631b7626a47d046bb5f842e997f50dfec50649b3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360606
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Auto-Submit: Adlai Holler <adlai@google.com>
This commit is contained in:
Adlai Holler 2021-01-28 13:23:13 -05:00 committed by Skia Commit-Bot
parent 0492a744a5
commit b5dda505cb
2 changed files with 59 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
src/gpu/GrRenderTaskCluster.cpp
View File

@ -62,6 +62,32 @@ static void validate(SkSpan<const sk_sp<GrRenderTask>> input,
#endif // SK_DEBUG
// Returns whether `dependee` is a formal dependent or if it uses a surface `depender` targets.
static bool depends_on(GrRenderTask* depender, GrRenderTask* dependee) {
// Check if depender writes to something dependee reads.
// TODO: Establish real DAG dependencies for this during recording? E.g. when a task adds a
// target, search backward for the last task that uses the target and add a dep.
for (int i = 0; i < depender->numTargets(); i++) {
if (dependee->isUsed(depender->target(i))) {
CLUSTER_DEBUGF("Cluster: Bail, %s can't write before %s reads from %s.\n",
describe_task(depender).c_str(),
describe_task(dependee).c_str(),
depender->target(i)->getDebugName().c_str());
return true;
}
}
// Check for a formal dependency.
for (GrRenderTask* t : depender->dependencies()) {
if (dependee == t) {
CLUSTER_DEBUGF("Cluster: Bail, %s depends on %s.\n",
describe_task(depender).c_str(),
describe_task(dependee).c_str());
return true;
}
}
return false;
}
// Returns whether reordering occurred.
static bool task_cluster_visit(GrRenderTask* task, SkTInternalLList<GrRenderTask>* llist,
SkTHashMap<GrSurfaceProxy*, GrRenderTask*>* lastTaskMap) {
@ -107,16 +133,11 @@ static bool task_cluster_visit(GrRenderTask* task, SkTInternalLList<GrRenderTask
}
// We can't reorder if any moved task depends on anything in the cluster.
// Time complexity here is high, but making a hash set is a lot worse.
// Time complexity here is high, but making a hash set is worse in profiling.
for (GrRenderTask* moved = movedHead; moved; moved = moved->fNext) {
for (GrRenderTask* dep : moved->dependencies()) {
for (GrRenderTask* t = clusterHead; t != movedHead; t = t->fNext) {
if (t == dep) {
CLUSTER_DEBUGF("Cluster: Bail, %s depends on %s.\n",
describe_task(moved).c_str(),
describe_task(dep).c_str());
return false;
}
for (GrRenderTask* passed = clusterHead; passed != movedHead; passed = passed->fNext) {
if (depends_on(moved, passed)) {
return false;
}
}
}

30
tests/GrRenderTaskClusterTest.cpp
View File

@ -92,11 +92,39 @@ static void create_graph2(SkTArray<sk_sp<GrMockRenderTask>>* graph,
// expected is empty. Can't reorder.
}
/*
* Write-after-read case.
* In: A1 B1 A2 B2
* Srcs: A1->B1, A2->B2
* Used: B1(A), B2(A)
* Out: Can't reorder.
*/
static void create_graph3(SkTArray<sk_sp<GrMockRenderTask>>* graph,
SkTArray<sk_sp<GrMockRenderTask>>* expected) {
SkTArray<sk_sp<GrSurfaceProxy>> proxies;
make_proxies(2, &proxies);
make_tasks(4, graph);
graph->at(0)->addTarget(proxies[0]);
graph->at(1)->addTarget(proxies[1]);
graph->at(2)->addTarget(proxies[0]);
graph->at(3)->addTarget(proxies[1]);
graph->at(1)->addDependency(graph->at(0).get());
graph->at(3)->addDependency(graph->at(2).get());
graph->at(1)->addUsed(proxies[0]);
graph->at(3)->addUsed(proxies[0]);
// expected is empty. Can't reorder.
}
DEF_TEST(GrRenderTaskCluster, reporter) {
CreateGraphPF tests[] = {
create_graph0,
create_graph1,
create_graph2
create_graph2,
create_graph3
};
for (size_t i = 0; i < SK_ARRAY_COUNT(tests); ++i) {