Skip to content

Commit

Permalink
Rename intrusive graph dependency_kind enum values...
Browse files Browse the repository at this point in the history 
...to avoid collisions with the "TRUE" macro defined on Windows.
Always a joy running into this one!
  • Loading branch information
@psalz
psalz committed Sep 9, 2020
1 parent a51c98a commit 8de922e
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 76 deletions.
2 changes: 1 addition & 1 deletion include/command_graph.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ namespace detail {
void print_graph(logger& graph_logger) const;

// TODO unify dependency terminology to this
void add_dependency(abstract_command* depender, abstract_command* dependee, dependency_kind kind = dependency_kind::TRUE) {
void add_dependency(abstract_command* depender, abstract_command* dependee, dependency_kind kind = dependency_kind::TRUE_DEP) {
assert(depender->get_nid() == dependee->get_nid()); // We cannot depend on commands executed on another node!
assert(dependee != depender);
depender->add_dependency({dependee, kind});
Expand Down
6 changes: 3 additions & 3 deletions include/intrusive_graph.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ namespace celerity {
namespace detail {

enum class dependency_kind {
ANTI = 0, // Data anti-dependency, can be resolved by duplicating buffers
ORDER = 1, // Order pseudo-depencency, introduced by collective host task groups
TRUE = 2, // True data flow dependency
ANTI_DEP = 0, // Data anti-dependency, can be resolved by duplicating buffers
ORDER_DEP = 1, // Order pseudo-depencency, introduced by collective host task groups
TRUE_DEP = 2, // True data flow dependency
};

template <typename T>
Expand Down
12 changes: 6 additions & 6 deletions src/graph_generator.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ namespace detail {
// they are executed in the same order on every node.
auto cgid = tsk->get_collective_group_id();
if(auto prev = last_collective_commands.find({nid, cgid}); prev != last_collective_commands.end()) {
cdag.add_dependency(cmd, cdag.get(prev->second), dependency_kind::ORDER);
cdag.add_dependency(cmd, cdag.get(prev->second), dependency_kind::ORDER_DEP);
last_collective_commands.erase(prev);
}
last_collective_commands.emplace(std::pair{nid, cgid}, cmd->get_cid());
Expand Down Expand Up @@ -100,7 +100,7 @@ namespace detail {
bool has_dependents = false;
for(auto d : last_writer_cmd->get_dependents()) {
// Only consider true dependencies
if(d.kind != dependency_kind::TRUE) continue;
if(d.kind != dependency_kind::TRUE_DEP) continue;

const auto cmd = d.node;

Expand All @@ -113,7 +113,7 @@ namespace detail {
if(buffer_reads_it == command_reads.end()) continue; // The task might be a dependent because of another buffer
if(!GridRegion<3>::intersect(write_req, buffer_reads_it->second).empty()) {
has_dependents = true;
cdag.add_dependency(write_cmd, cmd, dependency_kind::ANTI);
cdag.add_dependency(write_cmd, cmd, dependency_kind::ANTI_DEP);
}
}

Expand All @@ -123,7 +123,7 @@ namespace detail {
// Don't add anti-dependencies onto the init command
if(last_writer_cid == node_data[write_cmd->get_nid()].init_cid) continue;

cdag.add_dependency(write_cmd, last_writer_cmd, dependency_kind::ANTI);
cdag.add_dependency(write_cmd, last_writer_cmd, dependency_kind::ANTI_DEP);

// This is a good time to validate our assumption that every AWAIT_PUSH command has a dependent
assert(!isa<await_push_command>(last_writer_cmd));
Expand Down Expand Up @@ -283,12 +283,12 @@ namespace detail {
if(isa<task_command>(writer_cmd) && static_cast<task_command*>(writer_cmd)->get_tid() == tid) {
// In certain situations the PUSH might have a true dependency on the last writer,
// in that case don't add an anti-dependency (as that would cause a cycle).
if(cmd->has_dependency(writer_cmd, dependency_kind::TRUE)) {
if(cmd->has_dependency(writer_cmd, dependency_kind::TRUE_DEP)) {
// This can currently only happen for AWAIT_PUSH commands.
assert(isa<await_push_command>(writer_cmd));
continue;
}
cdag.add_dependency(writer_cmd, push_cmd, dependency_kind::ANTI);
cdag.add_dependency(writer_cmd, push_cmd, dependency_kind::ANTI_DEP);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/print_graph.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ namespace detail {

const char* dependency_style(dependency_kind kind) {
switch(kind) {
case dependency_kind::ORDER: return "color=blue"; break;
case dependency_kind::ANTI: return "color=limegreen"; break;
case dependency_kind::ORDER_DEP: return "color=blue"; break;
case dependency_kind::ANTI_DEP: return "color=limegreen"; break;
default: return "";
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/task_manager.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ namespace detail {
// A valid use case (i.e., not reading garbage) for this is when the buffer has been initialized using a host pointer.
if(p.second == std::nullopt) continue;
const task_id last_writer = *p.second;
tsk->add_dependency({task_map[last_writer].get(), dependency_kind::TRUE});
tsk->add_dependency({task_map[last_writer].get(), dependency_kind::TRUE_DEP});
}
}

Expand Down Expand Up @@ -101,7 +101,7 @@ namespace detail {
dependent.node, bid, {detail::access::consumer_modes.cbegin(), detail::access::consumer_modes.cend()}, num_collective_nodes);
// Only add an anti-dependency if we are really writing over the region read by this task
if(!GridRegion<3>::intersect(write_requirements, dependent_read_requirements).empty()) {
tsk->add_dependency({dependent.node, dependency_kind::ANTI});
tsk->add_dependency({dependent.node, dependency_kind::ANTI_DEP});
has_anti_dependents = true;
}
}
Expand All @@ -112,7 +112,7 @@ namespace detail {
// While it might not always make total sense to have anti-dependencies between (pure) producers without an
// intermediate consumer, we at least have a defined behavior, and the thus enforced ordering of tasks
// likely reflects what the user expects.
tsk->add_dependency({last_writer.get(), dependency_kind::ANTI});
tsk->add_dependency({last_writer.get(), dependency_kind::ANTI_DEP});
}
}

Expand All @@ -122,7 +122,7 @@ namespace detail {

if(auto cgid = tsk->get_collective_group_id(); cgid != 0) {
if(auto prev = last_collective_tasks.find(cgid); prev != last_collective_tasks.end()) {
tsk->add_dependency({task_map.at(prev->second).get(), dependency_kind::ORDER});
tsk->add_dependency({task_map.at(prev->second).get(), dependency_kind::ORDER_DEP});
last_collective_tasks.erase(prev);
}
last_collective_tasks.emplace(cgid, tid);
Expand Down
2 changes: 1 addition & 1 deletion test/graph_compaction_tests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ namespace detail {
auto write_b_cmd = ctx.get_command_graph().get(*cmds.cbegin());
auto write_b_dependencies = write_b_cmd->get_dependencies();
CHECK(!write_b_dependencies.empty());
CHECK(write_b_dependencies.front().kind == dependency_kind::ANTI);
CHECK(write_b_dependencies.front().kind == dependency_kind::ANTI_DEP);

maybe_print_graphs(ctx);
}
Expand Down
30 changes: 15 additions & 15 deletions test/graph_generation_tests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace detail {
using celerity::access::fixed;
using celerity::access::one_to_one;

bool has_dependency(const task_manager& tm, task_id dependent, task_id dependency, dependency_kind kind = dependency_kind::TRUE) {
bool has_dependency(const task_manager& tm, task_id dependent, task_id dependency, dependency_kind kind = dependency_kind::TRUE_DEP) {
for(auto dep : tm.get_task(dependent)->get_dependencies()) {
if(dep.node->get_id() == dependency && dep.kind == kind) return true;
}
Expand Down Expand Up @@ -66,7 +66,7 @@ namespace detail {
buf_a.get_access<mode::discard_write>(cgh, fixed<1>({0, 128}));
buf_b.get_access<mode::discard_write>(cgh, fixed<1>({0, 128}));
});
CHECK(has_dependency(tm, tid_b, tid_a, dependency_kind::ANTI));
CHECK(has_dependency(tm, tid_b, tid_a, dependency_kind::ANTI_DEP));

const auto its = tm.get_task(tid_a)->get_dependents();
REQUIRE(std::distance(its.begin(), its.end()) == 1);
Expand All @@ -86,7 +86,7 @@ namespace detail {
buf_b.get_access<mode::discard_write>(cgh, fixed<1>({0, 128}));
});
CHECK(has_dependency(tm, tid_b, tid_a));
CHECK_FALSE(has_dependency(tm, tid_b, tid_a, dependency_kind::ANTI));
CHECK_FALSE(has_dependency(tm, tid_b, tid_a, dependency_kind::ANTI_DEP));

const auto its = tm.get_task(tid_a)->get_dependents();
REQUIRE(std::distance(its.begin(), its.end()) == 1);
Expand All @@ -104,7 +104,7 @@ namespace detail {
buf_b.get_access<mode::read>(cgh, fixed<1>({0, 128}));
});
CHECK(has_dependency(tm, tid_b, tid_a));
CHECK_FALSE(has_dependency(tm, tid_b, tid_a, dependency_kind::ANTI));
CHECK_FALSE(has_dependency(tm, tid_b, tid_a, dependency_kind::ANTI_DEP));

const auto its = tm.get_task(tid_a)->get_dependents();
REQUIRE(std::distance(its.begin(), its.end()) == 1);
Expand Down Expand Up @@ -149,14 +149,14 @@ namespace detail {
const auto tid_c = test_utils::add_compute_task<class UKN(task_c)>(tm, [&](handler& cgh) {
buf.get_access<mode::discard_write>(cgh, fixed<2, 1>({64, 64}));
});
REQUIRE(has_dependency(tm, tid_c, tid_a, dependency_kind::ANTI));
REQUIRE_FALSE(has_dependency(tm, tid_c, tid_b, dependency_kind::ANTI));
REQUIRE(has_dependency(tm, tid_c, tid_a, dependency_kind::ANTI_DEP));
REQUIRE_FALSE(has_dependency(tm, tid_c, tid_b, dependency_kind::ANTI_DEP));
// Overwrite the first half - now only an anti-dependency onto task_b should exist
const auto tid_d = test_utils::add_compute_task<class UKN(task_d)>(tm, [&](handler& cgh) {
buf.get_access<mode::discard_write>(cgh, fixed<2, 1>({0, 64}));
});
REQUIRE_FALSE(has_dependency(tm, tid_d, tid_a, dependency_kind::ANTI));
REQUIRE(has_dependency(tm, tid_d, tid_b, dependency_kind::ANTI));
REQUIRE_FALSE(has_dependency(tm, tid_d, tid_a, dependency_kind::ANTI_DEP));
REQUIRE(has_dependency(tm, tid_d, tid_b, dependency_kind::ANTI_DEP));

maybe_print_graph(tm);
}
Expand All @@ -180,12 +180,12 @@ namespace detail {
const auto tid_c = test_utils::add_compute_task<class UKN(task_c)>(tm, [&](handler& cgh) {
host_init_buf.get_access<mode::discard_write>(cgh, fixed<2, 1>({0, 128}));
});
REQUIRE(has_dependency(tm, tid_c, tid_a, dependency_kind::ANTI));
REQUIRE(has_dependency(tm, tid_c, tid_a, dependency_kind::ANTI_DEP));
const auto tid_d = test_utils::add_compute_task<class UKN(task_d)>(tm, [&](handler& cgh) {
non_host_init_buf.get_access<mode::discard_write>(cgh, fixed<2, 1>({0, 128}));
});
// Since task b is essentially reading uninitialized garbage, it doesn't make a difference if we write into it concurrently
REQUIRE_FALSE(has_dependency(tm, tid_d, tid_b, dependency_kind::ANTI));
REQUIRE_FALSE(has_dependency(tm, tid_d, tid_b, dependency_kind::ANTI_DEP));

maybe_print_graph(tm);
}
Expand Down Expand Up @@ -222,7 +222,7 @@ namespace detail {
const auto tid_b = test_utils::add_compute_task<class UKN(task_b)>(tm, [&](handler& cgh) {
buf.get_access<mode::discard_write>(cgh, fixed<2, 1>({0, 128}));
});
REQUIRE(has_dependency(tm, tid_b, tid_a, dependency_kind::ANTI));
REQUIRE(has_dependency(tm, tid_b, tid_a, dependency_kind::ANTI_DEP));
}
}

Expand Down Expand Up @@ -250,7 +250,7 @@ namespace detail {
});
const bool pure_consumer = consumer_mode == mode::read;
const bool pure_producer = producer_mode == mode::discard_read_write || producer_mode == mode::discard_write;
REQUIRE(has_dependency(tm, tid_c, tid_b, pure_consumer || pure_producer ? dependency_kind::ANTI : dependency_kind::TRUE));
REQUIRE(has_dependency(tm, tid_c, tid_b, pure_consumer || pure_producer ? dependency_kind::ANTI_DEP : dependency_kind::TRUE_DEP));
}
}
}
Expand All @@ -275,7 +275,7 @@ namespace detail {
CHECK_FALSE(has_any_dependency(tm, tid_collective_implicit_1, tid_collective_explicit_2));

CHECK_FALSE(has_any_dependency(tm, tid_collective_implicit_2, tid_master));
CHECK(has_dependency(tm, tid_collective_implicit_2, tid_collective_implicit_1, dependency_kind::ORDER));
CHECK(has_dependency(tm, tid_collective_implicit_2, tid_collective_implicit_1, dependency_kind::ORDER_DEP));
CHECK_FALSE(has_any_dependency(tm, tid_collective_implicit_2, tid_collective_explicit_1));
CHECK_FALSE(has_any_dependency(tm, tid_collective_implicit_2, tid_collective_explicit_2));

Expand All @@ -287,7 +287,7 @@ namespace detail {
CHECK_FALSE(has_any_dependency(tm, tid_collective_explicit_2, tid_master));
CHECK_FALSE(has_any_dependency(tm, tid_collective_explicit_2, tid_collective_implicit_1));
CHECK_FALSE(has_any_dependency(tm, tid_collective_explicit_2, tid_collective_implicit_2));
CHECK(has_dependency(tm, tid_collective_explicit_2, tid_collective_explicit_1, dependency_kind::ORDER));
CHECK(has_dependency(tm, tid_collective_explicit_2, tid_collective_explicit_1, dependency_kind::ORDER_DEP));
}

TEST_CASE("command_graph keeps track of created commands", "[command_graph][command-graph]") {
Expand Down Expand Up @@ -1361,7 +1361,7 @@ namespace detail {

auto has_dependencies_on_same_node = [&](task_id depender, task_id dependency) {
return all_command_dependencies(depender, dependency, [](auto depender_cmd, auto dependency_cmd) {
return depender_cmd->has_dependency(dependency_cmd, dependency_kind::ORDER) == (depender_cmd->get_nid() == dependency_cmd->get_nid());
return depender_cmd->has_dependency(dependency_cmd, dependency_kind::ORDER_DEP) == (depender_cmd->get_nid() == dependency_cmd->get_nid());
});
};

Expand Down
88 changes: 44 additions & 44 deletions test/intrusive_graph_tests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,67 +14,67 @@ namespace detail {
// Adding and removing true dependency
{
my_graph_node n0, n1;
n0.add_dependency({&n1, dependency_kind::TRUE});
REQUIRE(n0.has_dependency(&n1, dependency_kind::TRUE));
REQUIRE(n1.has_dependent(&n0, dependency_kind::TRUE));
n0.add_dependency({&n1, dependency_kind::TRUE_DEP});
REQUIRE(n0.has_dependency(&n1, dependency_kind::TRUE_DEP));
REQUIRE(n1.has_dependent(&n0, dependency_kind::TRUE_DEP));
n0.remove_dependency(&n1);
REQUIRE_FALSE(n0.has_dependency(&n1));
REQUIRE_FALSE(n1.has_dependent(&n0));
}
// Pseudo- or anti-dependency is upgraded to true dependency
{
my_graph_node n0, n1, n2;
n0.add_dependency({&n1, dependency_kind::ANTI});
n0.add_dependency({&n2, dependency_kind::ORDER});
CHECK(n0.has_dependency(&n1, dependency_kind::ANTI));
CHECK(n1.has_dependent(&n0, dependency_kind::ANTI));
CHECK(n0.has_dependency(&n2, dependency_kind::ORDER));
CHECK(n2.has_dependent(&n0, dependency_kind::ORDER));
n0.add_dependency({&n1, dependency_kind::TRUE});
n0.add_dependency({&n2, dependency_kind::ANTI});
REQUIRE_FALSE(n0.has_dependency(&n1, dependency_kind::ANTI));
REQUIRE_FALSE(n1.has_dependent(&n0, dependency_kind::ANTI));
REQUIRE(n0.has_dependency(&n1, dependency_kind::TRUE));
REQUIRE(n0.has_dependency(&n2, dependency_kind::ORDER));
REQUIRE(n2.has_dependent(&n0, dependency_kind::ORDER));
REQUIRE(n1.has_dependent(&n0, dependency_kind::TRUE));
REQUIRE_FALSE(n0.has_dependency(&n2, dependency_kind::ANTI));
REQUIRE_FALSE(n2.has_dependent(&n0, dependency_kind::ANTI));
n0.add_dependency({&n1, dependency_kind::ANTI_DEP});
n0.add_dependency({&n2, dependency_kind::ORDER_DEP});
CHECK(n0.has_dependency(&n1, dependency_kind::ANTI_DEP));
CHECK(n1.has_dependent(&n0, dependency_kind::ANTI_DEP));
CHECK(n0.has_dependency(&n2, dependency_kind::ORDER_DEP));
CHECK(n2.has_dependent(&n0, dependency_kind::ORDER_DEP));
n0.add_dependency({&n1, dependency_kind::TRUE_DEP});
n0.add_dependency({&n2, dependency_kind::ANTI_DEP});
REQUIRE_FALSE(n0.has_dependency(&n1, dependency_kind::ANTI_DEP));
REQUIRE_FALSE(n1.has_dependent(&n0, dependency_kind::ANTI_DEP));
REQUIRE(n0.has_dependency(&n1, dependency_kind::TRUE_DEP));
REQUIRE(n0.has_dependency(&n2, dependency_kind::ORDER_DEP));
REQUIRE(n2.has_dependent(&n0, dependency_kind::ORDER_DEP));
REQUIRE(n1.has_dependent(&n0, dependency_kind::TRUE_DEP));
REQUIRE_FALSE(n0.has_dependency(&n2, dependency_kind::ANTI_DEP));
REQUIRE_FALSE(n2.has_dependent(&n0, dependency_kind::ANTI_DEP));
}
}

SECTION("anti-dependencies") {
// True dependency cannot be downgraded to anti-dependency
{
my_graph_node n0, n1;
n0.add_dependency({&n1, dependency_kind::TRUE});
CHECK(n0.has_dependency(&n1, dependency_kind::TRUE));
CHECK(n1.has_dependent(&n0, dependency_kind::TRUE));
n0.add_dependency({&n1, dependency_kind::ANTI});
REQUIRE_FALSE(n0.has_dependency(&n1, dependency_kind::ANTI));
REQUIRE_FALSE(n1.has_dependent(&n0, dependency_kind::ANTI));
REQUIRE_FALSE(n0.has_dependency(&n1, dependency_kind::ORDER));
REQUIRE_FALSE(n1.has_dependent(&n0, dependency_kind::ORDER));
REQUIRE(n0.has_dependency(&n1, dependency_kind::TRUE));
REQUIRE(n1.has_dependent(&n0, dependency_kind::TRUE));
n0.add_dependency({&n1, dependency_kind::ORDER});
REQUIRE_FALSE(n0.has_dependency(&n1, dependency_kind::ANTI));
REQUIRE_FALSE(n1.has_dependent(&n0, dependency_kind::ANTI));
REQUIRE_FALSE(n0.has_dependency(&n1, dependency_kind::ORDER));
REQUIRE_FALSE(n1.has_dependent(&n0, dependency_kind::ORDER));
REQUIRE(n0.has_dependency(&n1, dependency_kind::TRUE));
REQUIRE(n1.has_dependent(&n0, dependency_kind::TRUE));
n0.add_dependency({&n1, dependency_kind::TRUE_DEP});
CHECK(n0.has_dependency(&n1, dependency_kind::TRUE_DEP));
CHECK(n1.has_dependent(&n0, dependency_kind::TRUE_DEP));
n0.add_dependency({&n1, dependency_kind::ANTI_DEP});
REQUIRE_FALSE(n0.has_dependency(&n1, dependency_kind::ANTI_DEP));
REQUIRE_FALSE(n1.has_dependent(&n0, dependency_kind::ANTI_DEP));
REQUIRE_FALSE(n0.has_dependency(&n1, dependency_kind::ORDER_DEP));
REQUIRE_FALSE(n1.has_dependent(&n0, dependency_kind::ORDER_DEP));
REQUIRE(n0.has_dependency(&n1, dependency_kind::TRUE_DEP));
REQUIRE(n1.has_dependent(&n0, dependency_kind::TRUE_DEP));
n0.add_dependency({&n1, dependency_kind::ORDER_DEP});
REQUIRE_FALSE(n0.has_dependency(&n1, dependency_kind::ANTI_DEP));
REQUIRE_FALSE(n1.has_dependent(&n0, dependency_kind::ANTI_DEP));
REQUIRE_FALSE(n0.has_dependency(&n1, dependency_kind::ORDER_DEP));
REQUIRE_FALSE(n1.has_dependent(&n0, dependency_kind::ORDER_DEP));
REQUIRE(n0.has_dependency(&n1, dependency_kind::TRUE_DEP));
REQUIRE(n1.has_dependent(&n0, dependency_kind::TRUE_DEP));
}
}
}

TEST_CASE("intrusive_graph_node removes itself from all connected nodes upon destruction", "[intrusive_graph_node]") {
my_graph_node n0, n2;
auto n1 = std::make_unique<my_graph_node>();
n0.add_dependency({n1.get(), dependency_kind::TRUE});
n1->add_dependency({&n2, dependency_kind::TRUE});
CHECK(n0.has_dependency(n1.get(), dependency_kind::TRUE));
CHECK(n2.has_dependent(n1.get(), dependency_kind::TRUE));
n0.add_dependency({n1.get(), dependency_kind::TRUE_DEP});
n1->add_dependency({&n2, dependency_kind::TRUE_DEP});
CHECK(n0.has_dependency(n1.get(), dependency_kind::TRUE_DEP));
CHECK(n2.has_dependent(n1.get(), dependency_kind::TRUE_DEP));
n1.reset();
REQUIRE(std::distance(n0.get_dependencies().begin(), n0.get_dependencies().end()) == 0);
REQUIRE(std::distance(n2.get_dependents().begin(), n2.get_dependents().end()) == 0);
Expand All @@ -83,12 +83,12 @@ namespace detail {
TEST_CASE("intrusive_graph_node keeps track of the pseudo critical path length", "[intrusive_graph_node]") {
my_graph_node n0, n1, n2, n3;
REQUIRE(n3.get_pseudo_critical_path_length() == 0);
n3.add_dependency({&n2, dependency_kind::TRUE});
n3.add_dependency({&n2, dependency_kind::TRUE_DEP});
REQUIRE(n3.get_pseudo_critical_path_length() == 1);
n3.add_dependency({&n0, dependency_kind::TRUE});
n3.add_dependency({&n0, dependency_kind::TRUE_DEP});
REQUIRE(n3.get_pseudo_critical_path_length() == 1);
n1.add_dependency({&n0, dependency_kind::TRUE});
n3.add_dependency({&n1, dependency_kind::TRUE});
n1.add_dependency({&n0, dependency_kind::TRUE_DEP});
n3.add_dependency({&n1, dependency_kind::TRUE_DEP});
REQUIRE(n3.get_pseudo_critical_path_length() == 2);
}

Expand Down

0 comments on commit 8de922e

Please sign in to comment.