Skip to content

Conversation

@lydkim
Copy link
Contributor

@lydkim lydkim commented Jan 14, 2026

Summary:
The user-defined copy assignment operator made the type non-trivially copyable and just copied x with a self-assignment check, even though the copy constructor is defaulted. Since the struct only contains a single uint16_t, this diff defaults the copy assignment operator and fixes the -Wnontrivial-memcall warning introduced when building with llvm 21.
(fyi: This is code clean up - the flag -Wnontrivial-memcall is planned on being disabled anyways for nowT251075049)

Action failed: fbcode//gloo:gloo (cxx_compile allreduce_local.cc (pic))
Local command returned non-zero exit code 1
Reproduce locally: `env --chdir="$(buck2 root --kind project)" -- 'BUCK_SCRATCH_PATH=buck-out/v2/tmp/fbcode/d083cfbac224 ...<omitted>... g-info=buck-out/v2/gen/fbcode/gloo/__gloo__/output_artifacts/__objects__/allreduce_local.cc.pic.dwo' (run `buck2 log what-failed` to get the full command)`
stdout:
stderr:
fbcode/gloo/allreduce_local.cc:35:12: error: first argument in call to 'memcpy' is a pointer to non-trivially copyable type 'gloo::float16' [-Werror,-Wnontrivial-memcall]
   35 |     memcpy(ptrs_[i], ptrs_[0], bytes_);
      |            ^
fbcode/gloo/allreduce_local.cc:49:1: note: in instantiation of member function 'gloo::AllreduceLocal<gloo::float16>::run' requested here
   49 | INSTANTIATE_TEMPLATE(float16);
      | ^
fbcode/gloo/allreduce_local.cc:40:48: note: expanded from macro 'INSTANTIATE_TEMPLATE'
   40 | #define INSTANTIATE_TEMPLATE(T) template class AllreduceLocal<T>;
      |                                                ^
fbcode/gloo/allreduce_local.cc:35:12: note: explicitly cast the pointer to silence this warning
   35 |     memcpy(ptrs_[i], ptrs_[0], bytes_);
      |            ^
      |            (void*)
1 error generated.

Reviewed By: r-barnes

Differential Revision: D90615212


Summary:
The user-defined copy assignment operator made the type non-trivially copyable and just copied x with a self-assignment check, even though the copy constructor is defaulted. Since the struct only contains a single uint16_t, this diff defaults the copy assignment operator and fixes the `-Wnontrivial-memcall` warning introduced when building with llvm 21.
(fyi: This is code clean up - the flag `-Wnontrivial-memcall` is planned on being disabled anyways for nowT251075049)

```
Action failed: fbcode//gloo:gloo (cxx_compile allreduce_local.cc (pic))
Local command returned non-zero exit code 1
Reproduce locally: `env --chdir="$(buck2 root --kind project)" -- 'BUCK_SCRATCH_PATH=buck-out/v2/tmp/fbcode/d083cfbac224 ...<omitted>... g-info=buck-out/v2/gen/fbcode/gloo/__gloo__/output_artifacts/__objects__/allreduce_local.cc.pic.dwo' (run `buck2 log what-failed` to get the full command)`
stdout:
stderr:
fbcode/gloo/allreduce_local.cc:35:12: error: first argument in call to 'memcpy' is a pointer to non-trivially copyable type 'gloo::float16' [-Werror,-Wnontrivial-memcall]
   35 |     memcpy(ptrs_[i], ptrs_[0], bytes_);
      |            ^
fbcode/gloo/allreduce_local.cc:49:1: note: in instantiation of member function 'gloo::AllreduceLocal<gloo::float16>::run' requested here
   49 | INSTANTIATE_TEMPLATE(float16);
      | ^
fbcode/gloo/allreduce_local.cc:40:48: note: expanded from macro 'INSTANTIATE_TEMPLATE'
   40 | #define INSTANTIATE_TEMPLATE(T) template class AllreduceLocal<T>;
      |                                                ^
fbcode/gloo/allreduce_local.cc:35:12: note: explicitly cast the pointer to silence this warning
   35 |     memcpy(ptrs_[i], ptrs_[0], bytes_);
      |            ^
      |            (void*)
1 error generated.

Reviewed By: r-barnes

Differential Revision: D90615212
@meta-cla meta-cla bot added the CLA Signed label Jan 14, 2026
@meta-codesync
Copy link

meta-codesync bot commented Jan 14, 2026

@lydkim has exported this pull request. If you are a Meta employee, you can view the originating Diff in D90615212.

@meta-codesync meta-codesync bot merged commit 8d0b9a4 into pytorch:main Jan 14, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants