compiler: Enhance IR to support more advanced parlang (CUDA/HIP/SYCL) features #2840
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2840 +/- ##
==========================================
- Coverage 78.97% 78.95% -0.02%
==========================================
Files 248 248
Lines 50936 51043 +107
Branches 4404 4414 +10
==========================================
+ Hits 40226 40303 +77
- Misses 9909 9934 +25
- Partials 801 806 +5 ☔ 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
FabioLuporini
force-pushed
the
the-TMA
branch
from
1213648 to
0bc0755
Compare
FabioLuporini
force-pushed
the
the-TMA
branch
from
0bc0755 to
03f1196
Compare
3 participants
No description provided.