nvtx: fix missing .get() on shared_ptr cast in RECV_SEGMENT_COMPLETE tracepoint#1221
Open
bibrakc wants to merge 1 commit intoaws:masterfrom
Open
nvtx: fix missing .get() on shared_ptr cast in RECV_SEGMENT_COMPLETE tracepoint#1221bibrakc wants to merge 1 commit intoaws:masterfrom
bibrakc wants to merge 1 commit intoaws:masterfrom
Conversation
NCCL_OFI_TRACE_RECV_SEGMENT_COMPLETE_NVTX cast request->comm->ep directly, but since PR aws#1193 ep is a std::shared_ptr rather than a raw pointer. The cast fails to compile. Add the missing .get() call so NVTX builds succeed. This went undetected because CI does not build with --with-nvtx=...; a follow-up commit adds NVTX to the CI matrix so regressions like this are caught before merge. Signed-off-by: Bibrak Qamar Chandio <bibracha@amazon.com>
bwbarrett
approved these changes
May 5, 2026
rauteric
approved these changes
May 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of changes:
One macro in
include/tracing_impl/nvtx.h(
NCCL_OFI_TRACE_RECV_SEGMENT_COMPLETE_NVTX) castsrequest->comm->epdirectly, but since PR #1193 (shared_ptr refactor),
epis astd::shared_ptrrather than a raw pointer. The cast fails tocompile with
--with-nvtx=....Add the missing
.get()call. Every other similar cast in thisfile already does this correctly; this single line was missed.
This went undetected because the CI matrix does not include an
--with-nvtxbuild. Adding NVTX to CI is a separate follow-up sothese kinds of regressions get caught before merge.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.