compiler: Add a check to pre-empt expensive symbolic comparisons before try-except

Author: EdCaunt

devito/ir/clusters/algorithms.py

@@ -156,8 +156,6 @@ def callback(self, clusters, prefix, backlog=None, known_break=None):
         # Schedule Clusters over different IterationSpaces if this increases
         # parallelism
         for i in range(1, len(clusters)):
-            # FIXME: This eats a lot of time (four seconds each time)
-            # FIXME: Pull scope out of this
             if self._break_for_parallelism(scope, candidates, i):
                 return self.callback(clusters[:i], prefix, clusters[i:] + backlog,
                                      candidates | known_break)
@@ -193,9 +191,6 @@ def callback(self, clusters, prefix, backlog=None, known_break=None):
     def _break_for_parallelism(self, scope, candidates, i):
         # `test` will be True if there's at least one data-dependence that would
         # break parallelism
-
-        # TODO: Can this loop be made to short-circuit?
-        # TODO: Most of the time is burned in d_from_access_gen
         test = False
         for d in scope.d_from_access_gen(scope.a_query(i)):
             if d.is_local or d.is_storage_related(candidates):

devito/ir/support/basic.py

@@ -317,7 +317,7 @@ def lex_le(self, other):
     def lex_lt(self, other):
         return self.timestamp < other.timestamp
 
-    # NOTE: This is called a lot with the same arguments - memoize yields mild speedup
+    # Note: memoization yields mild compiler speedup
     @memoized_meth
     def distance(self, other):
         """
@@ -365,14 +365,21 @@ def distance(self, other):
                 # Case 1: `sit` is an IterationInterval with statically known
                 # trip count. E.g. it ranges from 0 to 3; `other` performs a
                 # constant access at 4
-                for v in (self[n], other[n]):
-                    try:
-                        # NOTE: Split the boolean to make the conditional short circuit
-                        # more frequently for mild speedup
-                        if bool(v < sit.symbolic_min) or bool(v > sit.symbolic_max):
-                            return Vector(S.ImaginaryUnit)
-                    except TypeError:
-                        pass
+
+                # To avoid evaluating expensive symbolic Lt or Gt operations,
+                # we pre-empt such operations by checking if the values to be compared
+                # to are symbolic, and skip this case if not.
+                if not any(isinstance(i, sympy.core.Basic)
+                           for i in (sit.symbolic_min, sit.symbolic_max)):
+
+                    for v in (self[n], other[n]):
+                        try:
+                            # Note: Boolean is split to make the conditional short
+                            # circuit more frequently for mild speedup
+                            if bool(v < sit.symbolic_min) or bool(v > sit.symbolic_max):
+                                return Vector(S.ImaginaryUnit)
+                        except TypeError:
+                            pass
 
                 # Case 2: `sit` is an IterationInterval over a local SubDimension
                 # and `other` performs a constant access
@@ -1170,7 +1177,6 @@ def d_from_access_gen(self, accesses):
         Generate all flow, anti, and output dependences involving any of
         the given TimedAccess objects.
         """
-        # FIXME: This seems to be a hotspot
         accesses = as_tuple(accesses)
         for d in self.d_all_gen():
             for i in accesses:

devito/operator/operator.py

@@ -967,10 +967,8 @@ def _emit_build_profiling(self):
         tot = timings.pop('op-compile')
         perf(f"Operator `{self.name}` generated in {fround(tot):.2f} s")
 
-        # max_hotspots = 3
-        # threshold = 20.
-        max_hotspots = 300
-        threshold = 0.5
+        max_hotspots = 3
+        threshold = 20.
 
         def _emit_timings(timings, indent=''):
             timings.pop('total', None)

devito/passes/clusters/cse.py

@@ -234,7 +234,6 @@ def _compact(exprs, exclude):
 
     # If there are ctemps in the expressions, then add any to the mapper which only
     # appear once
-    # TODO: Double check this is exactly the prior behaviour
     if ctemps:
         mapper.update({e.lhs: e.rhs for e in candidates
                        if ctemp_count[e.lhs] == 1})

devito/passes/clusters/misc.py

@@ -362,7 +362,6 @@ def is_cross(source, sink):
                 # * All ClusterGroups between `cg0` and `cg1` must precede `cg1`
                 # * All ClusterGroups after `cg1` cannot precede `cg1`
 
-                # FIXME: Slow
                 if any(i.cause & prefix for i in scope.d_anti_gen()):
                     for cg2 in cgroups[n:cgroups.index(cg1)]:
                         dag.add_edge(cg2, cg1)

devito/passes/iet/mpi.py

@@ -24,7 +24,6 @@ def optimize_halospots(iet, **kwargs):
     merged and moved around in order to improve the halo exchange performance.
     """
     iet = _drop_reduction_halospots(iet)
-    # TODO: _hoist_invariant and _merge_halospots are both slow
     iet = _hoist_invariant(iet)
     iet = _merge_halospots(iet)
     iet = _drop_if_unwritten(iet, **kwargs)
@@ -82,8 +81,6 @@ def _hoist_invariant(iet):
     cond_mapper = _make_cond_mapper(iet)
     iter_mapper = _filter_iter_mapper(iet)
 
-    # from IPython import embed; embed()
-
     for it, halo_spots in iter_mapper.items():
         for hs0, hs1 in combinations(halo_spots, r=2):
             if _check_control_flow(hs0, hs1, cond_mapper):
@@ -160,7 +157,6 @@ def _merge_halospots(iet):
 
             for f, v in hs1.fmapper.items():
                 for dep in scope.d_flow.project(f):
-                    # NOTE: The else clause gets skipped if this is ever true
                     if not any(rule(dep, hs1, v.loc_indices) for rule in rules):
                         break
                 else: