Skip to content

Commit

Permalink
Raise PyTorch compiler standard to C++17
Browse files Browse the repository at this point in the history
Fixes #56055
  • Loading branch information
malfet committed Sep 30, 2022
1 parent dab1c7c commit 23cb59f
Show file tree
Hide file tree
Showing 11 changed files with 39 additions and 252 deletions.
6 changes: 2 additions & 4 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ string(FIND "${CMAKE_CXX_FLAGS}" "-std=c++" env_cxx_standard)
if(env_cxx_standard GREATER -1)
message(
WARNING "C++ standard version definition detected in environment variable."
"PyTorch requires -std=c++14. Please remove -std=c++ settings in your environment.")
"PyTorch requires -std=c++17. Please remove -std=c++ settings in your environment.")
endif()
set(CMAKE_CXX_STANDARD 14 CACHE STRING "The C++ standard whose features are requested to build this target.")
set(CMAKE_CXX_STANDARD 17 CACHE STRING "The C++ standard whose features are requested to build this target.")
set(CMAKE_C_STANDARD 11 CACHE STRING "The C standard whose features are requested to build this target.")

if(DEFINED GLIBCXX_USE_CXX11_ABI)
Expand Down Expand Up @@ -891,7 +891,6 @@ if(NOT MSVC)
append_cxx_flag_if_supported("-Wno-unused-private-field" CMAKE_CXX_FLAGS)
append_cxx_flag_if_supported("-Wno-inconsistent-missing-override" CMAKE_CXX_FLAGS)
append_cxx_flag_if_supported("-Wno-aligned-allocation-unavailable" CMAKE_CXX_FLAGS)
append_cxx_flag_if_supported("-Wno-c++14-extensions" CMAKE_CXX_FLAGS)
append_cxx_flag_if_supported("-Wno-constexpr-not-const" CMAKE_CXX_FLAGS)
append_cxx_flag_if_supported("-Wno-missing-braces" CMAKE_CXX_FLAGS)
append_cxx_flag_if_supported("-Wunused-lambda-capture" CMAKE_CXX_FLAGS)
Expand Down Expand Up @@ -993,7 +992,6 @@ if(APPLE)
endif()
append_cxx_flag_if_supported("-Wno-unused-private-field" CMAKE_CXX_FLAGS)
append_cxx_flag_if_supported("-Wno-missing-braces" CMAKE_CXX_FLAGS)
append_cxx_flag_if_supported("-Wno-c++14-extensions" CMAKE_CXX_FLAGS)
append_cxx_flag_if_supported("-Wno-constexpr-not-const" CMAKE_CXX_FLAGS)
endif()

Expand Down
13 changes: 0 additions & 13 deletions aten/src/ATen/Dispatch.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@ TORCH_API void record_kernel_function_dtype(std::string name);
#define RECORD_KERNEL_FUNCTION_DTYPE(NAME, enum_type)
#endif

// Avoid if_constexpr if possble, as it's more expensive to compile
#if defined __cpp_if_constexpr
#define AT_PRIVATE_CHECK_SELECTIVE_BUILD(enum_type) \
do { \
if constexpr (!at::should_include_kernel_dtype( \
Expand All @@ -64,17 +62,6 @@ TORCH_API void record_kernel_function_dtype(std::string name);
at_dispatch_name); \
} \
} while (0)
#else // defined __cpp_if_constexpr
#define AT_PRIVATE_CHECK_SELECTIVE_BUILD(enum_type) \
at::guts::if_constexpr<!at::should_include_kernel_dtype( \
at_dispatch_name, enum_type)>([&] { \
AT_ERROR( \
"dtype '", \
toString(enum_type), \
"' not selected for kernel tag ", \
at_dispatch_name); \
})
#endif

// Workaround for C10_UNUSED because CUDA 10.2 and below fails to handle unused
// attribute in the type aliasing context. Keep name long and verbose to avoid
Expand Down
25 changes: 11 additions & 14 deletions aten/src/ATen/core/boxing/impl/boxing.h
Original file line number Diff line number Diff line change
Expand Up @@ -226,20 +226,17 @@ struct BoxedKernelWrapper<
torch::jit::Stack stack = boxArgs<Args...>(std::forward<Args>(args)...);
boxed_kernel_func.callBoxed(opHandle, dispatchKeySet, &stack);

return guts::if_constexpr<!std::is_same<void, Result>::value>(
[&] (auto delay_check) {
// op has pushed one or more values onto the stack.
return delay_check(PopResult<Result>::call(stack));
},
[&] {
// op returns void, boxed kernel has pushed nothing onto stack.
TORCH_INTERNAL_ASSERT_DEBUG_ONLY(
stack.size() == 0,
"Boxed kernel was expected to return no values on the stack, ",
"but instead returned ", stack.size(), " values."
);
}
);
if constexpr (!std::is_same_v<void, Result>) {
// op has pushed one or more values onto the stack.
return PopResult<Result>::call(stack);
} else {
// op returns void, boxed kernel has pushed nothing onto stack.
TORCH_INTERNAL_ASSERT_DEBUG_ONLY(
stack.size() == 0,
"Boxed kernel was expected to return no values on the stack, ",
"but instead returned ", stack.size(), " values."
);
}
}
};

Expand Down
29 changes: 6 additions & 23 deletions aten/src/ATen/core/boxing/impl/make_boxed_from_unboxed_functor.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,12 @@ namespace impl {
template<class T, bool AllowDeprecatedTypes, class Enable = void>
struct assert_is_valid_input_type {
assert_is_valid_input_type() {
guts::if_constexpr<guts::typelist::contains<supported_primitive_arg_types, T>::value>([] {
if constexpr (guts::typelist::contains<supported_primitive_arg_types, T>::value) {
/* everything is ok, this is a primitive type */
}, /* else */ [] {
} else {
/* otherwise this must be an instance of a valid custom class, since it can only
have been created via IValue(x), which ensures this. */
});
};
}
};

Expand Down Expand Up @@ -209,12 +209,12 @@ namespace impl {
template<class T, bool AllowDeprecatedTypes, class Enable = void>
struct assert_is_valid_output_type {
assert_is_valid_output_type() {
guts::if_constexpr<guts::typelist::contains<supported_primitive_arg_types, T>::value>([] {
if constexpr (guts::typelist::contains<supported_primitive_arg_types, T>::value) {
/* everything is ok, this is a primitive type */
}, /* else */ [] {
} else {
/* otherwise T is verified to be a registered custom class in the IValue
constructor, so no benefit in double-checking here */
});
}
}
};

Expand Down Expand Up @@ -556,34 +556,17 @@ namespace impl {
using ArgTypes = typename c10::remove_DispatchKeySet_arg_from_func<KernelFunctor>::parameter_types;
constexpr bool has_outputs = !std::is_same<void, ReturnType>::value;
constexpr size_t num_inputs = guts::typelist::size<ArgTypes>::value;
#ifdef __cpp_if_constexpr
if constexpr (has_outputs) {
#else
guts::if_constexpr<has_outputs>([&] (auto delay_check) {
#endif
// Decay ReturnType to ReturnType_ so that if a reference gets returned, we actually store it by value
// and don't get a dangling reference. This is only required because some kernels still return `Tensor&`.
#ifdef __cpp_if_constexpr
using ReturnType_ = std::decay_t<ReturnType>;
ReturnType_ output = call_functor_with_args_from_stack<KernelFunctor, AllowDeprecatedTypes>(functor, dispatchKeySet, stack);
#else
using ReturnType_ = std::decay_t<typename decltype(delay_check)::template type_identity<ReturnType>>;
ReturnType_ output = call_functor_with_args_from_stack<KernelFunctor, AllowDeprecatedTypes>(functor, dispatchKeySet, delay_check(stack));
#endif
torch::jit::drop(*stack, num_inputs);
push_outputs<ReturnType_, AllowDeprecatedTypes>::call(std::move(output), stack);
#ifdef __cpp_if_constexpr
} else {
#else
}, /* else */ [&] {
#endif
call_functor_with_args_from_stack<KernelFunctor, AllowDeprecatedTypes>(functor, dispatchKeySet, stack);
torch::jit::drop(*stack, num_inputs);
#ifdef __cpp_if_constexpr
}
#else
});
#endif
}
};
} // namespace impl
Expand Down
22 changes: 6 additions & 16 deletions aten/src/ATen/core/ivalue_inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -975,11 +975,9 @@ struct C10_EXPORT ivalue::Future final : c10::intrusive_ptr_target {
*/
template <typename T>
void addCallback(T callback) {
#if __cpp_lib_is_invocable >= 201703
static_assert(
std::is_invocable_r<void, T, Future&>::value,
"The callback must have signature void(Future&)");
#endif
std::unique_lock<std::mutex> lock(mutex_);
if (completed()) {
lock.unlock();
Expand All @@ -997,30 +995,26 @@ struct C10_EXPORT ivalue::Future final : c10::intrusive_ptr_target {
template <typename T>
c10::intrusive_ptr<Future> then(T callback, TypePtr type) {
using IValueWithStorages = std::tuple<IValue, std::vector<WeakStorage>>;
#if __cpp_lib_is_invocable >= 201703
static_assert(
guts::disjunction<
std::is_invocable_r<IValue, T, Future&>,
std::is_invocable_r<IValueWithStorages, T, Future&>>::value,
"The callback must have signature IValue(Future&) or "
"std::tuple<IValue, std::vector<Storage>>(Future&)");
#endif
auto childFut = createInstance(std::move(type));
addCallback([childFut,
cb = std::move(callback)](Future& parentFut) mutable {
try {
guts::if_constexpr<std::is_convertible<
if constexpr (std::is_convertible_v<
typename c10::invoke_result_t<T &&, Future&>,
IValueWithStorages>::value>(
[&](auto identity) {
IValueWithStorages>) {
IValue value;
std::vector<WeakStorage> storages;
std::tie(value, storages) = identity(cb)(parentFut);
std::tie(value, storages) = cb(parentFut);
childFut->markCompleted(std::move(value), std::move(storages));
},
[&](auto identity) {
childFut->markCompleted(identity(cb)(parentFut));
});
} else {
childFut->markCompleted(cb(parentFut));
};
} catch (std::exception&) {
childFut->setError(std::current_exception());
}
Expand All @@ -1030,11 +1024,9 @@ struct C10_EXPORT ivalue::Future final : c10::intrusive_ptr_target {

template <typename T>
c10::intrusive_ptr<Future> thenAsync(T callback, TypePtr type) {
#if __cpp_lib_is_invocable >= 201703
static_assert(
std::is_invocable_r<c10::intrusive_ptr<Future>, T, Future&>::value,
"The callback must have signature c10::intrusive_ptr<Future>(Future&)");
#endif
auto childFut = createInstance(std::move(type));
addCallback(
[childFut, cb = std::move(callback)](Future& parentFut) mutable {
Expand Down Expand Up @@ -1111,11 +1103,9 @@ struct C10_EXPORT ivalue::Future final : c10::intrusive_ptr_target {
// synchronize them with the value, and so on (if needed).
template<typename T>
void invokeCallback(T callback) {
#if __cpp_lib_is_invocable >= 201703
static_assert(
std::is_invocable_r<void, T, Future&>::value,
"The callback must have signature void(Future&)");
#endif

c10::OptionalDeviceGuard deviceGuard(currentDevice_);

Expand Down
10 changes: 5 additions & 5 deletions aten/src/ATen/native/ReduceOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1155,11 +1155,11 @@ Tensor trace_cpu(const Tensor& self) {
sum += t_data[i * (t_stride_0 + t_stride_1)];
}

c10::guts::if_constexpr<std::is_integral<accscalar_t>::value>(
// all integer types get promoted to kLong
[&] (auto _) { *result.data_ptr<int64_t>() = _(sum); }, // then-case, invalid for non-integral types
[&] (auto _) { *result.data_ptr<scalar_t>() = _(sum); } // else-case, invalid for integral types
);
if constexpr (std::is_integral_v<accscalar_t>) {
*result.data_ptr<int64_t>() = sum;
} else {
*result.data_ptr<scalar_t>() = sum;
}
});

return result;
Expand Down
4 changes: 2 additions & 2 deletions aten/src/ATen/native/cuda/jit_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1531,7 +1531,7 @@ NvrtcFunction jit_pwise_function(
&program, code.c_str(), nullptr, 0, nullptr, nullptr));

#ifdef USE_ROCM
std::vector<const char*> args = {"--std=c++14"};
std::vector<const char*> args = {"--std=c++17"};
#else
// Constructs nvrtc build arguments
// CUDA 11.1 allows going directly to SASS (sm_) instead of PTX (compute_)
Expand All @@ -1546,7 +1546,7 @@ NvrtcFunction jit_pwise_function(
std::to_string(cuda_minor);
// NOLINTNEXTLINE(cppcoreguidelines-init-variables)
std::vector<const char*> args = {
"--std=c++14", compute.c_str(), "-default-device"};
"--std=c++17", compute.c_str(), "-default-device"};
#endif

#ifndef NDEBUG
Expand Down
2 changes: 1 addition & 1 deletion c10/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
cmake_minimum_required(VERSION 3.10 FATAL_ERROR)
project(c10 CXX)

set(CMAKE_CXX_STANDARD 14 CACHE STRING "The C++ standard whose features are requested to build this target.")
set(CMAKE_CXX_STANDARD 17 CACHE STRING "The C++ standard whose features are requested to build this target.")
set(CMAKE_EXPORT_COMPILE_COMMANDS ON)

# Main build file for the C10 library.
Expand Down

0 comments on commit 23cb59f

Please sign in to comment.