Eric and I discussed this further on IRC, and r314965 implements his suggestions.
From: Eric Fiselier <e...@efcs.ca> Date: Wednesday, October 4, 2017 at 6:11 PM To: Shoaib Meenai <smee...@fb.com> Cc: cfe-commits <cfe-commits@lists.llvm.org> Subject: Re: [libcxx] r314949 - [libc++] Allow users to explicitly specify ABI On Wed, Oct 4, 2017 at 5:44 PM, Shoaib Meenai via cfe-commits <cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote: Author: smeenai Date: Wed Oct 4 16:44:38 2017 New Revision: 314949 URL: http://llvm.org/viewvc/llvm-project?rev=314949&view=rev<https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D314949-26view-3Drev&d=DwMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=FfTS_aoYUPTcrKzdSBjzDilbxxSRUA__6T0fQtUoDw4&s=cckMmTlQdJTp0e-PZSnITzPCbwRVbumfloKSl3KQOGM&e=> Log: [libc++] Allow users to explicitly specify ABI libc++'s current heuristic for detecting Itanium vs. Microsoft ABI falls short in some cases. For example, it will detect windows-itanium targets as using the Microsoft ABI, since they set `_MSC_VER` (for compatibility with Microsoft headers). Leave the current heuristic in place by default but also allow users to explicitly specify the ABI if need be. Modified: libcxx/trunk/CMakeLists.txt libcxx/trunk/include/__config libcxx/trunk/include/__config_site.in<https://urldefense.proofpoint.com/v2/url?u=http-3A__config-5Fsite.in&d=DwMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=FfTS_aoYUPTcrKzdSBjzDilbxxSRUA__6T0fQtUoDw4&s=DFPNV_9W5sjp5dUlpv6oF__hPknbPHkZ22xnaZhWOfQ&e=> Modified: libcxx/trunk/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/CMakeLists.txt?rev=314949&r1=314948&r2=314949&view=diff<https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_libcxx_trunk_CMakeLists.txt-3Frev-3D314949-26r1-3D314948-26r2-3D314949-26view-3Ddiff&d=DwMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=FfTS_aoYUPTcrKzdSBjzDilbxxSRUA__6T0fQtUoDw4&s=LJoAA9_RLFbfZHWQ0omY5NNRseSbizGWiRSUeJPtuSY&e=> ============================================================================== --- libcxx/trunk/CMakeLists.txt (original) +++ libcxx/trunk/CMakeLists.txt Wed Oct 4 16:44:38 2017 @@ -99,6 +99,8 @@ cmake_dependent_option(LIBCXX_INSTALL_EX "LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY;LIBCXX_INSTALL_LIBRARY" OFF) set(LIBCXX_ABI_VERSION 1 CACHE STRING "ABI version of libc++.") option(LIBCXX_ABI_UNSTABLE "Unstable ABI of libc++." OFF) +option(LIBCXX_ABI_ITANIUM "Ignore auto-detection and force use of the Itanium ABI.") +option(LIBCXX_ABI_MICROSOFT "Ignore auto-detection and force use of the Microsoft ABI.") Shouldn't these specify a default option? option(LIBCXX_USE_COMPILER_RT "Use compiler-rt instead of libgcc" OFF) if (NOT LIBCXX_ENABLE_SHARED AND NOT LIBCXX_ENABLE_STATIC) @@ -337,6 +339,10 @@ if (LIBCXX_HAS_MUSL_LIBC AND NOT LIBCXX_ "when building for Musl with LIBCXX_HAS_MUSL_LIBC.") endif() +if (LIBCXX_ABI_ITANIUM AND LIBCXX_ABI_MICROSOFT) + message(FATAL_ERROR "Only one of LIBCXX_ABI_ITANIUM and LIBCXX_ABI_MICROSOFT can be specified.") +endif () + #=============================================================================== # Configure System #=============================================================================== @@ -594,6 +600,8 @@ if (NOT LIBCXX_ABI_VERSION EQUAL "1") config_define(${LIBCXX_ABI_VERSION} _LIBCPP_ABI_VERSION) endif() config_define_if(LIBCXX_ABI_UNSTABLE _LIBCPP_ABI_UNSTABLE) +config_define_if(LIBCXX_ABI_ITANIUM _LIBCPP_ABI_ITANIUM) +config_define_if(LIBCXX_ABI_MICROSOFT _LIBCPP_ABI_MICROSOFT) I'm not a fan of the direction this is going in. It seems to require the generation and use of a __config_site header in all cases where it's used. I don't think we want to require that in the common case where you're using Itanium on Linux or Windows. However, like you said, the attempt of this is to override the automatic detection done in the headers. Maybe the name should reflect that (Ex. _LIBCPP_ABI_ITANIUM_OVERRIDE)? And then the autodetection can operate by checking for an override first? config_define_if_not(LIBCXX_ENABLE_GLOBAL_FILESYSTEM_NAMESPACE _LIBCPP_HAS_NO_GLOBAL_FILESYSTEM_NAMESPACE) config_define_if_not(LIBCXX_ENABLE_STDIN _LIBCPP_HAS_NO_STDIN) Modified: libcxx/trunk/include/__config URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__config?rev=314949&r1=314948&r2=314949&view=diff<https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_libcxx_trunk_include_-5F-5Fconfig-3Frev-3D314949-26r1-3D314948-26r2-3D314949-26view-3Ddiff&d=DwMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=FfTS_aoYUPTcrKzdSBjzDilbxxSRUA__6T0fQtUoDw4&s=tvy2iLdreg0H7Q_MnfrAK_3incAXulpdzljgAwQKG-w&e=> ============================================================================== --- libcxx/trunk/include/__config (original) +++ libcxx/trunk/include/__config Wed Oct 4 16:44:38 2017 @@ -157,11 +157,15 @@ // FIXME: ABI detection should be done via compiler builtin macros. This // is just a placeholder until Clang implements such macros. For now assume -// that Windows compilers pretending to be MSVC++ target the microsoft ABI. -#if defined(_WIN32) && defined(_MSC_VER) -# define _LIBCPP_ABI_MICROSOFT -#else -# define _LIBCPP_ABI_ITANIUM +// that Windows compilers pretending to be MSVC++ target the Microsoft ABI, +// and allow the user to explicitly specify the ABI to handle cases where this +// heuristic falls short. +#if !defined(_LIBCPP_ABI_ITANIUM) && !defined(_LIBCPP_ABI_MICROSOFT) +# if defined(_WIN32) && defined(_MSC_VER) +# define _LIBCPP_ABI_MICROSOFT +# else +# define _LIBCPP_ABI_ITANIUM +# endif #endif // Need to detect which libc we're using if we're on Linux. Modified: libcxx/trunk/include/__config_site.in<https://urldefense.proofpoint.com/v2/url?u=http-3A__config-5Fsite.in&d=DwMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=FfTS_aoYUPTcrKzdSBjzDilbxxSRUA__6T0fQtUoDw4&s=DFPNV_9W5sjp5dUlpv6oF__hPknbPHkZ22xnaZhWOfQ&e=> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__config_site.in?rev=314949&r1=314948&r2=314949&view=diff<https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_libcxx_trunk_include_-5F-5Fconfig-5Fsite.in-3Frev-3D314949-26r1-3D314948-26r2-3D314949-26view-3Ddiff&d=DwMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=FfTS_aoYUPTcrKzdSBjzDilbxxSRUA__6T0fQtUoDw4&s=Rg-KKft-HF2-v-QJ1nWPRgmlFH_1lwORx4JGwgU6vXA&e=> ============================================================================== --- libcxx/trunk/include/__config_site.in<https://urldefense.proofpoint.com/v2/url?u=http-3A__config-5Fsite.in&d=DwMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=FfTS_aoYUPTcrKzdSBjzDilbxxSRUA__6T0fQtUoDw4&s=DFPNV_9W5sjp5dUlpv6oF__hPknbPHkZ22xnaZhWOfQ&e=> (original) +++ libcxx/trunk/include/__config_site.in<https://urldefense.proofpoint.com/v2/url?u=http-3A__config-5Fsite.in&d=DwMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=FfTS_aoYUPTcrKzdSBjzDilbxxSRUA__6T0fQtUoDw4&s=DFPNV_9W5sjp5dUlpv6oF__hPknbPHkZ22xnaZhWOfQ&e=> Wed Oct 4 16:44:38 2017 @@ -12,6 +12,8 @@ #cmakedefine _LIBCPP_ABI_VERSION @_LIBCPP_ABI_VERSION@ #cmakedefine _LIBCPP_ABI_UNSTABLE +#cmakedefine _LIBCPP_ABI_ITANIUM +#cmakedefine _LIBCPP_ABI_MICROSOFT #cmakedefine _LIBCPP_HAS_NO_GLOBAL_FILESYSTEM_NAMESPACE #cmakedefine _LIBCPP_HAS_NO_STDIN #cmakedefine _LIBCPP_HAS_NO_STDOUT _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits<https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_cfe-2Dcommits&d=DwMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=FfTS_aoYUPTcrKzdSBjzDilbxxSRUA__6T0fQtUoDw4&s=dZkfFqYiz1I7piI6omswkTMBWfOVWN57_yumHBs6O3M&e=>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
