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.

Sign up for GitHub

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

Jump to bottom

TL/MLX5: adding mcast helper functions #880

Merged
merged 1 commit into from
Jan 13, 2024
Merged

TL/MLX5: adding mcast helper functions #880

merged 1 commit into from
Jan 13, 2024

Conversation

MamziB
Copy link
Collaborator

@MamziB MamziB commented Nov 15, 2023

TL/MLX5: adding mcast helper functions

@MamziB MamziB self-assigned this Nov 15, 2023
@MamziB MamziB force-pushed the mamzi/mcast-merge-6-2 branch 2 times, most recently from 43a963c to da14f43 Compare November 15, 2023 21:41
Sergei-Lebedev
Sergei-Lebedev reviewed Nov 29, 2023
View reviewed changes
Copy link
Contributor

@Sergei-Lebedev Sergei-Lebedev left a comment

Choose a reason for hiding this comment

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

@MamziB please fix CI

src/components/tl/mlx5/mcast/tl_mlx5_mcast_context.c Outdated Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_context.c Outdated Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_context.c Outdated Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_context.c Outdated Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_context.c Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_context.c Outdated Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_context.c Outdated Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_team.c Outdated Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_team.c Outdated Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_context.c Outdated Show resolved Hide resolved
@MamziB MamziB force-pushed the mamzi/mcast-merge-6-2 branch from dd697de to dec07bb Compare November 29, 2023 19:42
@MamziB
Copy link
Collaborator Author

MamziB commented Nov 29, 2023 •
edited
Loading

@Sergei-Lebedev Thanks for the comments, I appreciate your time. I incorporated your comments, and updated the commit. @samnordmann Please kindly take a look at this PR and let me know your thoughts. Your constructive comments are much appreciated.
Please note that HPC_SDK fails somewhere out of this PR scope:

"/__w/ucc/ucc/src/coll_patterns/double_binary_tree.h", line 55: error #68: integer conversion resulted in a change of sign [integer_sign_change]
          return -1;
```

@MamziB MamziB force-pushed the mamzi/mcast-merge-6-2 branch 2 times, most recently from dc3eaae to 6ad7f8e Compare November 29, 2023 22:08
samnordmann
samnordmann reviewed Dec 3, 2023
View reviewed changes
src/components/tl/mlx5/mcast/tl_mlx5_mcast_context.c Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_context.c Outdated Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_context.c Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_context.c Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_team.c Outdated Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_helper.c Outdated Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_helper.c Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_helper.c Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_context.c Outdated Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_context.c Show resolved Hide resolved
@MamziB MamziB force-pushed the mamzi/mcast-merge-6-2 branch from 6ad7f8e to 622a09e Compare December 4, 2023 18:45
@MamziB
Copy link
Collaborator Author

MamziB commented Dec 4, 2023

@samnordmann Thanks for all the comments I appreciate it. @Sergei-Lebedev Please let me know if you have more comment otherwise we can finalize this PR

Sergei-Lebedev
Sergei-Lebedev reviewed Dec 5, 2023
View reviewed changes
src/components/tl/mlx5/mcast/tl_mlx5_mcast_context.c Outdated Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_context.c Outdated Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_context.c Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_context.c Outdated Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_context.c Outdated Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_context.c Outdated Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_context.c Outdated Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_context.c Outdated Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_context.c Outdated Show resolved Hide resolved
src/components/tl/mlx5/mcast/tl_mlx5_mcast_helper.c Outdated Show resolved Hide resolved
@MamziB MamziB force-pushed the mamzi/mcast-merge-6-2 branch from 622a09e to 036d1d0 Compare December 5, 2023 21:56
@MamziB
Copy link
Collaborator Author

MamziB commented Dec 5, 2023

Thank you @Sergei-Lebedev for the latest comments, I have updated the commit. @samnordmann Please kindly let me know if you have further thoughts about the PR otherwise we can finalize it

@MamziB
Copy link
Collaborator Author

MamziB commented Dec 7, 2023

@Sergei-Lebedev build UCC is failing in this PR because of some issues unrelated to it:

"/__w/ucc/ucc/src/coll_patterns/double_binary_tree.h", line 139: error #68: integer conversion resulted in a change of sign [integer_sign_change]

@MamziB MamziB force-pushed the mamzi/mcast-merge-6-2 branch from 036d1d0 to 910000a Compare December 13, 2023 21:45
@Sergei-Lebedev
Copy link
Contributor

@Sergei-Lebedev build UCC is failing in this PR because of some issues unrelated to it:

"/__w/ucc/ucc/src/coll_patterns/double_binary_tree.h", line 139: error #68: integer conversion resulted in a change of sign [integer_sign_change]

this issue is fixed, but now CI fails due to another issue that seems to be relevant to this PR

swx-clx02:rank4.lt-ucc_test_mpi: Failed to modify UD QP to INIT on mlx5_0: Operation not permitted

@MamziB
Copy link
Collaborator Author

MamziB commented Dec 15, 2023

@Sergei-Lebedev This error is not related to this PR. If it was, it should have printed this:

failed to move mcast qp to INIT

@MamziB MamziB force-pushed the mamzi/mcast-merge-6-2 branch from 910000a to e07d13c Compare December 15, 2023 18:35
@MamziB
Copy link
Collaborator Author

MamziB commented Dec 15, 2023

@Sergei-Lebedev I just updated the commit. Please let me know if you have further comments. Thanks

@Sergei-Lebedev
Copy link
Contributor

@MamziB please fix CI failures

@MamziB MamziB force-pushed the mamzi/mcast-merge-6-2 branch 4 times, most recently from 3035525 to 4d66b98 Compare January 7, 2024 08:27
@MamziB MamziB force-pushed the mamzi/mcast-merge-6-2 branch from 22832e1 to bc13ed7 Compare January 12, 2024 17:48
@Sergei-Lebedev Sergei-Lebedev merged commit 974605a into openucx:master Jan 13, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samnordmann samnordmann samnordmann approved these changes

@Sergei-Lebedev Sergei-Lebedev Sergei-Lebedev approved these changes

Assignees

@MamziB MamziB

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MamziB @Sergei-Lebedev @samnordmann