compiler: Improve quality of generated code with Bundles and MPI #2587
Conversation
FabioLuporini
requested review from
EdCaunt,
JDBetteridge,
georgebisbas and
mloubout
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2587 +/- ##
==========================================
- Coverage 92.00% 91.88% -0.12%
==========================================
Files 244 245 +1
Lines 47915 48113 +198
Branches 4222 4235 +13
==========================================
+ Hits 44082 44210 +128
- Misses 3159 3229 +70
Partials 674 674
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| if expr.dtype: | ||
| # CXX, unlike C99, does not support compound literals | ||
| tstr = dtype_to_cstr(expr.dtype) | ||
| return f'std::array<{tstr}, {len(expr.params)}>{li}.data()' |
There was a problem hiding this comment.
I take it back -- I cannot. Because of the PRO issue that currently using CBB isntead of CXXBB
There was a problem hiding this comment.
Not sure how it's related this only sets the std:: part. And the Pro should be switched to CXXBB for the cpp languages too.
There was a problem hiding this comment.
I admit I didn't really have the mental strength to chase absolutely everything, but this is what I saw for some reason:
https://github.com/devitocodespro/devitopro/blob/main/devitopro/passes/languages/cuda.py#L184
and it's probably related...
devito/ir/support/basic.py
Outdated
| if q_comp_acc(self.source.access) and not q_comp_acc(self.sink.access): | ||
| # E.g., `source=ab[x].x` and `sink=ab[x]` -> `a(x)` | ||
| return self.source.access.function_access | ||
| elif q_comp_acc(self.sink.access) and not q_comp_acc(self.source.access): |
There was a problem hiding this comment.
nitpicking: keep same condition order so not q_comp_acc(self.source.access) and q_comp_acc(self.sink.access)
devito/passes/iet/engine.py
Outdated
| if not isinstance(efunc, (HaloUpdate, HaloWait)): | ||
| continue | ||
|
|
||
| calls = FindNodes((Gather, Scatter)).visit(efunc) |
There was a problem hiding this comment.
good catch, I'm going to improve this bit
| try: | ||
| return issubclass(self.dtype, np.number) | ||
| except TypeError: | ||
| return self.dtype in ctypes_vector_mapper |
There was a problem hiding this comment.
That seems oddly specific here.
There was a problem hiding this comment.
it's in main too
| """ | ||
|
|
||
| def _print_ComponentAccess(self, expr): | ||
| return f"{self._print(expr.base)}.<{expr.sindex},{expr.function.ncomp}>" |
There was a problem hiding this comment.
Why do you need that? How is it printed without it?
There was a problem hiding this comment.
it'd be printed without this: {expr.function.ncomp}
meaning that two apparently identical kernels in which f.x appears would be syntactically identical even though in one case f has 3 components for example and in the other 4.
| @property | ||
| def view(self): | ||
| """A representation of the IET rooted in ``self``.""" | ||
| """A high-level representation of the IET rooted in `self`.""" |
There was a problem hiding this comment.
Ultra-nitpick: I think our convention is to only use a full stop for multiline docstrings
There was a problem hiding this comment.
I thought that was changing towards full stop everywhere in docstrings? IIRC (but I could well be wrong) it's only the code comments that spare the full stop
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
FabioLuporini
force-pushed
the
hotfix-parlang-lowering-3
branch
from
8d1c43d to
126a0fd
Compare
FabioLuporini
force-pushed
the
hotfix-parlang-lowering-3
branch
from
68ac98d to
2b98129
Compare
3 participants
All of the juicy tests are in PRO ... because that's where Bundles are created