[PATCH] D50344: [libc++] Enable aligned allocation based on feature test macro, irrespective of standard
This revision was automatically updated to reflect the committed changes. ldionne marked an inline comment as done. Closed by commit rL339431: [libc++] Enable aligned allocation based on feature test macro, irrespective of… (authored by ldionne, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D50344?vs=159903=160100#toc Repository: rL LLVM https://reviews.llvm.org/D50344 Files: libcxx/trunk/include/__config libcxx/trunk/include/new libcxx/trunk/test/libcxx/memory/aligned_allocation_macro.pass.cpp Index: libcxx/trunk/include/__config === --- libcxx/trunk/include/__config +++ libcxx/trunk/include/__config @@ -988,6 +988,11 @@ # endif #endif // defined(__APPLE__) +#if !defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) && \ +!defined(_LIBCPP_BUILDING_LIBRARY) && \ +(!defined(__cpp_aligned_new) || __cpp_aligned_new < 201606) +# define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION +#endif #if defined(__APPLE__) || defined(__FreeBSD__) #define _LIBCPP_HAS_DEFAULTRUNELOCALE Index: libcxx/trunk/include/new === --- libcxx/trunk/include/new +++ libcxx/trunk/include/new @@ -108,13 +108,6 @@ # define _LIBCPP_HAS_NO_SIZED_DEALLOCATION #endif -#if !defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) && \ -(!(defined(_LIBCPP_BUILDING_LIBRARY) || _LIBCPP_STD_VER > 14 || \ -(defined(__cpp_aligned_new) && __cpp_aligned_new >= 201606))) -# define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION -#endif - - #if !__has_builtin(__builtin_operator_new) || \ __has_builtin(__builtin_operator_new) < 201802L || \ defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) || \ Index: libcxx/trunk/test/libcxx/memory/aligned_allocation_macro.pass.cpp === --- libcxx/trunk/test/libcxx/memory/aligned_allocation_macro.pass.cpp +++ libcxx/trunk/test/libcxx/memory/aligned_allocation_macro.pass.cpp @@ -0,0 +1,25 @@ +//===--===// +// +// The LLVM Compiler Infrastructure +// +// This file is dual licensed under the MIT and the University of Illinois Open +// Source Licenses. See LICENSE.TXT for details. +// +//===--===// + +// UNSUPPORTED: c++98, c++03, c++11, c++14 +// XFAIL: with_system_cxx_lib=macosx10.12 +// XFAIL: with_system_cxx_lib=macosx10.11 +// XFAIL: with_system_cxx_lib=macosx10.10 +// XFAIL: with_system_cxx_lib=macosx10.9 +// XFAIL: with_system_cxx_lib=macosx10.8 +// XFAIL: with_system_cxx_lib=macosx10.7 + +#include + + +#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION +# error "libc++ should have aligned allocation in C++17 and up when targeting a platform that supports it" +#endif + +int main() { } Index: libcxx/trunk/include/__config === --- libcxx/trunk/include/__config +++ libcxx/trunk/include/__config @@ -988,6 +988,11 @@ # endif #endif // defined(__APPLE__) +#if !defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) && \ +!defined(_LIBCPP_BUILDING_LIBRARY) && \ +(!defined(__cpp_aligned_new) || __cpp_aligned_new < 201606) +# define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION +#endif #if defined(__APPLE__) || defined(__FreeBSD__) #define _LIBCPP_HAS_DEFAULTRUNELOCALE Index: libcxx/trunk/include/new === --- libcxx/trunk/include/new +++ libcxx/trunk/include/new @@ -108,13 +108,6 @@ # define _LIBCPP_HAS_NO_SIZED_DEALLOCATION #endif -#if !defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) && \ -(!(defined(_LIBCPP_BUILDING_LIBRARY) || _LIBCPP_STD_VER > 14 || \ -(defined(__cpp_aligned_new) && __cpp_aligned_new >= 201606))) -# define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION -#endif - - #if !__has_builtin(__builtin_operator_new) || \ __has_builtin(__builtin_operator_new) < 201802L || \ defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) || \ Index: libcxx/trunk/test/libcxx/memory/aligned_allocation_macro.pass.cpp === --- libcxx/trunk/test/libcxx/memory/aligned_allocation_macro.pass.cpp +++ libcxx/trunk/test/libcxx/memory/aligned_allocation_macro.pass.cpp @@ -0,0 +1,25 @@ +//===--===// +// +// The LLVM Compiler Infrastructure +// +// This file is dual licensed under the MIT and the University of Illinois Open +// Source Licenses. See LICENSE.TXT for details. +// +//===--===// + +// UNSUPPORTED: c++98, c++03, c++11, c++14 +// XFAIL: with_system_cxx_lib=macosx10.12 +// XFAIL: with_system_cxx_lib=macosx10.11 +// XFAIL: with_system_cxx_lib=macosx10.10 +// XFAIL: with_system_cxx_lib=macosx10.9 +// XFAIL:
[PATCH] D50344: [libc++] Enable aligned allocation based on feature test macro, irrespective of standard
vsapsai accepted this revision. vsapsai added inline comments. This revision is now accepted and ready to land. Comment at: libcxx/include/new:111-116 #if !__has_builtin(__builtin_operator_new) || \ __has_builtin(__builtin_operator_new) < 201802L || \ defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) || \ !defined(__cpp_aligned_new) || __cpp_aligned_new < 201606 #define _LIBCPP_HAS_NO_BUILTIN_ALIGNED_OPERATOR_NEW_DELETE #endif ldionne wrote: > vsapsai wrote: > > Maybe move this to `__config` too? This way we'll have > > `__cpp_aligned_new`-related macros together. > The big difference is that > `_LIBCPP_HAS_NO_BUILTIN_ALIGNED_OPERATOR_NEW_DELETE` is only used in ``, > whereas `_LIBCPP_HAS_NO_ALIGNED_ALLOCATION` was used in other files as well. > Hence, I feel like it makes more sense to lift > `_LIBCPP_HAS_NO_ALIGNED_ALLOCATION` into `<__config>`, but not > `_LIBCPP_HAS_NO_BUILTIN_ALIGNED_OPERATOR_NEW_DELETE `. Agree with your reasoning. Comment at: libcxx/test/libcxx/memory/aligned_allocation_macro.pass.cpp:11 +// UNSUPPORTED: c++98, c++03, c++11, c++14 +// XFAIL: with_system_cxx_lib=macosx10.12 +// XFAIL: with_system_cxx_lib=macosx10.11 ldionne wrote: > vsapsai wrote: > > Initially `with_system_cxx_lib` made me suspicious because macro > > `_LIBCPP_HAS_NO_ALIGNED_ALLOCATION` doesn't need specific dylib. And > > `XFAIL: availability=macosx10.12` seems to be working. > > > > [Documentation](https://github.com/llvm-mirror/libcxx/blob/master/docs/DesignDocs/AvailabilityMarkup.rst#testing) > > describes which feature should be used in different cases but in this case > > I cannot definitely say if test uses unavailable feature. I think it is > > acceptable to stick with `with_system_cxx_lib` but I decided to brought to > > your attention the alternative. > My understanding is that the `availability` feature may not be there if we're > not using availability macros, since they can be disabled entirely. `availability` feature [will be present](https://github.com/llvm-mirror/libcxx/blob/5428c6b9d64dd4cc65fd5665c977c11dc6f8a547/utils/libcxx/test/config.py#L403-L426) when tests are invoked with `use_system_cxx_lib`. I suspect in this case both `with_system_cxx_lib` and `availability` will work, so let's keep `with_system_cxx_lib`. Repository: rCXX libc++ https://reviews.llvm.org/D50344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50344: [libc++] Enable aligned allocation based on feature test macro, irrespective of standard
ldionne marked an inline comment as done. ldionne added inline comments. Comment at: libcxx/test/libcxx/memory/aligned_allocation_macro.pass.cpp:11 +// UNSUPPORTED: c++98, c++03, c++11, c++14 +// XFAIL: with_system_cxx_lib=macosx10.12 +// XFAIL: with_system_cxx_lib=macosx10.11 vsapsai wrote: > Initially `with_system_cxx_lib` made me suspicious because macro > `_LIBCPP_HAS_NO_ALIGNED_ALLOCATION` doesn't need specific dylib. And `XFAIL: > availability=macosx10.12` seems to be working. > > [Documentation](https://github.com/llvm-mirror/libcxx/blob/master/docs/DesignDocs/AvailabilityMarkup.rst#testing) > describes which feature should be used in different cases but in this case I > cannot definitely say if test uses unavailable feature. I think it is > acceptable to stick with `with_system_cxx_lib` but I decided to brought to > your attention the alternative. My understanding is that the `availability` feature may not be there if we're not using availability macros, since they can be disabled entirely. Repository: rCXX libc++ https://reviews.llvm.org/D50344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50344: [libc++] Enable aligned allocation based on feature test macro, irrespective of standard
ldionne added inline comments. Comment at: libcxx/include/new:111-116 #if !__has_builtin(__builtin_operator_new) || \ __has_builtin(__builtin_operator_new) < 201802L || \ defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) || \ !defined(__cpp_aligned_new) || __cpp_aligned_new < 201606 #define _LIBCPP_HAS_NO_BUILTIN_ALIGNED_OPERATOR_NEW_DELETE #endif vsapsai wrote: > Maybe move this to `__config` too? This way we'll have > `__cpp_aligned_new`-related macros together. The big difference is that `_LIBCPP_HAS_NO_BUILTIN_ALIGNED_OPERATOR_NEW_DELETE` is only used in ``, whereas `_LIBCPP_HAS_NO_ALIGNED_ALLOCATION` was used in other files as well. Hence, I feel like it makes more sense to lift `_LIBCPP_HAS_NO_ALIGNED_ALLOCATION` into `<__config>`, but not `_LIBCPP_HAS_NO_BUILTIN_ALIGNED_OPERATOR_NEW_DELETE `. Repository: rCXX libc++ https://reviews.llvm.org/D50344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50344: [libc++] Enable aligned allocation based on feature test macro, irrespective of standard
ldionne updated this revision to Diff 159903. ldionne marked an inline comment as done. ldionne added a comment. Address vsapsai's comments Repository: rCXX libc++ https://reviews.llvm.org/D50344 Files: libcxx/include/__config libcxx/include/new libcxx/test/libcxx/memory/aligned_allocation_macro.pass.cpp Index: libcxx/test/libcxx/memory/aligned_allocation_macro.pass.cpp === --- /dev/null +++ libcxx/test/libcxx/memory/aligned_allocation_macro.pass.cpp @@ -0,0 +1,25 @@ +//===--===// +// +// The LLVM Compiler Infrastructure +// +// This file is dual licensed under the MIT and the University of Illinois Open +// Source Licenses. See LICENSE.TXT for details. +// +//===--===// + +// UNSUPPORTED: c++98, c++03, c++11, c++14 +// XFAIL: with_system_cxx_lib=macosx10.12 +// XFAIL: with_system_cxx_lib=macosx10.11 +// XFAIL: with_system_cxx_lib=macosx10.10 +// XFAIL: with_system_cxx_lib=macosx10.9 +// XFAIL: with_system_cxx_lib=macosx10.8 +// XFAIL: with_system_cxx_lib=macosx10.7 + +#include + + +#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION +# error "libc++ should have aligned allocation in C++17 and up when targeting a platform that supports it" +#endif + +int main() { } Index: libcxx/include/new === --- libcxx/include/new +++ libcxx/include/new @@ -108,13 +108,6 @@ # define _LIBCPP_HAS_NO_SIZED_DEALLOCATION #endif -#if !defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) && \ -(!(defined(_LIBCPP_BUILDING_LIBRARY) || _LIBCPP_STD_VER > 14 || \ -(defined(__cpp_aligned_new) && __cpp_aligned_new >= 201606))) -# define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION -#endif - - #if !__has_builtin(__builtin_operator_new) || \ __has_builtin(__builtin_operator_new) < 201802L || \ defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) || \ Index: libcxx/include/__config === --- libcxx/include/__config +++ libcxx/include/__config @@ -988,6 +988,11 @@ # endif #endif // defined(__APPLE__) +#if !defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) && \ +!defined(_LIBCPP_BUILDING_LIBRARY) && \ +(!defined(__cpp_aligned_new) || __cpp_aligned_new < 201606) +# define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION +#endif #if defined(__APPLE__) || defined(__FreeBSD__) #define _LIBCPP_HAS_DEFAULTRUNELOCALE Index: libcxx/test/libcxx/memory/aligned_allocation_macro.pass.cpp === --- /dev/null +++ libcxx/test/libcxx/memory/aligned_allocation_macro.pass.cpp @@ -0,0 +1,25 @@ +//===--===// +// +// The LLVM Compiler Infrastructure +// +// This file is dual licensed under the MIT and the University of Illinois Open +// Source Licenses. See LICENSE.TXT for details. +// +//===--===// + +// UNSUPPORTED: c++98, c++03, c++11, c++14 +// XFAIL: with_system_cxx_lib=macosx10.12 +// XFAIL: with_system_cxx_lib=macosx10.11 +// XFAIL: with_system_cxx_lib=macosx10.10 +// XFAIL: with_system_cxx_lib=macosx10.9 +// XFAIL: with_system_cxx_lib=macosx10.8 +// XFAIL: with_system_cxx_lib=macosx10.7 + +#include + + +#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION +# error "libc++ should have aligned allocation in C++17 and up when targeting a platform that supports it" +#endif + +int main() { } Index: libcxx/include/new === --- libcxx/include/new +++ libcxx/include/new @@ -108,13 +108,6 @@ # define _LIBCPP_HAS_NO_SIZED_DEALLOCATION #endif -#if !defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) && \ -(!(defined(_LIBCPP_BUILDING_LIBRARY) || _LIBCPP_STD_VER > 14 || \ -(defined(__cpp_aligned_new) && __cpp_aligned_new >= 201606))) -# define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION -#endif - - #if !__has_builtin(__builtin_operator_new) || \ __has_builtin(__builtin_operator_new) < 201802L || \ defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) || \ Index: libcxx/include/__config === --- libcxx/include/__config +++ libcxx/include/__config @@ -988,6 +988,11 @@ # endif #endif // defined(__APPLE__) +#if !defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) && \ +!defined(_LIBCPP_BUILDING_LIBRARY) && \ +(!defined(__cpp_aligned_new) || __cpp_aligned_new < 201606) +# define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION +#endif #if defined(__APPLE__) || defined(__FreeBSD__) #define _LIBCPP_HAS_DEFAULTRUNELOCALE ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50344: [libc++] Enable aligned allocation based on feature test macro, irrespective of standard
vsapsai added inline comments. Comment at: libcxx/include/__config:993 +!defined(_LIBCPP_BUILDING_LIBRARY) && \ +!defined(__cpp_aligned_new) +# define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION I think I'd rather keep `__cpp_aligned_new >= 201606` check. I don't know about cases where the smaller value is possible, Clang always had 201606 as far as I (and SVN) know. But it seems to be safer and more correct. Don't have objective arguments for keeping the check, so if you think it's not worth it, I trust your judgement. Comment at: libcxx/include/new:111-116 #if !__has_builtin(__builtin_operator_new) || \ __has_builtin(__builtin_operator_new) < 201802L || \ defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) || \ !defined(__cpp_aligned_new) || __cpp_aligned_new < 201606 #define _LIBCPP_HAS_NO_BUILTIN_ALIGNED_OPERATOR_NEW_DELETE #endif Maybe move this to `__config` too? This way we'll have `__cpp_aligned_new`-related macros together. Comment at: libcxx/test/libcxx/memory/aligned_allocation_macro.pass.cpp:11 +// UNSUPPORTED: c++98, c++03, c++11, c++14 +// XFAIL: with_system_cxx_lib=macosx10.12 +// XFAIL: with_system_cxx_lib=macosx10.11 Initially `with_system_cxx_lib` made me suspicious because macro `_LIBCPP_HAS_NO_ALIGNED_ALLOCATION` doesn't need specific dylib. And `XFAIL: availability=macosx10.12` seems to be working. [Documentation](https://github.com/llvm-mirror/libcxx/blob/master/docs/DesignDocs/AvailabilityMarkup.rst#testing) describes which feature should be used in different cases but in this case I cannot definitely say if test uses unavailable feature. I think it is acceptable to stick with `with_system_cxx_lib` but I decided to brought to your attention the alternative. Repository: rCXX libc++ https://reviews.llvm.org/D50344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50344: [libc++] Enable aligned allocation based on feature test macro, irrespective of standard
ldionne created this revision. ldionne added a reviewer: vsapsai. Herald added a reviewer: EricWF. Herald added subscribers: cfe-commits, dexonsmith, christof. The current code enables aligned allocation functions when compiling in C++17 and later. This is a problem because aligned allocation functions might not be supported on the target platform, which leads to an error at link time. Since r338934, Clang knows not to define __cpp_aligned_new when it's not available on the target platform -- this commit takes advantage of that to only use aligned allocation functions when they are available. Repository: rCXX libc++ https://reviews.llvm.org/D50344 Files: libcxx/include/__config libcxx/include/new libcxx/test/libcxx/memory/aligned_allocation_macro.pass.cpp Index: libcxx/test/libcxx/memory/aligned_allocation_macro.pass.cpp === --- /dev/null +++ libcxx/test/libcxx/memory/aligned_allocation_macro.pass.cpp @@ -0,0 +1,25 @@ +//===--===// +// +// The LLVM Compiler Infrastructure +// +// This file is dual licensed under the MIT and the University of Illinois Open +// Source Licenses. See LICENSE.TXT for details. +// +//===--===// + +// UNSUPPORTED: c++98, c++03, c++11, c++14 +// XFAIL: with_system_cxx_lib=macosx10.12 +// XFAIL: with_system_cxx_lib=macosx10.11 +// XFAIL: with_system_cxx_lib=macosx10.10 +// XFAIL: with_system_cxx_lib=macosx10.9 +// XFAIL: with_system_cxx_lib=macosx10.8 +// XFAIL: with_system_cxx_lib=macosx10.7 + +#include + + +#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION +# error "libc++ should have aligned allocation in C++17 and up when targeting a platform that supports it" +#endif + +int main() { } Index: libcxx/include/new === --- libcxx/include/new +++ libcxx/include/new @@ -108,13 +108,6 @@ # define _LIBCPP_HAS_NO_SIZED_DEALLOCATION #endif -#if !defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) && \ -(!(defined(_LIBCPP_BUILDING_LIBRARY) || _LIBCPP_STD_VER > 14 || \ -(defined(__cpp_aligned_new) && __cpp_aligned_new >= 201606))) -# define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION -#endif - - #if !__has_builtin(__builtin_operator_new) || \ __has_builtin(__builtin_operator_new) < 201802L || \ defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) || \ Index: libcxx/include/__config === --- libcxx/include/__config +++ libcxx/include/__config @@ -988,6 +988,11 @@ # endif #endif // defined(__APPLE__) +#if !defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) && \ +!defined(_LIBCPP_BUILDING_LIBRARY) && \ +!defined(__cpp_aligned_new) +# define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION +#endif #if defined(__APPLE__) || defined(__FreeBSD__) #define _LIBCPP_HAS_DEFAULTRUNELOCALE Index: libcxx/test/libcxx/memory/aligned_allocation_macro.pass.cpp === --- /dev/null +++ libcxx/test/libcxx/memory/aligned_allocation_macro.pass.cpp @@ -0,0 +1,25 @@ +//===--===// +// +// The LLVM Compiler Infrastructure +// +// This file is dual licensed under the MIT and the University of Illinois Open +// Source Licenses. See LICENSE.TXT for details. +// +//===--===// + +// UNSUPPORTED: c++98, c++03, c++11, c++14 +// XFAIL: with_system_cxx_lib=macosx10.12 +// XFAIL: with_system_cxx_lib=macosx10.11 +// XFAIL: with_system_cxx_lib=macosx10.10 +// XFAIL: with_system_cxx_lib=macosx10.9 +// XFAIL: with_system_cxx_lib=macosx10.8 +// XFAIL: with_system_cxx_lib=macosx10.7 + +#include + + +#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION +# error "libc++ should have aligned allocation in C++17 and up when targeting a platform that supports it" +#endif + +int main() { } Index: libcxx/include/new === --- libcxx/include/new +++ libcxx/include/new @@ -108,13 +108,6 @@ # define _LIBCPP_HAS_NO_SIZED_DEALLOCATION #endif -#if !defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) && \ -(!(defined(_LIBCPP_BUILDING_LIBRARY) || _LIBCPP_STD_VER > 14 || \ -(defined(__cpp_aligned_new) && __cpp_aligned_new >= 201606))) -# define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION -#endif - - #if !__has_builtin(__builtin_operator_new) || \ __has_builtin(__builtin_operator_new) < 201802L || \ defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) || \ Index: libcxx/include/__config === --- libcxx/include/__config +++ libcxx/include/__config @@ -988,6 +988,11 @@ # endif #endif // defined(__APPLE__) +#if !defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) && \ +