I’m looking into an issue in the handling of complex number types, specifically in the vendored implementation of bit_cast which does not work with c10::complex or c10::complex
torch/headeronly/util/bit_cast.h:L34-37
static_assert(
std::is_trivially_constructible_v,
"This implementation additionally requires "
“destination type to be trivially constructible”);
I believe this should be is_default_constructible.
The c10::complex type includes default member initializers for float and double which makes it not trivially constructible, but does remain default constructible.
torch/headeronly/util/complex.h:L151-152
T real_ = T(0);
T imag_ = T(0);
Interestingly the definition of c10:complex does not have the default initializers, so works.
torch/headeronly/util/complex.h:L545-546
Half real_;
Half imag_;
I have running and deployed code where we have been removing this assert entirely, but I’m moving our internal patch to this more correct and robust check. I’d like to get away from needing this patch and upstream the change. Torch is a very large and complex project so I wanted to get feedback here first.
Some other observations:
This bit_cast scenario is supposed to work with complex float and double. There’s actually a potential internal usage,
torch/_inductor/codegen/cpp.py:L734-737
def to_dtype_bitcast(x, dtype, src_dtype):
if dtype not in DTYPE_TO_CPP:
raise AssertionError(f"{dtype} missing from {name}.DTYPE_TO_CPP")
return f"c10::bit_cast<{DTYPE_TO_CPP[dtype]}>({x})"
DTYPE_TO_CPP:
torch/_inductor/codegen/cpp_utils.py:L45-48
torch.complex32: “at::complexat::Half”,
torch.bcomplex32: “at::complexat::BFloat16”,
torch.complex64: “at::complex”,
torch.complex128: “at::complex”,
However it is not called because of
torch/_inductor/lowering.py:L1117-1122
def _view_dtype(x: TensorBox, dtype: torch.dtype):
if dtype.is_complex or x.get_dtype().is_complex:
return TensorBox.create(
ir.ComplexView.create(torch.ops.aten.view.dtype, x, dtype)
)
return to_dtype_bitcast(x, dtype)
Further, I think there’s an issue that means the vendored bit_cast is still used when the C++20 version is available. __cpp_lib_bit_cast is defined in and , but if neither have been imported then this check may erroneously fall back on the vendered version.
torch/headeronly/util/bit_cast.h:L8-14
#if __has_include() && (defined(__cpp_lib_bit_cast) && __cpp_lib_bit_cast >= 201806L)
#include <bit>
#define C10_HAVE_STD_BIT_CAST 1
#else
#define C10_HAVE_STD_BIT_CAST 0
#endif // __has_include() && (__cplusplus >= 202002L ||
// (defined(__cpp_lib_bit_cast) && __cpp_lib_bit_cast >= 201806L))
I think this should be replaced by first checking for and including since it’s lightweight, and then doing the check for the cpp lib bit_cast.
#if __has_include()
#include <version> // defines __cpp_lib_bit_cast
#endif
#if defined(__cpp_lib_bit_cast) && __cpp_lib_bit_cast >= 201806L
#include <bit>
#define C10_HAVE_STD_BIT_CAST 1
#else
#define C10_HAVE_STD_BIT_CAST 0
#endif
Semaphore.h already does an unconditional include of before doing a similar cpp lib check.
c10/util/Semaphore.h
#include <version>
...
#if __has_include() && defined(__cpp_lib_semaphore) &&__cpp_lib_semaphore >= 201907L && !defined(GLIBCXX)
#define C10_SEMAPHORE_USE_STL
#endif
These are two separate issues. The bit_cast cpp lib check will result in the vendored version being used sometimes and the std::bit_cast others dependent on what the order of includes in code. The vendored bit_cast will always reject the c10::complex float and double, incorrectly.
I’m happy to create one or two PRs on this, if I haven’t missed something. The only mild concern I have at the moment is that the vendored bit_cast has the C10_HOST_DEVICE marker. This doesn’t affect the validity of the change to vendored bit_cast, but is something the folks testing for C++20 should be conscious of as a potential change in behavior.
Thank you for your time. I tried including these all as permalinks to github, but as a new user I can’t include that many links…