Skip to content

Handle duplicate allocation labels during replay - #53

Merged
daxmawal merged 22 commits into
mainfrom
fix/replay-duplicate-labels
Oct 6, 2026
Merged

daxmawal merged 22 commits into
mainfrom
fix/replay-duplicate-labels

Conversation

@daxmawal

@daxmawal daxmawal commented Sep 9, 2026 •

Copy link
Copy Markdown
Member

Kokkos allocations can share the same label. Previously, replay allocations and reference outputs were indexed by label, so one allocation could hide another and make output comparison unreliable. This change preserves each allocation separately and matches inputs with their reference outputs using the captured allocation pointer and memory space. The new get_allocations API exposes these pairs through ReplayAllocation descriptors. New compare_views overloads accept a descriptor and support explicit dimensions or a flat view whose size is inferred from the recorded byte count.

Follow-up PR will add a compare_views(replayed_view, comparator) overload that uses the replayed view’s data pointer and memory space to locate the corresponding reference.

P.S. I encountered this issue in a Dyablo Kokkos kernel where multiple views shared the same label.

Match input allocations and reference outputs by captured allocation pointer
and memory space. Expose allocation descriptors for unambiguous comparisons,
including flat views whose extent is inferred from the recorded byte count.

Reject ambiguous label-only lookups and cover duplicate labels, reordered
references, empty allocations, and comparison size validation.
Signed-off-by: daxmawal <jeanfrancoismanutea@gmail.com>
Comment thread src/krepe/replay/kernel_replayer.cpp Outdated
Signed-off-by: daxmawal <jeanfrancoismanutea@gmail.com>
Signed-off-by: daxmawal <jeanfrancoismanutea@gmail.com>
@daxmawal
daxmawal marked this pull request as ready for review September 14, 2026 13:49
@daxmawal
daxmawal marked this pull request as draft September 24, 2026 08:11
@daxmawal
daxmawal marked this pull request as ready for review September 24, 2026 09:47
Signed-off-by: daxmawal <jeanfrancoismanutea@gmail.com>
@tretre91
tretre91 self-requested a review September 24, 2026 16:18
@daxmawal

daxmawal commented Sep 24, 2026 •

Copy link
Copy Markdown
Member Author

@tretre91 I'm wondering which interface we should prioritize for comparing replay results.

This PR fixes handling of allocations that share a label and rejects ambiguous label-based comparisons. Another option would be compare_views(kokkos::view replayed_view, KOKKOS_LAMBDA comparator), which finds the reference using the view's address and memory space.

Should I keep the allocation identification by address and memory space in this PR, drop my changes to label-based compare_views, and add a compare_views overload taking a Kokkos::View in a follow-up PR built on this work ?

@tretre91

Copy link
Copy Markdown
Member

I will try to look at it but I don't think I will finish my review before monday, we'll be able to discuss the changes in person by then

Comment thread src/krepe/replay/kernel_replayer.cpp
Comment thread src/krepe/replay/kernel_replayer.cpp Outdated
Comment thread src/krepe/replay/kernel_replayer.cpp
Comment thread src/krepe/replay/kernel_replayer.cpp Outdated
Comment thread src/krepe/replay/kernel_replayer.cpp Outdated
Comment thread src/krepe/replay/kernel_replayer.hpp
Comment thread src/krepe/replay/kernel_replayer.hpp
Comment thread src/krepe/replay/kernel_replayer.hpp Outdated
Comment thread tests/multiple_views_capture/main.cpp
Comment thread src/krepe/replay/kernel_replayer.cpp Outdated
Comment thread tests/duplicate_labels_capture/replay_main.cpp
Comment thread src/krepe/replay/kernel_replayer.cpp Outdated
Comment thread src/krepe/replay/kernel_replayer.cpp Outdated
Comment thread src/krepe/replay/kernel_replayer.cpp Outdated
Comment thread src/krepe/replay/kernel_replayer.cpp
Comment thread src/krepe/replay/kernel_replayer.cpp
Comment thread src/krepe/replay/kernel_replayer.cpp Outdated
Comment thread src/krepe/replay/kernel_replayer.cpp Outdated
Comment thread src/krepe/replay/kernel_replayer.hpp Outdated
Comment thread tests/duplicate_labels_capture/main.cpp
Comment thread tests/view_comparison/main.cpp Outdated
daxmawal and others added 6 commits October 5, 2026 23:51
Co-authored-by: Trévis Morvany <63788850+tretre91@users.noreply.github.com>
Signed-off-by: Jean-François <jeanfrancoismanutea@gmail.com>
Signed-off-by: daxmawal <jeanfrancoismanutea@gmail.com>

@tretre91 tretre91 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@daxmawal
daxmawal merged commit 08975cc into main Oct 6, 2026
17 checks passed
@daxmawal

daxmawal commented Oct 6, 2026

Copy link
Copy Markdown
Member Author

Merged, @tretre91. Thanks a lot !

I'll work on the nested lambda, and I'll come back with another PR to fix the other things we discussed in this PR.

@daxmawal
daxmawal deleted the fix/replay-duplicate-labels branch October 6, 2026 08:37
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