Repository navigation
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3037 +/- ##
=======================================
Coverage 84.28% 84.29%
=======================================
Files 258 258
Lines 57469 57487 +18
Branches 4914 4912 -2
=======================================
+ Hits 48439 48459 +20
+ Misses 8206 8205 -1
+ Partials 824 823 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
mloubout
changed the title
FabioLuporini
left a comment
FabioLuporini
left a comment
There was a problem hiding this comment.
this should be handled by Scope, not with an ad-hoc solution in here
it's plausible we have to revisit the way we construct it: https://github.com/devitocodes/devito/blob/main/devito/ir/clusters/cluster.py#L159-L160
e.g. by explictly supply the syncs as a sequence of dummy equations or, probably simpler, by explicitly passing extra Scope(..., reads=...)
751250e to
ac60b1c
Compare
|
Moved to Scope |
| # Any access to a Function may depend on a wait on it | ||
| for i, e in enumerate(self.exprs): | ||
| for f in {j.function for j in e.reads_explicit}: | ||
| yield TimedAccess(mocksym_wait(f), 'R', i, e.ispace) |
There was a problem hiding this comment.
why do you need a mock symbol and not just f, or (should standalone f be problematic) f.indexed ?
There was a problem hiding this comment.
Because this is an access on the function, but without dimensions (a plain "wait on f"). If you use f or f.iindexed then you introduce dimensions and it will break most cases
| yield TimedAccess(mocksym_wait(f), 'W', 0, self.exprs[0].ispace) | ||
|
|
||
| @cached_property | ||
| def waited(self): |
There was a problem hiding this comment.
I don't think Scope should see synchronization operations as it's a different level of abstraction. Instead, these should be pulled in Cluster/ClusterGroup, and then fed to Scope as reads/writes or , if necessary (I don't think so) mock expressions
There was a problem hiding this comment.
It can't work both way. It's either in the Scope and it needs to know the syncs or it needs to be back to what it was and filtered in the clusters.
There was a problem hiding this comment.
it still in the Scope; but it's rather passed to Scope as further read/written Functions
in ir/clusters/cluster.py , look for def scope and change its implementation to also include such syncs , that's basically what I was suggesting
58a6bcf to
2176a13
Compare
| TimedAccesses, to be fed to a Scope. | ||
| """ | ||
| waited = {s.target for s in flatten(syncs.values()) if isinstance(s, WaitLock)} | ||
| writes = [TimedAccess(mocksym_wait(f.name), 'W', 0, exprs[0].ispace) |
There was a problem hiding this comment.
why not just f.indexed?
There was a problem hiding this comment.
probably safer to pass in self instead of exprs/syncs, and rather use self.ispace (ie cluster.ispace) instead of exprs[0].ispace
we may also consider calling it extract_accesses instead of wait_accesses since in the future we might need generalisations
| TimedAccesses, to be fed to a Scope. | ||
| """ | ||
| waited = {s.target for s in flatten(syncs.values()) if isinstance(s, WaitLock)} | ||
| writes = [TimedAccess(mocksym_wait(f.name), 'W', 0, exprs[0].ispace) |
There was a problem hiding this comment.
probably safer to pass in self instead of exprs/syncs, and rather use self.ispace (ie cluster.ispace) instead of exprs[0].ispace
we may also consider calling it extract_accesses instead of wait_accesses since in the future we might need generalisations
| waited = {s.target for s in flatten(syncs.values()) if isinstance(s, WaitLock)} | ||
| writes = [TimedAccess(mocksym_wait(f.name), 'W', 0, exprs[0].ispace) | ||
| for f in waited if exprs] | ||
| reads = [TimedAccess(mocksym_wait(f.name), 'R', i, e.ispace) |
There was a problem hiding this comment.
again, f.indexed should suffice
2176a13 to
6d2ad82
Compare
Fusion's toposort orders ClusterGroups by the data dependences their Scopes report, and a WaitLock, e.g. for a prefetch to complete, induced none. A reader of the prefetched buffer could then be scheduled ahead of the Cluster holding the wait, e.g. to fuse with a Cluster with the same guards, and read the previous snapshot: under the CUDA backend, the mass term of a reflection-FWI adjoint ran before the wait while its halo readers ran after. A Cluster(Group) now feeds its Scope the accesses of its syncs: a WaitLock is an index-free write of its target, on which every access to the target depends. The `reads` and `writes` of a Scope are thus accesses on top of those of its expressions.
No description provided.