Re: [committed] libstdc++: Only use __builtin_sprintf if supported [PR 96083]
On 16/12/20 17:02 +, Jonathan Wakely wrote:
On 16/12/20 09:23 -0700, Martin Sebor via Libstdc++ wrote:
On 12/16/20 8:00 AM, Jonathan Wakely via Gcc-patches wrote:
Clang doesn't support __builtin_sprintf, so use std::sprintf instead.
libstdc++-v3/ChangeLog:
PR libstdc++/96083
* include/ext/throw_allocator.h: Use __has_builtin to check for
__builtin_sprintf support, and use std::sprtinf if necessary.
Tested powerpc64le-linux. Committed to trunk.
I expected to see the test itself guarded by #ifdef __has_builtin
like in . Or is this code only [meant to be] used with
compilers that understand __has_builtin? (I'm mostly just curious
here about support for other compilers like ICC, not necessarily
pointing out a problem with the patch.)
All recent versions of Intel and Clang support __has_builtin.
(For Intel __has_builtin(__builtin_sprintf) is true, for Clang it's
false. But they both support checking it.)
The #ifdef __has_builtin in was added more than three years
ago, before GCC supported __has_builtin. So that check was there to
make the code work with GCC, not other compilers.
Now that GCC supports it, I can simplify that.
Like so.
Tested powerpc64-linux, committed to trunk.
commit 4d4f82959aa0802611f1183389c4c74d22431e49
Author: Jonathan Wakely
Date: Wed Dec 16 17:18:10 2020
libstdc++: Simplify built-in detection in
Now that GCC supports __has_builtin there is no need to test whether
it's defined, we can just use it unconditionally.
libstdc++-v3/ChangeLog:
* include/std/utility: Use __has_builtin without checking if
it's defined.
diff --git a/libstdc++-v3/include/std/utility b/libstdc++-v3/include/std/utility
index 4a9ad604cbc..61573f42f3f 100644
--- a/libstdc++-v3/include/std/utility
+++ b/libstdc++-v3/include/std/utility
@@ -297,27 +297,23 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// extract the elements in a tuple.
template struct _Index_tuple { };
-#ifdef __has_builtin
-# if __has_builtin(__make_integer_seq)
-# define _GLIBCXX_USE_MAKE_INTEGER_SEQ 1
-# endif
-#endif
-
// Builds an _Index_tuple<0, 1, 2, ..., _Num-1>.
template
struct _Build_index_tuple
{
-#if _GLIBCXX_USE_MAKE_INTEGER_SEQ
+#if __has_builtin(__make_integer_seq)
template
using _IdxTuple = _Index_tuple<_Indices...>;
+ // Clang defines __make_integer_seq for this purpose.
using __type = __make_integer_seq<_IdxTuple, size_t, _Num>;
#else
+ // For GCC and other compilers, use __integer_pack instead.
using __type = _Index_tuple<__integer_pack(_Num)...>;
#endif
};
-#if __cplusplus > 201103L
+#if __cplusplus >= 201402L
#define __cpp_lib_integer_sequence 201304
@@ -332,14 +328,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
/// Alias template make_integer_sequence
template
using make_integer_sequence
-#if _GLIBCXX_USE_MAKE_INTEGER_SEQ
+#if __has_builtin(__make_integer_seq)
= __make_integer_seq;
#else
= integer_sequence<_Tp, __integer_pack(_Num)...>;
#endif
-#undef _GLIBCXX_USE_MAKE_INTEGER_SEQ
-
/// Alias template index_sequence
template
using index_sequence = integer_sequence;
Re: [committed] libstdc++: Only use __builtin_sprintf if supported [PR 96083]
On 12/16/20 10:04 AM, Jonathan Wakely wrote: On 16/12/20 17:02 +, Jonathan Wakely wrote: On 16/12/20 09:23 -0700, Martin Sebor via Libstdc++ wrote: On 12/16/20 8:00 AM, Jonathan Wakely via Gcc-patches wrote: Clang doesn't support __builtin_sprintf, so use std::sprintf instead. libstdc++-v3/ChangeLog: PR libstdc++/96083 * include/ext/throw_allocator.h: Use __has_builtin to check for __builtin_sprintf support, and use std::sprtinf if necessary. Tested powerpc64le-linux. Committed to trunk. I expected to see the test itself guarded by #ifdef __has_builtin like in . Or is this code only [meant to be] used with compilers that understand __has_builtin? (I'm mostly just curious here about support for other compilers like ICC, not necessarily pointing out a problem with the patch.) All recent versions of Intel and Clang support __has_builtin. (For Intel __has_builtin(__builtin_sprintf) is true, for Clang it's false. But they both support checking it.) I recently discussed this subject with Judy Ward at Intel, and their preference is for libstdc++ to use __has_builtin, see 6aa12274007bccbae2691a9d336c37fe167bb535 for a recent change to fix feature detection for Intel. Looks good, thanks. Martin
Re: [committed] libstdc++: Only use __builtin_sprintf if supported [PR 96083]
On 16/12/20 17:02 +, Jonathan Wakely wrote: On 16/12/20 09:23 -0700, Martin Sebor via Libstdc++ wrote: On 12/16/20 8:00 AM, Jonathan Wakely via Gcc-patches wrote: Clang doesn't support __builtin_sprintf, so use std::sprintf instead. libstdc++-v3/ChangeLog: PR libstdc++/96083 * include/ext/throw_allocator.h: Use __has_builtin to check for __builtin_sprintf support, and use std::sprtinf if necessary. Tested powerpc64le-linux. Committed to trunk. I expected to see the test itself guarded by #ifdef __has_builtin like in . Or is this code only [meant to be] used with compilers that understand __has_builtin? (I'm mostly just curious here about support for other compilers like ICC, not necessarily pointing out a problem with the patch.) All recent versions of Intel and Clang support __has_builtin. (For Intel __has_builtin(__builtin_sprintf) is true, for Clang it's false. But they both support checking it.) I recently discussed this subject with Judy Ward at Intel, and their preference is for libstdc++ to use __has_builtin, see 6aa12274007bccbae2691a9d336c37fe167bb535 for a recent change to fix feature detection for Intel.
Re: [committed] libstdc++: Only use __builtin_sprintf if supported [PR 96083]
On 16/12/20 09:23 -0700, Martin Sebor via Libstdc++ wrote: On 12/16/20 8:00 AM, Jonathan Wakely via Gcc-patches wrote: Clang doesn't support __builtin_sprintf, so use std::sprintf instead. libstdc++-v3/ChangeLog: PR libstdc++/96083 * include/ext/throw_allocator.h: Use __has_builtin to check for __builtin_sprintf support, and use std::sprtinf if necessary. Tested powerpc64le-linux. Committed to trunk. I expected to see the test itself guarded by #ifdef __has_builtin like in . Or is this code only [meant to be] used with compilers that understand __has_builtin? (I'm mostly just curious here about support for other compilers like ICC, not necessarily pointing out a problem with the patch.) All recent versions of Intel and Clang support __has_builtin. (For Intel __has_builtin(__builtin_sprintf) is true, for Clang it's false. But they both support checking it.) The #ifdef __has_builtin in was added more than three years ago, before GCC supported __has_builtin. So that check was there to make the code work with GCC, not other compilers. Now that GCC supports it, I can simplify that.
Re: [committed] libstdc++: Only use __builtin_sprintf if supported [PR 96083]
On 12/16/20 8:00 AM, Jonathan Wakely via Gcc-patches wrote: Clang doesn't support __builtin_sprintf, so use std::sprintf instead. libstdc++-v3/ChangeLog: PR libstdc++/96083 * include/ext/throw_allocator.h: Use __has_builtin to check for __builtin_sprintf support, and use std::sprtinf if necessary. Tested powerpc64le-linux. Committed to trunk. I expected to see the test itself guarded by #ifdef __has_builtin like in . Or is this code only [meant to be] used with compilers that understand __has_builtin? (I'm mostly just curious here about support for other compilers like ICC, not necessarily pointing out a problem with the patch.) Martin
[committed] libstdc++: Only use __builtin_sprintf if supported [PR 96083]
Clang doesn't support __builtin_sprintf, so use std::sprintf instead.
libstdc++-v3/ChangeLog:
PR libstdc++/96083
* include/ext/throw_allocator.h: Use __has_builtin to check for
__builtin_sprintf support, and use std::sprtinf if necessary.
Tested powerpc64le-linux. Committed to trunk.
commit 96d9670e88333d8896a5d2f2bb0403c1e2ad19ab
Author: Jonathan Wakely
Date: Wed Dec 16 13:50:34 2020
libstdc++: Only use __builtin_sprintf if supported [PR 96083]
Clang doesn't support __builtin_sprintf, so use std::sprintf instead.
libstdc++-v3/ChangeLog:
PR libstdc++/96083
* include/ext/throw_allocator.h: Use __has_builtin to check for
__builtin_sprintf support, and use std::sprtinf if necessary.
diff --git a/libstdc++-v3/include/ext/throw_allocator.h
b/libstdc++-v3/include/ext/throw_allocator.h
index 0ab174f19a5..2364827a632 100644
--- a/libstdc++-v3/include/ext/throw_allocator.h
+++ b/libstdc++-v3/include/ext/throw_allocator.h
@@ -64,6 +64,10 @@
#endif
#include
+#if !__has_builtin(__builtin_sprintf)
+# include
+#endif
+
namespace __gnu_cxx _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_VERSION
@@ -310,6 +314,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
static void
log_to_string(std::string& s, const_reference ref)
{
+#if ! __has_builtin(__builtin_sprintf)
+ __typeof__(::sprintf) __builtin_sprintf = ::sprintf;
+#endif
+
char buf[40];
const char tab('\t');
s += "label: ";
@@ -332,6 +340,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
static void
log_to_string(std::string& s, const std::pair& ref)
{
+#if ! __has_builtin(__builtin_sprintf)
+ auto __builtin_sprintf = ::sprintf;
+#endif
+
char buf[40];
const char tab('\t');
s += "label: ";
@@ -566,6 +578,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
static gen_t generator(engine(), distribution);
#endif
+#if ! __has_builtin(__builtin_sprintf)
+ __typeof__(::sprintf) __builtin_sprintf = ::sprintf;
+#endif
+
double random = generator();
if (random < distribution.min() || random > distribution.max())
{
