compiler: Enhance IR to support more advanced parlang (CUDA/HIP/SYCL) features #2840
compiler: Enhance IR to support more advanced parlang (CUDA/HIP/SYCL) features #2840FabioLuporini wants to merge 25 commits intomainfrom
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2840 +/- ##
========================================
Coverage 78.97% 78.97%
========================================
Files 248 248
Lines 50901 51003 +102
Branches 4395 4403 +8
========================================
+ Hits 40199 40280 +81
- Misses 9904 9921 +17
- Partials 798 802 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mloubout
left a comment
mloubout
left a comment
There was a problem hiding this comment.
Mintoi comments but was already reviewed in the previous one so looks good
devito/ir/clusters/cluster.py
Outdated
| return {i for i in self.free_symbols if i.is_Dimension} | idims | ||
| dims_exprs = {i for i in self.free_symbols if i.is_Dimension} | ||
|
|
||
| dims_implicit = set().union(*[set(e.implicit_dims) for e in self.exprs]) |
There was a problem hiding this comment.
don't need set(e.implicit_dims) just e.implicit_dims
There was a problem hiding this comment.
Could even just be dims_implicit = {d for e in self.exprs for d in e.implicit_dims}
There was a problem hiding this comment.
ok fixing, Ed has a point
| sub_iterators = dict([(k, tuple(filter_ordered(as_tuple(v)))) | ||
| for k, v in (sub_iterators or {}).items()]) | ||
| sub_iterators = sub_iterators or {} | ||
| sub_iterators = {d: tuple(filter_ordered(as_tuple(v))) |
There was a problem hiding this comment.
That's quite a lot of tuple conversion but doubt can be changed
devito/ir/clusters/cluster.py
Outdated
| return {i for i in self.free_symbols if i.is_Dimension} | idims | ||
| dims_exprs = {i for i in self.free_symbols if i.is_Dimension} | ||
|
|
||
| dims_implicit = set().union(*[set(e.implicit_dims) for e in self.exprs]) |
There was a problem hiding this comment.
Could even just be dims_implicit = {d for e in self.exprs for d in e.implicit_dims}
devito/ir/clusters/cluster.py
Outdated
|
|
||
| dims_implicit = set().union(*[set(e.implicit_dims) for e in self.exprs]) | ||
|
|
||
| syms_guards = set().union(*[e.free_symbols for e in self.guards.values()]) |
There was a problem hiding this comment.
You can do the same with this one, which will improve readability and homogeneity
devito/symbolics/extended_sympy.py
Outdated
| class Terminal: | ||
|
|
||
| """ | ||
| Abstract base class for all terminal objects, that is, those objects |
There was a problem hiding this comment.
This docstring seems a little tautologous - a terminal is an object collected by retrieve_terminals which is a function that retrieves objects that are terminals. Might be worth noting what makes an object "terminal"? I'm guessing anything which is always a leaf node with no children?
There was a problem hiding this comment.
tweaking it
btw:
a leaf node with no children
by definition a leaf node has no children
There was a problem hiding this comment.
You know what I was saying about tautology...
devito/symbolics/extended_sympy.py
Outdated
|
|
||
| Reserved objects have the following properties: | ||
|
|
||
| * `estimate_cost(o) = 0`, where `o` is an instance of Reserved |
There was a problem hiding this comment.
Is this enforced by a subclass or superclass?
There was a problem hiding this comment.
I will delete that bit as it's irrelevant and perhaps confusing
FabioLuporini
force-pushed
the
the-TMA
branch
from
206c29c to
1213648
Compare
3 participants
No description provided.