celerity / celerity-runtime

High-level C++ for Accelerator Clusters
https://celerity.github.io
MIT License
139 stars 18 forks source link

Region Map support extents with non-zero offset; make dimensionality implicit; fix empty queries #242

Closed fknorr closed 7 months ago

fknorr commented 7 months ago

This is a back-port from IDAG development.

The instruction graph generator tracks buffer accesses on a per-allocation basis, where each allocation has a potentially non-zero offset. With the existing region map API, this either requires maintaining rectangles within the "offset border" that are never queried, or a coordinate transformation in the generator. Fortunately region_map_impl already works on a box<Dims> extent, so we can just pass a box instead of a range to the region_map ctor.

Also, the implementation dimensionality is currently passed as an explicit ctor parameter to region_map, when since #201 , the minimum / effective dimensionality can be inferred from a range or box automatically. This avoids the need for passing int dims everywhere and has the benefit of making a buffer<int, 3>(range(1000, 1, 1)) exactly as expensive to track as a buffer<int, 1>(1000).

Lastly, the region_map_impl<0> specialization was behaving inconsistently around queries with empty boxes (aka box<3>({0, 0, 0}, {0, 0, 0})). This box cannot be cast to box<0> (which always has size 1). The incorrect behavior was even expected in an unit test - this was brought back in line with other dimensionalities; I've also added a more specific test.

github-actions[bot] commented 7 months ago

Check-perf-impact results: (4c65f1399a47e0eb1340f63004745b17)

:question: No new benchmark data submitted. :question:
Please re-run the microbenchmarks and include the results if your commit could potentially affect performance.