From 345393632873390303415eea7a7e04ebb350afaf Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 9 Apr 2020 07:28:58 +0300 Subject: [PATCH 01/27] #690 , starting with tests to make sure they fail --- .../VSO_157296 _overaligned_function/env.lst | 4 ++ .../VSO_157296 _overaligned_function/test.cpp | 38 +++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 tests/std/tests/VSO_157296 _overaligned_function/env.lst create mode 100644 tests/std/tests/VSO_157296 _overaligned_function/test.cpp diff --git a/tests/std/tests/VSO_157296 _overaligned_function/env.lst b/tests/std/tests/VSO_157296 _overaligned_function/env.lst new file mode 100644 index 00000000000..19f025bd0e6 --- /dev/null +++ b/tests/std/tests/VSO_157296 _overaligned_function/env.lst @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +RUNALL_INCLUDE ..\usual_matrix.lst diff --git a/tests/std/tests/VSO_157296 _overaligned_function/test.cpp b/tests/std/tests/VSO_157296 _overaligned_function/test.cpp new file mode 100644 index 00000000000..a87f392f388 --- /dev/null +++ b/tests/std/tests/VSO_157296 _overaligned_function/test.cpp @@ -0,0 +1,38 @@ +#include +#include +#include +#include +#include +#include + +#pragma warning(disable : 4324) // "was padded due to alignment specifier", thanks, Captain + +struct alignas(16) overaligned_t { + char non_empty; + + void operator()() const { + assert(static_cast(reinterpret_cast(this) % alignof(overaligned_t)) == 0); + } +}; + +static_assert(alignof(overaligned_t) == 16); +static_assert(alignof(overaligned_t) > alignof(std::max_align_t)); + +static_assert(alignof(std::max_align_t) == 8, "max_align_t has changed, the whole stuff should be revised"); + +struct functions_t { + using function_t = std::function; + + function_t first{overaligned_t{}}; + char smallest_pad; + function_t second{overaligned_t{}}; +}; + + + +int main() { + functions_t functions; + functions.first(); + functions.second(); + return 0; +} \ No newline at end of file From 5af04f71476760c68f86af5f08858f67862f3c3d Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 9 Apr 2020 07:32:03 +0300 Subject: [PATCH 02/27] env.lst --- tests/std/test.lst | 1 + .../env.lst | 0 .../test.cpp | 4 +--- 3 files changed, 2 insertions(+), 3 deletions(-) rename tests/std/tests/{VSO_157296 _overaligned_function => VSO_1062649_overaligned_function}/env.lst (100%) rename tests/std/tests/{VSO_157296 _overaligned_function => VSO_1062649_overaligned_function}/test.cpp (91%) diff --git a/tests/std/test.lst b/tests/std/test.lst index 6d2945178b3..e94c27b0430 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -340,3 +340,4 @@ tests\VSO_0849827_multicontainer_emplace_hint_position tests\VSO_0938757_attribute_order tests\VSO_0961751_hash_range_erase tests\VSO_0971246_legacy_await_headers +tests\VSO_1062649_overaligned_function diff --git a/tests/std/tests/VSO_157296 _overaligned_function/env.lst b/tests/std/tests/VSO_1062649_overaligned_function/env.lst similarity index 100% rename from tests/std/tests/VSO_157296 _overaligned_function/env.lst rename to tests/std/tests/VSO_1062649_overaligned_function/env.lst diff --git a/tests/std/tests/VSO_157296 _overaligned_function/test.cpp b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp similarity index 91% rename from tests/std/tests/VSO_157296 _overaligned_function/test.cpp rename to tests/std/tests/VSO_1062649_overaligned_function/test.cpp index a87f392f388..3a2d8046682 100644 --- a/tests/std/tests/VSO_157296 _overaligned_function/test.cpp +++ b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp @@ -2,8 +2,6 @@ #include #include #include -#include -#include #pragma warning(disable : 4324) // "was padded due to alignment specifier", thanks, Captain @@ -35,4 +33,4 @@ int main() { functions.first(); functions.second(); return 0; -} \ No newline at end of file +} From 699e6fa41adca63311e3434ddf9a3767b6d70791 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 9 Apr 2020 07:39:20 +0300 Subject: [PATCH 03/27] copyrignt, license --- tests/std/tests/VSO_1062649_overaligned_function/test.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp index 3a2d8046682..85162374841 100644 --- a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp +++ b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp @@ -1,3 +1,6 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + #include #include #include @@ -24,13 +27,13 @@ struct functions_t { function_t first{overaligned_t{}}; char smallest_pad; function_t second{overaligned_t{}}; + function_t third{overaligned_t{}}; }; - - int main() { functions_t functions; functions.first(); functions.second(); + functions.third(); return 0; } From 93e06653e8c2fea70f5a842e26c64bee0ae08feb Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 9 Apr 2020 08:27:08 +0300 Subject: [PATCH 04/27] the fix for #690 --- stl/inc/functional | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/stl/inc/functional b/stl/inc/functional index 36f7e4f1ef8..76ac7f70ebd 100644 --- a/stl/inc/functional +++ b/stl/inc/functional @@ -795,7 +795,8 @@ private: constexpr size_t _Space_size = (_Small_object_num_ptrs - 1) * sizeof(void*); template // determine whether _Impl must be dynamically allocated -_INLINE_VAR constexpr bool _Is_large = (_Space_size < sizeof(_Impl)) || !_Impl::_Nothrow_move::value; +_INLINE_VAR constexpr bool _Is_large = (_Space_size < sizeof(_Impl)) || alignof(_Impl) > alignof(max_align_t) + || !_Impl::_Nothrow_move::value; #if _HAS_FUNCTION_ALLOCATOR_SUPPORT // CLASS TEMPLATE _Func_impl From 7fda7bf59ff4b948416f0115e2d94c2b77b6ea32 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 9 Apr 2020 09:21:59 +0300 Subject: [PATCH 05/27] unterse `static_assert` --- tests/std/tests/VSO_1062649_overaligned_function/test.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp index 85162374841..3a6eee5e242 100644 --- a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp +++ b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp @@ -16,9 +16,8 @@ struct alignas(16) overaligned_t { } }; -static_assert(alignof(overaligned_t) == 16); -static_assert(alignof(overaligned_t) > alignof(std::max_align_t)); - +static_assert(alignof(overaligned_t) == 16, "overaligned_t is not aligned as expected"); +static_assert(alignof(overaligned_t) > alignof(std::max_align_t), "overaligned_t is not overaligned"); static_assert(alignof(std::max_align_t) == 8, "max_align_t has changed, the whole stuff should be revised"); struct functions_t { From ec38c3b209f83557c7523716be7cba0727d46882 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 9 Apr 2020 10:44:54 +0300 Subject: [PATCH 06/27] improve test condition --- .../VSO_1062649_overaligned_function/test.cpp | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp index 3a6eee5e242..1d0a4b6aa6b 100644 --- a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp +++ b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp @@ -11,7 +11,15 @@ struct alignas(16) overaligned_t { char non_empty; - void operator()() const { + void operator()(const void* storage, std::size_t storage_size) const { + const unsigned char* storage_bytes = reinterpret_cast(storage); + const unsigned char* this_bytes = reinterpret_cast(this); + + // loop instead of range comparison to avoid UB when not in range + for (std::size_t i = 0; i < storage_size; i++) { + assert(storage_bytes + i != this_bytes); + } + assert(static_cast(reinterpret_cast(this) % alignof(overaligned_t)) == 0); } }; @@ -21,7 +29,7 @@ static_assert(alignof(overaligned_t) > alignof(std::max_align_t), "overaligned_t static_assert(alignof(std::max_align_t) == 8, "max_align_t has changed, the whole stuff should be revised"); struct functions_t { - using function_t = std::function; + using function_t = std::function; function_t first{overaligned_t{}}; char smallest_pad; @@ -31,8 +39,9 @@ struct functions_t { int main() { functions_t functions; - functions.first(); - functions.second(); - functions.third(); + functions.first(&functions.first, sizeof(functions.first)); + functions.second(&functions.second, sizeof(functions.second)); + functions.third(&functions.third, sizeof(functions.third)); return 0; } + From 6eba7de9eb6803128e8f69821ec972548e16f394 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 9 Apr 2020 11:03:03 +0300 Subject: [PATCH 07/27] guard over-aligned feature --- .../tests/VSO_1062649_overaligned_function/test.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp index 1d0a4b6aa6b..16272751a99 100644 --- a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp +++ b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp @@ -1,6 +1,9 @@ // Copyright (c) Microsoft Corporation. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// this test is only valid if over-aligned allocation is supported +#ifdef __cpp_aligned_new + #include #include #include @@ -19,7 +22,6 @@ struct alignas(16) overaligned_t { for (std::size_t i = 0; i < storage_size; i++) { assert(storage_bytes + i != this_bytes); } - assert(static_cast(reinterpret_cast(this) % alignof(overaligned_t)) == 0); } }; @@ -45,3 +47,10 @@ int main() { return 0; } +#else // ^^^ __cpp_aligned_new / !__cpp_aligned_new vvv + +int main() { + return 0; +} + +#endif // __cpp_aligned_new From a9455b78bceb7b5baa0e72935f22a207a8eba556 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 9 Apr 2020 11:09:00 +0300 Subject: [PATCH 08/27] Don't pad then if pre-__cpp_aligned_new --- stl/inc/functional | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/stl/inc/functional b/stl/inc/functional index 76ac7f70ebd..ed69ca346e8 100644 --- a/stl/inc/functional +++ b/stl/inc/functional @@ -795,8 +795,11 @@ private: constexpr size_t _Space_size = (_Small_object_num_ptrs - 1) * sizeof(void*); template // determine whether _Impl must be dynamically allocated -_INLINE_VAR constexpr bool _Is_large = (_Space_size < sizeof(_Impl)) || alignof(_Impl) > alignof(max_align_t) - || !_Impl::_Nothrow_move::value; +_INLINE_VAR constexpr bool _Is_large = (_Space_size < sizeof(_Impl)) || !_Impl::_Nothrow_move::value +#ifdef __cpp_aligned_new + || alignof(_Impl) > alignof(max_align_t) +#endif + ; #if _HAS_FUNCTION_ALLOCATOR_SUPPORT // CLASS TEMPLATE _Func_impl From 2b9cae1afe8be834e97b139febfeb3f6f56d33f7 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 9 Apr 2020 12:03:11 +0300 Subject: [PATCH 09/27] skip for pre-17 by test matrix, not by ifdef --- .../VSO_1062649_overaligned_function/env.lst | 2 +- .../VSO_1062649_overaligned_function/test.cpp | 15 ++++----------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/tests/std/tests/VSO_1062649_overaligned_function/env.lst b/tests/std/tests/VSO_1062649_overaligned_function/env.lst index 19f025bd0e6..2de7aab2959 100644 --- a/tests/std/tests/VSO_1062649_overaligned_function/env.lst +++ b/tests/std/tests/VSO_1062649_overaligned_function/env.lst @@ -1,4 +1,4 @@ # Copyright (c) Microsoft Corporation. # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -RUNALL_INCLUDE ..\usual_matrix.lst +RUNALL_INCLUDE ..\usual_17_matrix.lst diff --git a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp index 16272751a99..8d713d11a23 100644 --- a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp +++ b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp @@ -1,14 +1,15 @@ // Copyright (c) Microsoft Corporation. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// this test is only valid if over-aligned allocation is supported -#ifdef __cpp_aligned_new - #include #include #include #include +#ifndef __cpp_aligned_new +#pragma error("This test is only valid when C++17 over-aligned allication is supported") +#endif + #pragma warning(disable : 4324) // "was padded due to alignment specifier", thanks, Captain struct alignas(16) overaligned_t { @@ -46,11 +47,3 @@ int main() { functions.third(&functions.third, sizeof(functions.third)); return 0; } - -#else // ^^^ __cpp_aligned_new / !__cpp_aligned_new vvv - -int main() { - return 0; -} - -#endif // __cpp_aligned_new From 7aa7d1f7d9f3f93446e80b911a8a79b7b44deac3 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 9 Apr 2020 20:34:59 +0300 Subject: [PATCH 10/27] Update tests/std/tests/VSO_1062649_overaligned_function/test.cpp Co-Authored-By: Casey Carter --- tests/std/tests/VSO_1062649_overaligned_function/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp index 8d713d11a23..8009c30f95f 100644 --- a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp +++ b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp @@ -23,7 +23,7 @@ struct alignas(16) overaligned_t { for (std::size_t i = 0; i < storage_size; i++) { assert(storage_bytes + i != this_bytes); } - assert(static_cast(reinterpret_cast(this) % alignof(overaligned_t)) == 0); + assert(reinterpret_cast(this) % alignof(overaligned_t) == 0); } }; From 55371d1407074af95b8f8365097ed3c26b83fe21 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 9 Apr 2020 20:42:07 +0300 Subject: [PATCH 11/27] Update tests/std/tests/VSO_1062649_overaligned_function/test.cpp Co-Authored-By: Casey Carter --- tests/std/tests/VSO_1062649_overaligned_function/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp index 8009c30f95f..058ebd81aad 100644 --- a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp +++ b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp @@ -12,7 +12,7 @@ #pragma warning(disable : 4324) // "was padded due to alignment specifier", thanks, Captain -struct alignas(16) overaligned_t { +struct alignas(4 * alignof(std::max_align_t)) overaligned_t { char non_empty; void operator()(const void* storage, std::size_t storage_size) const { From 74f4592b2d61d22993a01064f58be31866c339b3 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 9 Apr 2020 20:43:03 +0300 Subject: [PATCH 12/27] Update tests/std/tests/VSO_1062649_overaligned_function/test.cpp Co-Authored-By: Casey Carter --- tests/std/tests/VSO_1062649_overaligned_function/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp index 058ebd81aad..6a5780498aa 100644 --- a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp +++ b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp @@ -15,7 +15,7 @@ struct alignas(4 * alignof(std::max_align_t)) overaligned_t { char non_empty; - void operator()(const void* storage, std::size_t storage_size) const { + void operator()(const void* const storage, const std::size_t storage_size) const { const unsigned char* storage_bytes = reinterpret_cast(storage); const unsigned char* this_bytes = reinterpret_cast(this); From 792994c6305060a92e0ebd01e944dfc7aa551498 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 9 Apr 2020 20:56:57 +0300 Subject: [PATCH 13/27] #error correctly, simplify checks --- .../VSO_1062649_overaligned_function/test.cpp | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp index 6a5780498aa..9055cc5a327 100644 --- a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp +++ b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp @@ -7,29 +7,26 @@ #include #ifndef __cpp_aligned_new -#pragma error("This test is only valid when C++17 over-aligned allication is supported") +#error This test is only valid when C++17 over-aligned allocation is supported #endif #pragma warning(disable : 4324) // "was padded due to alignment specifier", thanks, Captain -struct alignas(4 * alignof(std::max_align_t)) overaligned_t { +struct alignas(2 * alignof(std::max_align_t)) overaligned_t { char non_empty; - void operator()(const void* const storage, const std::size_t storage_size) const { + void operator()(const void* storage, std::size_t storage_size) const { const unsigned char* storage_bytes = reinterpret_cast(storage); const unsigned char* this_bytes = reinterpret_cast(this); - // loop instead of range comparison to avoid UB when not in range - for (std::size_t i = 0; i < storage_size; i++) { - assert(storage_bytes + i != this_bytes); - } - assert(reinterpret_cast(this) % alignof(overaligned_t) == 0); + // platform-specific behavior not covered by Standard C++, but fine for such test + assert(this_bytes < storage_bytes || this_bytes > storage_bytes + storage_size); + assert(static_cast(reinterpret_cast(this) % alignof(overaligned_t)) == 0); } }; -static_assert(alignof(overaligned_t) == 16, "overaligned_t is not aligned as expected"); +static_assert(alignof(overaligned_t) < sizeof(std::function), "overaligned_t is not aligned as expected"); static_assert(alignof(overaligned_t) > alignof(std::max_align_t), "overaligned_t is not overaligned"); -static_assert(alignof(std::max_align_t) == 8, "max_align_t has changed, the whole stuff should be revised"); struct functions_t { using function_t = std::function; From c25d0a0d2242b866eef8af1a6704e21977269a9a Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 9 Apr 2020 21:12:28 +0300 Subject: [PATCH 14/27] getting back lost const --- tests/std/tests/VSO_1062649_overaligned_function/test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp index 9055cc5a327..371cd1c8dd9 100644 --- a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp +++ b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp @@ -15,13 +15,13 @@ struct alignas(2 * alignof(std::max_align_t)) overaligned_t { char non_empty; - void operator()(const void* storage, std::size_t storage_size) const { + void operator()(const void* const storage, const std::size_t storage_size) const { const unsigned char* storage_bytes = reinterpret_cast(storage); const unsigned char* this_bytes = reinterpret_cast(this); // platform-specific behavior not covered by Standard C++, but fine for such test assert(this_bytes < storage_bytes || this_bytes > storage_bytes + storage_size); - assert(static_cast(reinterpret_cast(this) % alignof(overaligned_t)) == 0); + assert(reinterpret_cast(this) % alignof(overaligned_t) == 0); } }; From 01e9c0d5f573e3a6e3004ba9ad21f787349d4ccc Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 9 Apr 2020 21:16:49 +0300 Subject: [PATCH 15/27] Update tests/std/tests/VSO_1062649_overaligned_function/test.cpp Co-Authored-By: Billy O'Neal --- tests/std/tests/VSO_1062649_overaligned_function/test.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp index 371cd1c8dd9..00fe4a7308c 100644 --- a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp +++ b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp @@ -10,7 +10,8 @@ #error This test is only valid when C++17 over-aligned allocation is supported #endif -#pragma warning(disable : 4324) // "was padded due to alignment specifier", thanks, Captain +#pragma warning(disable : 4324) // structure was padded due to alignment specifier + struct alignas(2 * alignof(std::max_align_t)) overaligned_t { char non_empty; From 6b6ee27f800d773d0b592cd50d4209d0cdbee143 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 9 Apr 2020 21:22:19 +0300 Subject: [PATCH 16/27] unconditionally no SFO for over-aligned --- stl/inc/functional | 7 ++----- tests/std/tests/VSO_1062649_overaligned_function/test.cpp | 5 ++--- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/stl/inc/functional b/stl/inc/functional index ed69ca346e8..ef9bea1c60c 100644 --- a/stl/inc/functional +++ b/stl/inc/functional @@ -795,11 +795,8 @@ private: constexpr size_t _Space_size = (_Small_object_num_ptrs - 1) * sizeof(void*); template // determine whether _Impl must be dynamically allocated -_INLINE_VAR constexpr bool _Is_large = (_Space_size < sizeof(_Impl)) || !_Impl::_Nothrow_move::value -#ifdef __cpp_aligned_new - || alignof(_Impl) > alignof(max_align_t) -#endif - ; +_INLINE_VAR constexpr bool _Is_large = _Space_size < sizeof(_Impl) || alignof(_Impl) > alignof(max_align_t) + || !_Impl::_Nothrow_move::value; #if _HAS_FUNCTION_ALLOCATOR_SUPPORT // CLASS TEMPLATE _Func_impl diff --git a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp index 00fe4a7308c..885db0e15d7 100644 --- a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp +++ b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp @@ -12,7 +12,6 @@ #pragma warning(disable : 4324) // structure was padded due to alignment specifier - struct alignas(2 * alignof(std::max_align_t)) overaligned_t { char non_empty; @@ -21,12 +20,12 @@ struct alignas(2 * alignof(std::max_align_t)) overaligned_t { const unsigned char* this_bytes = reinterpret_cast(this); // platform-specific behavior not covered by Standard C++, but fine for such test - assert(this_bytes < storage_bytes || this_bytes > storage_bytes + storage_size); + assert(this_bytes < storage_bytes || this_bytes >= storage_bytes + storage_size); assert(reinterpret_cast(this) % alignof(overaligned_t) == 0); } }; -static_assert(alignof(overaligned_t) < sizeof(std::function), "overaligned_t is not aligned as expected"); +static_assert(sizeof(overaligned_t) < sizeof(std::function), "overaligned_t is not aligned as expected"); static_assert(alignof(overaligned_t) > alignof(std::max_align_t), "overaligned_t is not overaligned"); struct functions_t { From d811dbc359dec5729e6b41d073b582097a1fafa6 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 9 Apr 2020 21:27:53 +0300 Subject: [PATCH 17/27] integer math instead of (nonstandard) pointer math --- tests/std/tests/VSO_1062649_overaligned_function/test.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp index 885db0e15d7..5c6878b441b 100644 --- a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp +++ b/tests/std/tests/VSO_1062649_overaligned_function/test.cpp @@ -16,12 +16,12 @@ struct alignas(2 * alignof(std::max_align_t)) overaligned_t { char non_empty; void operator()(const void* const storage, const std::size_t storage_size) const { - const unsigned char* storage_bytes = reinterpret_cast(storage); - const unsigned char* this_bytes = reinterpret_cast(this); + const std::uintptr_t storage_ptr_value = reinterpret_cast(storage); + const std::uintptr_t this_ptr_value = reinterpret_cast(this); // platform-specific behavior not covered by Standard C++, but fine for such test - assert(this_bytes < storage_bytes || this_bytes >= storage_bytes + storage_size); - assert(reinterpret_cast(this) % alignof(overaligned_t) == 0); + assert(this_ptr_value < storage_ptr_value || this_ptr_value >= storage_ptr_value + storage_size); + assert(this_ptr_value % alignof(overaligned_t) == 0); } }; From d3be4b37e7855be1bfd0ffa39501020923fd2b5b Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 9 Apr 2020 21:30:17 +0300 Subject: [PATCH 18/27] rename the test --- tests/std/test.lst | 2 +- .../env.lst | 0 .../test.cpp | 0 3 files changed, 1 insertion(+), 1 deletion(-) rename tests/std/tests/{VSO_1062649_overaligned_function => GH_000690_overaligned_function}/env.lst (100%) rename tests/std/tests/{VSO_1062649_overaligned_function => GH_000690_overaligned_function}/test.cpp (100%) diff --git a/tests/std/test.lst b/tests/std/test.lst index e94c27b0430..58f51e623f5 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -157,6 +157,7 @@ tests\Dev11_1158803_regex_thread_safety tests\Dev11_1180290_filesystem_error_code tests\GH_000457_system_error_message tests\GH_000545_include_compare +tests\GH_000690_overaligned_function tests\P0024R2_parallel_algorithms_adjacent_difference tests\P0024R2_parallel_algorithms_adjacent_find tests\P0024R2_parallel_algorithms_all_of @@ -340,4 +341,3 @@ tests\VSO_0849827_multicontainer_emplace_hint_position tests\VSO_0938757_attribute_order tests\VSO_0961751_hash_range_erase tests\VSO_0971246_legacy_await_headers -tests\VSO_1062649_overaligned_function diff --git a/tests/std/tests/VSO_1062649_overaligned_function/env.lst b/tests/std/tests/GH_000690_overaligned_function/env.lst similarity index 100% rename from tests/std/tests/VSO_1062649_overaligned_function/env.lst rename to tests/std/tests/GH_000690_overaligned_function/env.lst diff --git a/tests/std/tests/VSO_1062649_overaligned_function/test.cpp b/tests/std/tests/GH_000690_overaligned_function/test.cpp similarity index 100% rename from tests/std/tests/VSO_1062649_overaligned_function/test.cpp rename to tests/std/tests/GH_000690_overaligned_function/test.cpp From cd84b7fcdded12b2dfd79c62651082bf319748c6 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 9 Apr 2020 21:36:48 +0300 Subject: [PATCH 19/27] these braces are needed for clang-format it interprets as template braces otherwise --- stl/inc/functional | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/functional b/stl/inc/functional index ef9bea1c60c..76ac7f70ebd 100644 --- a/stl/inc/functional +++ b/stl/inc/functional @@ -795,7 +795,7 @@ private: constexpr size_t _Space_size = (_Small_object_num_ptrs - 1) * sizeof(void*); template // determine whether _Impl must be dynamically allocated -_INLINE_VAR constexpr bool _Is_large = _Space_size < sizeof(_Impl) || alignof(_Impl) > alignof(max_align_t) +_INLINE_VAR constexpr bool _Is_large = (_Space_size < sizeof(_Impl)) || alignof(_Impl) > alignof(max_align_t) || !_Impl::_Nothrow_move::value; #if _HAS_FUNCTION_ALLOCATOR_SUPPORT From 62e273027912f91225b66efe2f45a0b90658c349 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 9 Apr 2020 21:56:35 +0300 Subject: [PATCH 20/27] runtime check for SFO to happen --- .../GH_000690_overaligned_function/test.cpp | 27 +++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/tests/std/tests/GH_000690_overaligned_function/test.cpp b/tests/std/tests/GH_000690_overaligned_function/test.cpp index 5c6878b441b..bf340e98fdb 100644 --- a/tests/std/tests/GH_000690_overaligned_function/test.cpp +++ b/tests/std/tests/GH_000690_overaligned_function/test.cpp @@ -12,12 +12,13 @@ #pragma warning(disable : 4324) // structure was padded due to alignment specifier +// SFO should not happen struct alignas(2 * alignof(std::max_align_t)) overaligned_t { char non_empty; void operator()(const void* const storage, const std::size_t storage_size) const { - const std::uintptr_t storage_ptr_value = reinterpret_cast(storage); - const std::uintptr_t this_ptr_value = reinterpret_cast(this); + const auto storage_ptr_value = reinterpret_cast(storage); + const auto this_ptr_value = reinterpret_cast(this); // platform-specific behavior not covered by Standard C++, but fine for such test assert(this_ptr_value < storage_ptr_value || this_ptr_value >= storage_ptr_value + storage_size); @@ -25,12 +26,24 @@ struct alignas(2 * alignof(std::max_align_t)) overaligned_t { } }; -static_assert(sizeof(overaligned_t) < sizeof(std::function), "overaligned_t is not aligned as expected"); +// SFO should happen +struct not_overaligned_t { + char data[sizeof(overaligned_t)]; + + void operator()(const void* const storage, const std::size_t storage_size) const { + const auto storage_ptr_value = reinterpret_cast(storage); + const auto this_ptr_value = reinterpret_cast(this); + + // platform-specific behavior not covered by Standard C++, but fine for such test + assert(this_ptr_value >= storage_ptr_value && this_ptr_value < storage_ptr_value + storage_size); + } +}; + static_assert(alignof(overaligned_t) > alignof(std::max_align_t), "overaligned_t is not overaligned"); -struct functions_t { - using function_t = std::function; +using function_t = std::function; +struct functions_t { function_t first{overaligned_t{}}; char smallest_pad; function_t second{overaligned_t{}}; @@ -42,5 +55,9 @@ int main() { functions.first(&functions.first, sizeof(functions.first)); functions.second(&functions.second, sizeof(functions.second)); functions.third(&functions.third, sizeof(functions.third)); + + function_t sfo{not_overaligned_t{}}; + sfo(&sfo, sizeof(sfo)); + return 0; } From c0ed4c61ce4fb68075343930e33b610d6cce367a Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Fri, 10 Apr 2020 05:39:55 +0300 Subject: [PATCH 21/27] Update stl/inc/functional Co-Authored-By: Stephan T. Lavavej --- stl/inc/functional | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/functional b/stl/inc/functional index 76ac7f70ebd..caf0134d004 100644 --- a/stl/inc/functional +++ b/stl/inc/functional @@ -795,7 +795,7 @@ private: constexpr size_t _Space_size = (_Small_object_num_ptrs - 1) * sizeof(void*); template // determine whether _Impl must be dynamically allocated -_INLINE_VAR constexpr bool _Is_large = (_Space_size < sizeof(_Impl)) || alignof(_Impl) > alignof(max_align_t) +_INLINE_VAR constexpr bool _Is_large = sizeof(_Impl) > _Space_size || alignof(_Impl) > alignof(max_align_t) || !_Impl::_Nothrow_move::value; #if _HAS_FUNCTION_ALLOCATOR_SUPPORT From 0ae9d8fd9e8f52bc74387cd09f27e4d1a5cba2a1 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Fri, 10 Apr 2020 05:41:03 +0300 Subject: [PATCH 22/27] Update tests/std/tests/GH_000690_overaligned_function/test.cpp Co-Authored-By: Stephan T. Lavavej --- tests/std/tests/GH_000690_overaligned_function/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/GH_000690_overaligned_function/test.cpp b/tests/std/tests/GH_000690_overaligned_function/test.cpp index bf340e98fdb..de117989d2e 100644 --- a/tests/std/tests/GH_000690_overaligned_function/test.cpp +++ b/tests/std/tests/GH_000690_overaligned_function/test.cpp @@ -12,7 +12,7 @@ #pragma warning(disable : 4324) // structure was padded due to alignment specifier -// SFO should not happen +// SFO (Small Functor Optimization) should not happen struct alignas(2 * alignof(std::max_align_t)) overaligned_t { char non_empty; From 819a202250df855804364555ea0bd36fecc06418 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Fri, 10 Apr 2020 05:48:14 +0300 Subject: [PATCH 23/27] Pre-C++17 version of test --- tests/std/tests/GH_000690_overaligned_function/env.lst | 2 +- .../std/tests/GH_000690_overaligned_function/test.cpp | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/std/tests/GH_000690_overaligned_function/env.lst b/tests/std/tests/GH_000690_overaligned_function/env.lst index 2de7aab2959..19f025bd0e6 100644 --- a/tests/std/tests/GH_000690_overaligned_function/env.lst +++ b/tests/std/tests/GH_000690_overaligned_function/env.lst @@ -1,4 +1,4 @@ # Copyright (c) Microsoft Corporation. # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -RUNALL_INCLUDE ..\usual_17_matrix.lst +RUNALL_INCLUDE ..\usual_matrix.lst diff --git a/tests/std/tests/GH_000690_overaligned_function/test.cpp b/tests/std/tests/GH_000690_overaligned_function/test.cpp index de117989d2e..b82d1603303 100644 --- a/tests/std/tests/GH_000690_overaligned_function/test.cpp +++ b/tests/std/tests/GH_000690_overaligned_function/test.cpp @@ -6,10 +6,6 @@ #include #include -#ifndef __cpp_aligned_new -#error This test is only valid when C++17 over-aligned allocation is supported -#endif - #pragma warning(disable : 4324) // structure was padded due to alignment specifier // SFO (Small Functor Optimization) should not happen @@ -20,9 +16,13 @@ struct alignas(2 * alignof(std::max_align_t)) overaligned_t { const auto storage_ptr_value = reinterpret_cast(storage); const auto this_ptr_value = reinterpret_cast(this); - // platform-specific behavior not covered by Standard C++, but fine for such test + // Platform-specific behavior not covered by Standard C++, but fine for such test assert(this_ptr_value < storage_ptr_value || this_ptr_value >= storage_ptr_value + storage_size); + + // Before C++17. alignas isn't helpful for aligning allocations via "new" +#ifndef __cpp_aligned_new assert(this_ptr_value % alignof(overaligned_t) == 0); +#endif } }; From 76bfccbdb112b2965b540f2f28719a275c0e40a2 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Fri, 10 Apr 2020 06:26:17 +0300 Subject: [PATCH 24/27] Removing macros, now I have ICE --- stl/inc/atomic | 94 ++++++++++++++++++++--------------------------- stl/inc/xatomic.h | 7 ++++ 2 files changed, 47 insertions(+), 54 deletions(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index 1ce5172cabd..a797c0b8580 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -30,25 +30,9 @@ _STL_DISABLE_CLANG_WARNINGS #if defined(_M_ARM) || defined(_M_ARM64) #define _Memory_barrier() __dmb(0xB) // inner shared data memory barrier #define _Compiler_or_memory_barrier() _Memory_barrier() - -#define _ISO_VOLATILE_STORE8(_Storage, _Value) __iso_volatile_store8(_Atomic_address_as(_Storage), _Value) -#define _ISO_VOLATILE_STORE16(_Storage, _Value) __iso_volatile_store16(_Atomic_address_as(_Storage), _Value) -#define _ISO_VOLATILE_STORE32(_Storage, _Value) __iso_volatile_store32(_Atomic_address_as(_Storage), _Value) -#define _ISO_VOLATILE_STORE64(_Storage, _Value) __iso_volatile_store64(_Atomic_address_as(_Storage), _Value) -#define _ISO_VOLATILE_LOAD8(_Storage) __iso_volatile_load8(_Atomic_address_as(_Storage)) -#define _ISO_VOLATILE_LOAD16(_Storage) __iso_volatile_load16(_Atomic_address_as(_Storage)) - #elif defined(_M_IX86) || defined(_M_X64) // x86/x64 hardware only emits memory barriers inside _Interlocked intrinsics #define _Compiler_or_memory_barrier() _Compiler_barrier() - -#define _ISO_VOLATILE_STORE8(_Storage, _Value) (*_Atomic_address_as(_Storage) = _Value) -#define _ISO_VOLATILE_STORE16(_Storage, _Value) (*_Atomic_address_as(_Storage) = _Value) -#define _ISO_VOLATILE_STORE32(_Storage, _Value) (*_Atomic_address_as(_Storage) = _Value) -#define _ISO_VOLATILE_STORE64(_Storage, _Value) (*_Atomic_address_as(_Storage) = _Value) -#define _ISO_VOLATILE_LOAD8(_Storage) (*_Atomic_address_as(_Storage)) -#define _ISO_VOLATILE_LOAD16(_Storage) (*_Atomic_address_as(_Storage)) - #else // ^^^ x86/x64 / unsupported hardware vvv #error Unsupported hardware #endif // hardware @@ -417,14 +401,15 @@ struct _Atomic_storage<_Ty, 1> { // lock-free using 1-byte intrinsics } void store(const _Ty _Value, const memory_order _Order) noexcept { // store with given memory order + const auto _Mem = _Atomic_address_as(_Storage); const char _As_bytes = _Atomic_reinterpret_as(_Value); switch (_Order) { case memory_order_relaxed: - _ISO_VOLATILE_STORE8(_Storage, _As_bytes); + __iso_volatile_store8(_Mem, _As_bytes); return; case memory_order_release: _Compiler_or_memory_barrier(); - _ISO_VOLATILE_STORE8(_Storage, _As_bytes); + __iso_volatile_store8(_Mem, _As_bytes); return; default: case memory_order_consume: @@ -439,13 +424,15 @@ struct _Atomic_storage<_Ty, 1> { // lock-free using 1-byte intrinsics } _NODISCARD _Ty load() const noexcept { // load with sequential consistency - char _As_bytes = _ISO_VOLATILE_LOAD8(_Storage); + const auto _Mem = _Atomic_address_as(_Storage); + char _As_bytes = __iso_volatile_load8(_Mem); _Compiler_or_memory_barrier(); return reinterpret_cast<_Ty&>(_As_bytes); } _NODISCARD _Ty load(const memory_order _Order) const noexcept { // load with given memory order - char _As_bytes = _ISO_VOLATILE_LOAD8(_Storage); + const auto _Mem = _Atomic_address_as(_Storage); + char _As_bytes = __iso_volatile_load8(_Mem); _Load_barrier(_Order); return reinterpret_cast<_Ty&>(_As_bytes); } @@ -496,14 +483,15 @@ struct _Atomic_storage<_Ty, 2> { // lock-free using 2-byte intrinsics } void store(const _Ty _Value, const memory_order _Order) noexcept { // store with given memory order + const auto _Mem = _Atomic_address_as(_Storage); const short _As_bytes = _Atomic_reinterpret_as(_Value); switch (_Order) { case memory_order_relaxed: - _ISO_VOLATILE_STORE16(_Storage, _As_bytes); + __iso_volatile_store16(_Mem, _As_bytes); return; case memory_order_release: _Compiler_or_memory_barrier(); - _ISO_VOLATILE_STORE16(_Storage, _As_bytes); + __iso_volatile_store16(_Mem, _As_bytes); return; default: case memory_order_consume: @@ -518,13 +506,15 @@ struct _Atomic_storage<_Ty, 2> { // lock-free using 2-byte intrinsics } _NODISCARD _Ty load() const noexcept { // load with sequential consistency - short _As_bytes = _ISO_VOLATILE_LOAD16(_Storage); + const auto _Mem = _Atomic_address_as(_Storage); + short _As_bytes = __iso_volatile_load16(_Mem); _Compiler_or_memory_barrier(); return reinterpret_cast<_Ty&>(_As_bytes); } _NODISCARD _Ty load(const memory_order _Order) const noexcept { // load with given memory order - short _As_bytes = _ISO_VOLATILE_LOAD16(_Storage); + const auto _Mem = _Atomic_address_as(_Storage); + short _As_bytes = __iso_volatile_load16(_Mem); _Load_barrier(_Order); return reinterpret_cast<_Ty&>(_As_bytes); } @@ -563,24 +553,26 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics } void store(const _Ty _Value) noexcept { // store with sequential consistency + const auto _Mem = _Atomic_address_as(_Storage); #if defined(_M_ARM) || defined(_M_ARM64) _Memory_barrier(); - _ISO_VOLATILE_STORE32(_Storage, _Atomic_reinterpret_as(_Value)); + __iso_volatile_store32(_Mem, _Atomic_reinterpret_as(_Value)); _Memory_barrier(); #else // ^^^ ARM32/ARM64 hardware / x86/x64 hardware vvv - (void) _InterlockedExchange(_Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Value)); + (void) _InterlockedExchange(_Mem, _Atomic_reinterpret_as(_Value)); #endif // hardware } void store(const _Ty _Value, const memory_order _Order) noexcept { // store with given memory order + const auto _Mem = _Atomic_address_as(_Storage); const int _As_bytes = _Atomic_reinterpret_as(_Value); switch (_Order) { case memory_order_relaxed: - _ISO_VOLATILE_STORE32(_Storage, _As_bytes); + __iso_volatile_store32(_Mem, _As_bytes); return; case memory_order_release: _Compiler_or_memory_barrier(); - _ISO_VOLATILE_STORE32(_Storage, _As_bytes); + __iso_volatile_store32(_Mem, _As_bytes); return; default: case memory_order_consume: @@ -595,13 +587,15 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics } _NODISCARD _Ty load() const noexcept { // load with sequential consistency - auto _As_bytes = _ISO_VOLATILE_LOAD32(_Storage); + const auto _Mem = _Atomic_address_as(_Storage); + auto _As_bytes = __iso_volatile_load32(_Mem); _Compiler_or_memory_barrier(); return reinterpret_cast<_Ty&>(_As_bytes); } _NODISCARD _Ty load(const memory_order _Order) const noexcept { // load with given memory order - auto _As_bytes = _ISO_VOLATILE_LOAD32(_Storage); + const auto _Mem = _Atomic_address_as(_Storage); + auto _As_bytes = __iso_volatile_load32(_Mem); _Load_barrier(_Order); return reinterpret_cast<_Ty&>(_As_bytes); } @@ -639,35 +633,36 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics // non-atomically initialize this atomic } -#ifdef _M_IX86 - void store(const _Ty _Value, const memory_order _Order = memory_order_seq_cst) noexcept { - // store with (effectively) sequential consistency - _Check_store_memory_order(_Order); - (void) exchange(_Value, _Order); - } -#else // ^^^ _M_IX86 / !_M_IX86 vvv - void store(const _Ty _Value) noexcept { // store with sequential consistency const auto _Mem = _Atomic_address_as(_Storage); const long long _As_bytes = _Atomic_reinterpret_as(_Value); -#ifdef _M_ARM64 +#if defined(_M_IX86) + auto _Comparand = __iso_volatile_load64(_Mem); + for (;;) { + auto _Result = _InterlockedCompareExchange64(_Mem, _As_bytes, _Comparand); + if (_Result == _Comparand) + break; + _Comparand = _Result; + } +#elif defined(_M_ARM64) _Memory_barrier(); __iso_volatile_store64(_Mem, _As_bytes); _Memory_barrier(); -#else // ^^^ _M_ARM64 / ARM32, x64 vvv +#else // ^^^ _M_ARM64 / ARM32, x64 (void) _InterlockedExchange64(_Mem, _As_bytes); #endif // _M_ARM64 } void store(const _Ty _Value, const memory_order _Order) noexcept { // store with given memory order + const auto _Mem = _Atomic_address_as(_Storage); const long long _As_bytes = _Atomic_reinterpret_as(_Value); switch (_Order) { case memory_order_relaxed: - _ISO_VOLATILE_STORE64(_Storage, _As_bytes); + __iso_volatile_store64(_Mem, _As_bytes); return; case memory_order_release: _Compiler_or_memory_barrier(); - _ISO_VOLATILE_STORE64(_Storage, _As_bytes); + __iso_volatile_store64(_Mem, _As_bytes); return; default: case memory_order_consume: @@ -680,7 +675,7 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics return; } } -#endif // _M_IX86 + _NODISCARD _Ty load() const noexcept { // load with sequential consistency const auto _Mem = _Atomic_address_as(_Storage); @@ -688,12 +683,9 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics #if defined(_M_ARM) _As_bytes = __ldrexd(_Mem); _Memory_barrier(); -#elif defined(_M_IX86) || defined(_M_ARM64) +#else // _M_X86/_M_X64/_M_ARM64 _As_bytes = __iso_volatile_load64(_Mem); _Compiler_or_memory_barrier(); -#else // _M_X64 - _As_bytes = *_Mem; - _Compiler_barrier(); #endif // hardware return reinterpret_cast<_Ty&>(_As_bytes); } @@ -2038,13 +2030,7 @@ _STD_END #undef _ATOMIC_CHOOSE_INTRINSIC #undef _ATOMIC_HAS_DCAS -#undef _ISO_VOLATILE_LOAD8 -#undef _ISO_VOLATILE_LOAD16 -// #undef _ISO_VOLATILE_LOAD32 // Used in -#undef _ISO_VOLATILE_STORE8 -#undef _ISO_VOLATILE_STORE16 -#undef _ISO_VOLATILE_STORE32 -#undef _ISO_VOLATILE_STORE64 + #undef _STD_COMPARE_EXCHANGE_128 #undef _INVALID_MEMORY_ORDER #undef _Compiler_or_memory_barrier diff --git a/stl/inc/xatomic.h b/stl/inc/xatomic.h index 6af55a96696..52d055bf127 100644 --- a/stl/inc/xatomic.h +++ b/stl/inc/xatomic.h @@ -104,6 +104,13 @@ _NODISCARD volatile _Integral* _Atomic_address_as(_Ty& _Source) noexcept { return &reinterpret_cast(_Source); } +template +_NODISCARD const volatile _Integral* _Atomic_address_as(const _Ty& _Source) noexcept { + // gets a pointer to the argument as an integral type (to pass to intrinsics) + static_assert(is_integral_v<_Integral>, "Tried to reinterpret memory as non-integral"); + return &reinterpret_cast(_Source); +} + _STD_END #pragma pop_macro("new") From 01d8f43b087f8afe32b21e489a6929b4c0272a26 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Fri, 10 Apr 2020 06:35:31 +0300 Subject: [PATCH 25/27] Revert "Removing macros, now I have ICE" This reverts commit 76bfccbdb112b2965b540f2f28719a275c0e40a2. It was meant for another branch --- stl/inc/atomic | 94 +++++++++++++++++++++++++++-------------------- stl/inc/xatomic.h | 7 ---- 2 files changed, 54 insertions(+), 47 deletions(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index a797c0b8580..1ce5172cabd 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -30,9 +30,25 @@ _STL_DISABLE_CLANG_WARNINGS #if defined(_M_ARM) || defined(_M_ARM64) #define _Memory_barrier() __dmb(0xB) // inner shared data memory barrier #define _Compiler_or_memory_barrier() _Memory_barrier() + +#define _ISO_VOLATILE_STORE8(_Storage, _Value) __iso_volatile_store8(_Atomic_address_as(_Storage), _Value) +#define _ISO_VOLATILE_STORE16(_Storage, _Value) __iso_volatile_store16(_Atomic_address_as(_Storage), _Value) +#define _ISO_VOLATILE_STORE32(_Storage, _Value) __iso_volatile_store32(_Atomic_address_as(_Storage), _Value) +#define _ISO_VOLATILE_STORE64(_Storage, _Value) __iso_volatile_store64(_Atomic_address_as(_Storage), _Value) +#define _ISO_VOLATILE_LOAD8(_Storage) __iso_volatile_load8(_Atomic_address_as(_Storage)) +#define _ISO_VOLATILE_LOAD16(_Storage) __iso_volatile_load16(_Atomic_address_as(_Storage)) + #elif defined(_M_IX86) || defined(_M_X64) // x86/x64 hardware only emits memory barriers inside _Interlocked intrinsics #define _Compiler_or_memory_barrier() _Compiler_barrier() + +#define _ISO_VOLATILE_STORE8(_Storage, _Value) (*_Atomic_address_as(_Storage) = _Value) +#define _ISO_VOLATILE_STORE16(_Storage, _Value) (*_Atomic_address_as(_Storage) = _Value) +#define _ISO_VOLATILE_STORE32(_Storage, _Value) (*_Atomic_address_as(_Storage) = _Value) +#define _ISO_VOLATILE_STORE64(_Storage, _Value) (*_Atomic_address_as(_Storage) = _Value) +#define _ISO_VOLATILE_LOAD8(_Storage) (*_Atomic_address_as(_Storage)) +#define _ISO_VOLATILE_LOAD16(_Storage) (*_Atomic_address_as(_Storage)) + #else // ^^^ x86/x64 / unsupported hardware vvv #error Unsupported hardware #endif // hardware @@ -401,15 +417,14 @@ struct _Atomic_storage<_Ty, 1> { // lock-free using 1-byte intrinsics } void store(const _Ty _Value, const memory_order _Order) noexcept { // store with given memory order - const auto _Mem = _Atomic_address_as(_Storage); const char _As_bytes = _Atomic_reinterpret_as(_Value); switch (_Order) { case memory_order_relaxed: - __iso_volatile_store8(_Mem, _As_bytes); + _ISO_VOLATILE_STORE8(_Storage, _As_bytes); return; case memory_order_release: _Compiler_or_memory_barrier(); - __iso_volatile_store8(_Mem, _As_bytes); + _ISO_VOLATILE_STORE8(_Storage, _As_bytes); return; default: case memory_order_consume: @@ -424,15 +439,13 @@ struct _Atomic_storage<_Ty, 1> { // lock-free using 1-byte intrinsics } _NODISCARD _Ty load() const noexcept { // load with sequential consistency - const auto _Mem = _Atomic_address_as(_Storage); - char _As_bytes = __iso_volatile_load8(_Mem); + char _As_bytes = _ISO_VOLATILE_LOAD8(_Storage); _Compiler_or_memory_barrier(); return reinterpret_cast<_Ty&>(_As_bytes); } _NODISCARD _Ty load(const memory_order _Order) const noexcept { // load with given memory order - const auto _Mem = _Atomic_address_as(_Storage); - char _As_bytes = __iso_volatile_load8(_Mem); + char _As_bytes = _ISO_VOLATILE_LOAD8(_Storage); _Load_barrier(_Order); return reinterpret_cast<_Ty&>(_As_bytes); } @@ -483,15 +496,14 @@ struct _Atomic_storage<_Ty, 2> { // lock-free using 2-byte intrinsics } void store(const _Ty _Value, const memory_order _Order) noexcept { // store with given memory order - const auto _Mem = _Atomic_address_as(_Storage); const short _As_bytes = _Atomic_reinterpret_as(_Value); switch (_Order) { case memory_order_relaxed: - __iso_volatile_store16(_Mem, _As_bytes); + _ISO_VOLATILE_STORE16(_Storage, _As_bytes); return; case memory_order_release: _Compiler_or_memory_barrier(); - __iso_volatile_store16(_Mem, _As_bytes); + _ISO_VOLATILE_STORE16(_Storage, _As_bytes); return; default: case memory_order_consume: @@ -506,15 +518,13 @@ struct _Atomic_storage<_Ty, 2> { // lock-free using 2-byte intrinsics } _NODISCARD _Ty load() const noexcept { // load with sequential consistency - const auto _Mem = _Atomic_address_as(_Storage); - short _As_bytes = __iso_volatile_load16(_Mem); + short _As_bytes = _ISO_VOLATILE_LOAD16(_Storage); _Compiler_or_memory_barrier(); return reinterpret_cast<_Ty&>(_As_bytes); } _NODISCARD _Ty load(const memory_order _Order) const noexcept { // load with given memory order - const auto _Mem = _Atomic_address_as(_Storage); - short _As_bytes = __iso_volatile_load16(_Mem); + short _As_bytes = _ISO_VOLATILE_LOAD16(_Storage); _Load_barrier(_Order); return reinterpret_cast<_Ty&>(_As_bytes); } @@ -553,26 +563,24 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics } void store(const _Ty _Value) noexcept { // store with sequential consistency - const auto _Mem = _Atomic_address_as(_Storage); #if defined(_M_ARM) || defined(_M_ARM64) _Memory_barrier(); - __iso_volatile_store32(_Mem, _Atomic_reinterpret_as(_Value)); + _ISO_VOLATILE_STORE32(_Storage, _Atomic_reinterpret_as(_Value)); _Memory_barrier(); #else // ^^^ ARM32/ARM64 hardware / x86/x64 hardware vvv - (void) _InterlockedExchange(_Mem, _Atomic_reinterpret_as(_Value)); + (void) _InterlockedExchange(_Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Value)); #endif // hardware } void store(const _Ty _Value, const memory_order _Order) noexcept { // store with given memory order - const auto _Mem = _Atomic_address_as(_Storage); const int _As_bytes = _Atomic_reinterpret_as(_Value); switch (_Order) { case memory_order_relaxed: - __iso_volatile_store32(_Mem, _As_bytes); + _ISO_VOLATILE_STORE32(_Storage, _As_bytes); return; case memory_order_release: _Compiler_or_memory_barrier(); - __iso_volatile_store32(_Mem, _As_bytes); + _ISO_VOLATILE_STORE32(_Storage, _As_bytes); return; default: case memory_order_consume: @@ -587,15 +595,13 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics } _NODISCARD _Ty load() const noexcept { // load with sequential consistency - const auto _Mem = _Atomic_address_as(_Storage); - auto _As_bytes = __iso_volatile_load32(_Mem); + auto _As_bytes = _ISO_VOLATILE_LOAD32(_Storage); _Compiler_or_memory_barrier(); return reinterpret_cast<_Ty&>(_As_bytes); } _NODISCARD _Ty load(const memory_order _Order) const noexcept { // load with given memory order - const auto _Mem = _Atomic_address_as(_Storage); - auto _As_bytes = __iso_volatile_load32(_Mem); + auto _As_bytes = _ISO_VOLATILE_LOAD32(_Storage); _Load_barrier(_Order); return reinterpret_cast<_Ty&>(_As_bytes); } @@ -633,36 +639,35 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics // non-atomically initialize this atomic } +#ifdef _M_IX86 + void store(const _Ty _Value, const memory_order _Order = memory_order_seq_cst) noexcept { + // store with (effectively) sequential consistency + _Check_store_memory_order(_Order); + (void) exchange(_Value, _Order); + } +#else // ^^^ _M_IX86 / !_M_IX86 vvv + void store(const _Ty _Value) noexcept { // store with sequential consistency const auto _Mem = _Atomic_address_as(_Storage); const long long _As_bytes = _Atomic_reinterpret_as(_Value); -#if defined(_M_IX86) - auto _Comparand = __iso_volatile_load64(_Mem); - for (;;) { - auto _Result = _InterlockedCompareExchange64(_Mem, _As_bytes, _Comparand); - if (_Result == _Comparand) - break; - _Comparand = _Result; - } -#elif defined(_M_ARM64) +#ifdef _M_ARM64 _Memory_barrier(); __iso_volatile_store64(_Mem, _As_bytes); _Memory_barrier(); -#else // ^^^ _M_ARM64 / ARM32, x64 +#else // ^^^ _M_ARM64 / ARM32, x64 vvv (void) _InterlockedExchange64(_Mem, _As_bytes); #endif // _M_ARM64 } void store(const _Ty _Value, const memory_order _Order) noexcept { // store with given memory order - const auto _Mem = _Atomic_address_as(_Storage); const long long _As_bytes = _Atomic_reinterpret_as(_Value); switch (_Order) { case memory_order_relaxed: - __iso_volatile_store64(_Mem, _As_bytes); + _ISO_VOLATILE_STORE64(_Storage, _As_bytes); return; case memory_order_release: _Compiler_or_memory_barrier(); - __iso_volatile_store64(_Mem, _As_bytes); + _ISO_VOLATILE_STORE64(_Storage, _As_bytes); return; default: case memory_order_consume: @@ -675,7 +680,7 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics return; } } - +#endif // _M_IX86 _NODISCARD _Ty load() const noexcept { // load with sequential consistency const auto _Mem = _Atomic_address_as(_Storage); @@ -683,9 +688,12 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics #if defined(_M_ARM) _As_bytes = __ldrexd(_Mem); _Memory_barrier(); -#else // _M_X86/_M_X64/_M_ARM64 +#elif defined(_M_IX86) || defined(_M_ARM64) _As_bytes = __iso_volatile_load64(_Mem); _Compiler_or_memory_barrier(); +#else // _M_X64 + _As_bytes = *_Mem; + _Compiler_barrier(); #endif // hardware return reinterpret_cast<_Ty&>(_As_bytes); } @@ -2030,7 +2038,13 @@ _STD_END #undef _ATOMIC_CHOOSE_INTRINSIC #undef _ATOMIC_HAS_DCAS - +#undef _ISO_VOLATILE_LOAD8 +#undef _ISO_VOLATILE_LOAD16 +// #undef _ISO_VOLATILE_LOAD32 // Used in +#undef _ISO_VOLATILE_STORE8 +#undef _ISO_VOLATILE_STORE16 +#undef _ISO_VOLATILE_STORE32 +#undef _ISO_VOLATILE_STORE64 #undef _STD_COMPARE_EXCHANGE_128 #undef _INVALID_MEMORY_ORDER #undef _Compiler_or_memory_barrier diff --git a/stl/inc/xatomic.h b/stl/inc/xatomic.h index 52d055bf127..6af55a96696 100644 --- a/stl/inc/xatomic.h +++ b/stl/inc/xatomic.h @@ -104,13 +104,6 @@ _NODISCARD volatile _Integral* _Atomic_address_as(_Ty& _Source) noexcept { return &reinterpret_cast(_Source); } -template -_NODISCARD const volatile _Integral* _Atomic_address_as(const _Ty& _Source) noexcept { - // gets a pointer to the argument as an integral type (to pass to intrinsics) - static_assert(is_integral_v<_Integral>, "Tried to reinterpret memory as non-integral"); - return &reinterpret_cast(_Source); -} - _STD_END #pragma pop_macro("new") From 2f46e69328f3f3caa1b086b16ecde55b1bf9eca0 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Fri, 10 Apr 2020 07:41:19 +0300 Subject: [PATCH 26/27] correct condition --- tests/std/tests/GH_000690_overaligned_function/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/GH_000690_overaligned_function/test.cpp b/tests/std/tests/GH_000690_overaligned_function/test.cpp index b82d1603303..03b99b98e17 100644 --- a/tests/std/tests/GH_000690_overaligned_function/test.cpp +++ b/tests/std/tests/GH_000690_overaligned_function/test.cpp @@ -20,7 +20,7 @@ struct alignas(2 * alignof(std::max_align_t)) overaligned_t { assert(this_ptr_value < storage_ptr_value || this_ptr_value >= storage_ptr_value + storage_size); // Before C++17. alignas isn't helpful for aligning allocations via "new" -#ifndef __cpp_aligned_new +#ifdef __cpp_aligned_new assert(this_ptr_value % alignof(overaligned_t) == 0); #endif } From ec176f2e9fe352b8789dd6b2bc66dcc844166695 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 20 Apr 2020 21:06:51 -0700 Subject: [PATCH 27/27] Small comment changes. --- tests/std/tests/GH_000690_overaligned_function/test.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/std/tests/GH_000690_overaligned_function/test.cpp b/tests/std/tests/GH_000690_overaligned_function/test.cpp index 03b99b98e17..effd03765af 100644 --- a/tests/std/tests/GH_000690_overaligned_function/test.cpp +++ b/tests/std/tests/GH_000690_overaligned_function/test.cpp @@ -16,10 +16,10 @@ struct alignas(2 * alignof(std::max_align_t)) overaligned_t { const auto storage_ptr_value = reinterpret_cast(storage); const auto this_ptr_value = reinterpret_cast(this); - // Platform-specific behavior not covered by Standard C++, but fine for such test + // Platform-specific behavior not covered by Standard C++, but fine for this test assert(this_ptr_value < storage_ptr_value || this_ptr_value >= storage_ptr_value + storage_size); - // Before C++17. alignas isn't helpful for aligning allocations via "new" + // Before C++17, alignas isn't helpful for aligning allocations via "new" #ifdef __cpp_aligned_new assert(this_ptr_value % alignof(overaligned_t) == 0); #endif @@ -34,7 +34,7 @@ struct not_overaligned_t { const auto storage_ptr_value = reinterpret_cast(storage); const auto this_ptr_value = reinterpret_cast(this); - // platform-specific behavior not covered by Standard C++, but fine for such test + // Platform-specific behavior not covered by Standard C++, but fine for this test assert(this_ptr_value >= storage_ptr_value && this_ptr_value < storage_ptr_value + storage_size); } };