* [committed] libstdc++: Only use __builtin_sprintf if supported [PR 96083]
@ 2020-12-16 15:00 Jonathan Wakely
2020-12-16 16:23 ` Martin Sebor
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Wakely @ 2020-12-16 15:00 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 292 bytes --]
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.
[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 1944 bytes --]
commit 96d9670e88333d8896a5d2f2bb0403c1e2ad19ab
Author: Jonathan Wakely <jwakely@redhat.com>
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 <ext/alloc_traits.h>
+#if !__has_builtin(__builtin_sprintf)
+# include <cstdio>
+#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__(&std::sprintf) __builtin_sprintf = &std::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<const void*, size_t>& ref)
{
+#if ! __has_builtin(__builtin_sprintf)
+ auto __builtin_sprintf = &std::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__(&std::sprintf) __builtin_sprintf = &std::sprintf;
+#endif
+
double random = generator();
if (random < distribution.min() || random > distribution.max())
{
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [committed] libstdc++: Only use __builtin_sprintf if supported [PR 96083]
2020-12-16 15:00 [committed] libstdc++: Only use __builtin_sprintf if supported [PR 96083] Jonathan Wakely
@ 2020-12-16 16:23 ` Martin Sebor
2020-12-16 17:02 ` Jonathan Wakely
0 siblings, 1 reply; 6+ messages in thread
From: Martin Sebor @ 2020-12-16 16:23 UTC (permalink / raw)
To: Jonathan Wakely, libstdc++, gcc-patches
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 <utility>. 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [committed] libstdc++: Only use __builtin_sprintf if supported [PR 96083]
2020-12-16 16:23 ` Martin Sebor
@ 2020-12-16 17:02 ` Jonathan Wakely
2020-12-16 17:04 ` Jonathan Wakely
2020-12-16 19:28 ` Jonathan Wakely
0 siblings, 2 replies; 6+ messages in thread
From: Jonathan Wakely @ 2020-12-16 17:02 UTC (permalink / raw)
To: Martin Sebor; +Cc: libstdc++, gcc-patches
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 <utility>. 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 <utility> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [committed] libstdc++: Only use __builtin_sprintf if supported [PR 96083]
2020-12-16 17:02 ` Jonathan Wakely
@ 2020-12-16 17:04 ` Jonathan Wakely
2020-12-16 18:18 ` Martin Sebor
2020-12-16 19:28 ` Jonathan Wakely
1 sibling, 1 reply; 6+ messages in thread
From: Jonathan Wakely @ 2020-12-16 17:04 UTC (permalink / raw)
To: Martin Sebor; +Cc: libstdc++, gcc-patches
On 16/12/20 17:02 +0000, 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 <utility>. 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [committed] libstdc++: Only use __builtin_sprintf if supported [PR 96083]
2020-12-16 17:04 ` Jonathan Wakely
@ 2020-12-16 18:18 ` Martin Sebor
0 siblings, 0 replies; 6+ messages in thread
From: Martin Sebor @ 2020-12-16 18:18 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches
On 12/16/20 10:04 AM, Jonathan Wakely wrote:
> On 16/12/20 17:02 +0000, 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 <utility>. 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [committed] libstdc++: Only use __builtin_sprintf if supported [PR 96083]
2020-12-16 17:02 ` Jonathan Wakely
2020-12-16 17:04 ` Jonathan Wakely
@ 2020-12-16 19:28 ` Jonathan Wakely
1 sibling, 0 replies; 6+ messages in thread
From: Jonathan Wakely @ 2020-12-16 19:28 UTC (permalink / raw)
To: Martin Sebor; +Cc: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1286 bytes --]
On 16/12/20 17:02 +0000, 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 <utility>. 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 <utility> 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.
[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 2143 bytes --]
commit 4d4f82959aa0802611f1183389c4c74d22431e49
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Wed Dec 16 17:18:10 2020
libstdc++: Simplify built-in detection in <utility>
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<size_t... _Indexes> 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<size_t _Num>
struct _Build_index_tuple
{
-#if _GLIBCXX_USE_MAKE_INTEGER_SEQ
+#if __has_builtin(__make_integer_seq)
template<typename, size_t... _Indices>
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<typename _Tp, _Tp _Num>
using make_integer_sequence
-#if _GLIBCXX_USE_MAKE_INTEGER_SEQ
+#if __has_builtin(__make_integer_seq)
= __make_integer_seq<integer_sequence, _Tp, _Num>;
#else
= integer_sequence<_Tp, __integer_pack(_Num)...>;
#endif
-#undef _GLIBCXX_USE_MAKE_INTEGER_SEQ
-
/// Alias template index_sequence
template<size_t... _Idx>
using index_sequence = integer_sequence<size_t, _Idx...>;
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-12-16 19:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-16 15:00 [committed] libstdc++: Only use __builtin_sprintf if supported [PR 96083] Jonathan Wakely
2020-12-16 16:23 ` Martin Sebor
2020-12-16 17:02 ` Jonathan Wakely
2020-12-16 17:04 ` Jonathan Wakely
2020-12-16 18:18 ` Martin Sebor
2020-12-16 19:28 ` Jonathan Wakely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).