Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add GDB pretty-printers for range and grid types #207

Merged
merged 3 commits into from
Nov 8, 2023
Merged

Conversation

fknorr
Copy link
Contributor

@fknorr fknorr commented Aug 28, 2023

For the following values:

int main() {
    celerity::detail::task_id tid = 10;
    celerity::detail::buffer_id bid = 11;
    celerity::detail::node_id nid = 12;
    celerity::detail::command_id cid = 13;
    celerity::detail::collective_group_id cgid = 14;
    celerity::detail::reduction_id rid = 15;
    celerity::detail::host_object_id hoid = 16;
    celerity::detail::hydration_id hyid = 17;
    celerity::detail::transfer_id trid = 18;
    
    celerity::id<3> id(1, 2, 3);
    celerity::range<3> range(1, 2, 3);
    celerity::subrange<3> subrange(celerity::id(1, 2, 3), celerity::range(4, 5, 6));
    celerity::chunk<3> chunk(celerity::id(1, 2, 3), celerity::range(4, 5, 6), celerity::range(7, 8, 9));
    celerity::nd_range<3> nd_range(celerity::range(2, 4, 6), celerity::range(1, 2, 3), celerity::id(7, 8, 9));
    celerity::detail::box<3> box(celerity::id(1, 2, 3), celerity::id(4, 5, 6));
    celerity::detail::region<3> empty_region;
    celerity::detail::region<3> region({
	      celerity::detail::box(celerity::id(1, 2, 3), celerity::id(4, 5, 6)),
	      celerity::detail::box(celerity::id(11, 2, 3), celerity::id(14, 5, 6)),
	      celerity::detail::box(celerity::id(21, 2, 3), celerity::id(24, 5, 6)),
    });
    
    celerity::detail::region_map<int> region_map(celerity::range<3>(10, 10, 10), 3);
    region_map.update_region(celerity::detail::box<3>({1, 1, 1}, {5, 5, 5}), 42);
    region_map.update_region(celerity::detail::box<3>({1, 1, 1}, {3, 3, 3}), 69);
    region_map.update_region(celerity::detail::box<3>({1, 1, 1}, {2, 2, 2}), 1337);
    
    celerity::detail::write_command_state wcs_fresh(celerity::detail::command_id(123));
    celerity::detail::write_command_state wcs_stale(celerity::detail::command_id(123));
    wcs_stale.mark_as_stale();
    celerity::detail::write_command_state wcs_replicated(celerity::detail::command_id(123), true /* replicated */);
}

The pretty-print appears as follows:

(gdb) i locals
tid = T10
bid = B11
nid = N12
cid = C13
cgid = CG14
rid = R15
hoid = H16
hyid = HY17
trid = TR18
id = [1, 2, 3]
range = [1, 2, 3]
subrange = [1, 2, 3] + [4, 5, 6]
chunk = {offset = [1, 2, 3], range = [4, 5, 6], global_size = [7, 8, 9]}
nd_range = {global_range = [2, 4, 6], local_range = [1, 2, 3], offset = [7, 8, 9]}
box = [1, 2, 3] - [4, 5, 6]
empty_region = {}
region = {[1, 2, 3] - [4, 5, 6], [11, 2, 3] - [14, 5, 6], [21, 2, 3] - [24, 5, 6]}
region_map = region_map([0, 0, 0] - [10, 10, 10]) = {[[0, 0, 0] - [10, 10, 1]] = 0, [[0, 0, 5] - [10, 10, 10]] = 0, [[0, 0, 1] - [10, 1, 5]] = 0, [[0, 5, 1] - [10, 10, 5]] = 0, [[0, 1, 1] - [1, 5, 5]] = 0, [[1, 1, 1] - [2, 2, 2]] = 1337, [[1, 1, 2] - [3, 3, 3]] = 69, [[1, 2, 1] - [3, 3, 2]] = 69, [[2, 1, 1] - [3, 2, 2]] = 69, [[1, 1, 3] - [5, 5, 5]] = 42, [[3, 1, 1] - [5, 3, 3]] = 42, [[1, 3, 1] - [5, 5, 3]] = 42, [[5, 1, 1] - [10, 5, 5]] = 0}
wcs_fresh = C123 (fresh)
wcs_stale = C123 (stale)
wcs_replicated = C123 (fresh, replicated)

The printers are auto-loaded through the root .gdbinit file together with pretty printers for gch::small_vector, but only if the work tree is configured as a safe auto-load path in GDB. This is can be configured locally as follows:

$ cat ~/.config/gdb/gdbinit
add-auto-load-safe-path /path/to/celerity-runtime

TODO

  • Add a test (checking the output of a remote controlled GDB - needs GDB in CI docker)
  • Test the appearance in GUI debuggers - how readable is the current format? (I've used this extensively during IDAG development for a month)
  • If we agree on the subrange notation, also use that in print_utils

@fknorr fknorr requested a review from psalz August 28, 2023 14:56
@fknorr fknorr self-assigned this Aug 28, 2023
@fknorr fknorr requested a review from PeterTh August 28, 2023 15:01
@celerity celerity deleted a comment from github-actions bot Aug 28, 2023
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

include/ranges.h Outdated Show resolved Hide resolved
src/grid.cc Outdated Show resolved Hide resolved
@celerity celerity deleted a comment from github-actions bot Aug 28, 2023
fknorr added a commit to fknorr/celerity-runtime that referenced this pull request Aug 28, 2023
@fknorr fknorr added this to the 0.5.0 milestone Sep 8, 2023
@fknorr fknorr force-pushed the gdb-pretty-print branch 7 times, most recently from 8f042b3 to 3d62e7b Compare September 14, 2023 07:29
@celerity celerity deleted a comment from github-actions bot Sep 14, 2023
github-actions[bot]

This comment was marked as off-topic.

@github-actions
Copy link

Check-perf-impact results: (5a19ced85f862a00d0114dd241122462)

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

github-actions[bot]

This comment was marked as off-topic.

Copy link
Contributor

@PeterTh PeterTh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. (I only looked through the code and the test, didn't try it locally)

After sourcing .gdbinit, GDB will pretty-print
    - phantom types (tast_id, node_id, ...)
    - range, subrange, chunk, nd_range
    - box, region
    - region_map
    - write_command_state
    - gch::small_vector
@github-actions
Copy link

Check-perf-impact results: (b003273516680ef3e6ca0110b3678f5e)

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

@fknorr fknorr requested a review from Luigi-Crisci November 2, 2023 13:42
@fknorr
Copy link
Contributor Author

fknorr commented Nov 8, 2023

Since this only adds debugging infrastructure and does not touch the runtime (apart from one renamed variable), I'm going to go ahead and merge this with the current state of reviews.

@fknorr fknorr merged commit 47bee9c into master Nov 8, 2023
28 checks passed
@fknorr fknorr deleted the gdb-pretty-print branch November 8, 2023 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants