StanfordLegion / legion

The Legion Parallel Programming System
https://legion.stanford.edu
Apache License 2.0
676 stars 145 forks source link

Realm: Deppart Ops for GPUs #516

Open lightsighter opened 5 years ago

lightsighter commented 5 years ago

Realm only supports dependent partitioning operations with instances in CPU visible memories at the moment. Legion has a check for this now to prevent mappers from putting instances in memories where it shouldn't (only checked when running with mapper checking, e.g. -lg:safe_mapper), but that doesn't mean that it should be the long term answer. Eventually Realm should support dependent partitioning operations which can be done by the GPU when the data is placed in a GPU-visible memory.

marcinz commented 4 years ago

I have observed this issue in the wild. I use partition by image range, and my image ranges are located on GPU. As a workaround, I will modify our mapper to copy the ranges to CPU first.

mariodirenzo commented 4 years ago

I've observed this issue with my solver too. For future reference, I've applied the following patch to the default mapper as a workaround.

diff --git a/runtime/mappers/default_mapper.cc b/runtime/mappers/default_mapper.cc
index 791b79de1..cc2df59b8 100644
--- a/runtime/mappers/default_mapper.cc
+++ b/runtime/mappers/default_mapper.cc
@@ -2997,16 +2997,26 @@ namespace Legion {
         runtime->acquire_and_filter_instances(ctx,
                                           output.chosen_instances);
       // Now see if we have any fields which we still make space for
+      std::vector<unsigned> to_erase;
       std::set<FieldID> missing_fields =
         partition.requirement.privilege_fields;
       for (std::vector<PhysicalInstance>::const_iterator it =
             output.chosen_instances.begin(); it !=
             output.chosen_instances.end(); it++)
       {
-        it->remove_space_fields(missing_fields);
-        if (missing_fields.empty())
-          break;
+        if (it->get_location().kind() == Memory::GPU_FB_MEM) {
+          // These instances are not supported yet (see Legion issue #516)
+          to_erase.push_back(it - output.chosen_instances.begin());
+        } else {
+          it->remove_space_fields(missing_fields);
+          if (missing_fields.empty())
+            break;
+        }
       }
+      // Erase undesired instances
+      for (std::vector<unsigned>::const_reverse_iterator it =
+               to_erase.rbegin(); it != to_erase.rend(); it++)
+        output.chosen_instances.erase((*it) + output.chosen_instances.begin());
       // If we've satisfied all our fields, then we are done
       if (missing_fields.empty())
         return;
lightsighter commented 4 years ago

I've applied the above patch to the default mapper in the master branch.