Fix lerp overload ambiguity with std::lerp under C++20#1985
Fix lerp overload ambiguity with std::lerp under C++20#1985crcrpar merged 1 commit intoNVIDIA:masterfrom
Conversation
PyTorch commit ad56ff73b751 ("[2/12] Upgrade cpp_extension and
cpp_builder to C++20", pytorch/pytorch#176659) changed the default
C++ standard from C++17 to C++20 for extensions built via
torch.utils.cpp_extension. Under C++20, std::lerp from <cmath> is
visible alongside the custom lerp(float,float,float) defined in this
file. When the third argument is c10::BFloat16 (implicitly convertible
to float), the compiler finds two equally-valid overload candidates
and fails with "more than one instance of overloaded function matches".
Rename the custom lerp to _lerp to eliminate the ambiguity.
Signed-off-by: Xiao Wang <24860335+xwang233@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Resolves a C++20 build failure in the CUDA multi-tensor distributed Adam optimizer by avoiding overload ambiguity between a local lerp helper and std::lerp introduced via <cmath>.
Changes:
- Rename the local device
lerphelper to_lerp. - Update all call sites in
DistAdamFunctorandDistAdamCapturableFunctorto use_lerp. - Add an explanatory comment about the C++20
std::lerpambiguity.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // (1-t)*x + t*y | ||
| __device__ __forceinline__ float lerp(float t, float x, float y) { | ||
| // Note: Named _lerp to avoid ambiguity with std::lerp under C++20. | ||
| __device__ __forceinline__ float _lerp(float t, float x, float y) { | ||
| // See https://developer.nvidia.com/blog/lerp-faster-cuda/ |
There was a problem hiding this comment.
The helper is renamed to _lerp, but identifiers beginning with an underscore at global scope are reserved in C++ and can conflict with compiler/libc++ internals. Please rename to a non-reserved internal name (e.g., fast_lerp, apex_lerp, or lerp_) or place it in a namespace/anonymous namespace and call it with a qualified name to avoid std::lerp ambiguity without using a reserved identifier.
There was a problem hiding this comment.
ah having this in an anonymous namespace sounds good
|
internal pipeline 45725282 looks good so far |
crcrpar
left a comment
There was a problem hiding this comment.
checked this can be compiled with a week old pytorch successfully
Summary
torch.utils.cpp_extensionstd::lerpfrom<cmath>becomes visible alongside the customlerp(float, float, float)defined inmulti_tensor_distopt_adam_kernel.cuc10::BFloat16(implicitly convertible tofloat), the compiler finds two equally-valid overload candidates and fails with "more than one instance of overloaded function lerp matches the argument list"lerpto_lerpto eliminate the ambiguityTest plan
🤖 Generated with Claude Code