[PATCH 1/3] C++20 constexpr lib part 1/3

classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/3] C++20 constexpr lib part 1/3

gcc - patches mailing list
Here is the first of three patches for C++20 constexpr library.

 ?????? Implement C++20 p0202 - Add constexpr Modifiers to Functions in
<algorithm> and <utility> Headers.
 ???? ??Implement C++20 p1023 - constexpr comparison operators for std::array.

I believe I have answered peoples concerns with the last patch attempts
[https://gcc.gnu.org/ml/libstdc++/2019-03/msg00132.html].

The patch is large because of test cases but really just boils down to
adding constexpr for c++2a.

The patch passes testing for gnu++2a and pre-gnu++2a on x86_64-linux:

$ make check -k -j4

$ make check RUNTESTFLAGS=--target_board=unix/-std=gnu++2a -k -j4

OK for trunk?

Ed Smith-Rowland



CL_constexpr_lib (9K) Download Attachment
patch_constexpr_lib.bz2 (18K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] C++20 constexpr lib part 1/3

gcc - patches mailing list
On 6/27/19 11:41 AM, Jonathan Wakely wrote:

> On 26/06/19 19:13 -0400, Ed Smith-Rowland via libstdc++ wrote:
>> Here is the first of three patches for C++20 constexpr library.
>>
>> ?????? Implement C++20 p0202 - Add constexpr Modifiers to Functions
>> in <algorithm> and <utility> Headers.
>> ???? ??Implement C++20 p1023 - constexpr comparison operators for
>> std::array.
>>
>> I believe I have answered peoples concerns with the last patch
>> attempts [https://gcc.gnu.org/ml/libstdc++/2019-03/msg00132.html].
>>
>> The patch is large because of test cases but really just boils down
>> to adding constexpr for c++2a.
>
> I still have some concerns about the changes like:
>
> ?????? template<typename _Iterator, typename _Value>
> +?????????? _GLIBCXX20_CONSTEXPR
> ?????????? bool
> -?????????? operator()(_Iterator __it, _Value& __val) const
> +?????????? operator()(_Iterator __it, const _Value& __val) const
> ?????????? { return *__it < __val; }
>
Make a type where operator< changes the rhs.?? I was thinking you'd want
a compare to operate on const things but I guess we have to be ready for
anything.?? I was also thinking < is left associative for a class member
but i guess a class could have a thing that mutates the rhs too.??
Nothing got hit in any of my testing.?? If this is a thing we probably
should make a general test for things like this somewhere.?? Maybe
someone wants to log references or something.

1. I'll experiment to see if I really need these (I *thought* I did, but...)

2. If I still do then I'll make overloads?

>
> I think that might change semantics for some types, and I haven't yet
> figured out if we care about those cases or not. I'll try to come up
> with some examples that change meaning.
>
> This could use _GLIBCXX14_CONSTEXPR:
>
> +?? /**
> +???? * A constexpr wrapper for __builtin_memmove.
> +???? * @param __num The number of elements of type _Tp (not bytes).
>
> I don't think we want a Doxygen comment for this, as it's not part of
> the public API. Just a normal comment is fine.
>
> +???? */
> +?? template<bool _IsMove, typename _Tp>
> +?????? _GLIBCXX20_CONSTEXPR
> +?????? inline void*
> +?????? __memmove(_Tp* __dst, const _Tp* __src, size_t __num)
> +?????? {
> +#if __cplusplus > 201703L \
> +?????? && defined(_GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED)
> +?????????? if (__builtin_is_constant_evaluated())
> +?????? {
> +?????????? for(; __num > 0; --__num)
> +?????????????? {
> +?????????????????? if constexpr (_IsMove)
> +?????????????? *__dst = std::move(*__src);
> +?????????????????? else
> +?????????????? *__dst = *__src;
>
> Do we need this _IsMove condition here? We should only be using this
> function for trivially copyable types, in which case copy vs move
> shouldn't matter, should it?
>
> Oh ... is it because we also end up using this __memmove function for
> non-trivially copyable types, during constant evaluation? Hmm. Then
> it's more than just a wrapper for __builtin_memmove, right? It's
> "either memmove or constexpr range copy", or something.

Yup.?? I'm also bad at naming things.?? Furthermore, I expect the
standards people to want to have our versions of memcpy, memcmp, memmove
that we own and can make constexpr.?? These are such important concepts.

Also, part 2 brings in constexpr swap. Maybe this is the may to
implement an actual memmove?

>
> I don't think this will work in a constant expression:
>
> ?? /// Assign @p __new_val to @p __obj and return its previous value.
> ?? template <typename _Tp, typename _Up = _Tp>
> +?????? _GLIBCXX20_CONSTEXPR
> ?????? inline _Tp
> ?????? exchange(_Tp& __obj, _Up&& __new_val)
> ?????? { return std::__exchange(__obj, std::forward<_Up>(__new_val)); }
>
> Because std::__exchange hasn't been marked constexpr. The test passes
> because it doesn't call it in a context that requires constant
> evaluation:
>
> ??const auto x = std::exchange(e, pi);
Derp.

>
> I see the same problem in other tests too:
>
> +?? constexpr std::array<int, 12> car{{0, 1, 2, 3, 4, 5, 6, 6, 8, 9, 9,
> 11}};
> +
> +?? const auto out0x = std::adjacent_find(car.begin(), car.end());
> +
> +?? const auto out1x = std::adjacent_find(car.begin(), car.end(),
> +?????????????????????????????????????? std::equal_to<int>())

I will go through all the tests and make sure that some nontrivial
calculation is done that contributes to a final bool return.?? And clean
up the mess.?? I did this in chunk 2 but I guess I left a lot of chunk 1.

Come to think of it, we could build *insane* concepts after this.?? For
good or ill.

Ed

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] C++20 constexpr lib part 1/3

gcc - patches mailing list
On 6/27/19 1:06 PM, Ville Voutilainen wrote:

> On Thu, 27 Jun 2019 at 19:55, Ed Smith-Rowland via libstdc++
> <[hidden email]> wrote:
>>> I don't think this will work in a constant expression:
>>>
>>> ?? /// Assign @p __new_val to @p __obj and return its previous value.
>>> ?? template <typename _Tp, typename _Up = _Tp>
>>> +?????? _GLIBCXX20_CONSTEXPR
>>> ?????? inline _Tp
>>> ?????? exchange(_Tp& __obj, _Up&& __new_val)
>>> ?????? { return std::__exchange(__obj, std::forward<_Up>(__new_val)); }
>>>
>>> Because std::__exchange hasn't been marked constexpr. The test passes
>>> because it doesn't call it in a context that requires constant
>>> evaluation:
>>>
>>> ??const auto x = std::exchange(e, pi);
>> Derp.
>>> I see the same problem in other tests too:
>>>
>>> +?? constexpr std::array<int, 12> car{{0, 1, 2, 3, 4, 5, 6, 6, 8, 9, 9,
>>> 11}};
>>> +
>>> +?? const auto out0x = std::adjacent_find(car.begin(), car.end());
>>> +
>>> +?? const auto out1x = std::adjacent_find(car.begin(), car.end(),
>>> +?????????????????????????????????????? std::equal_to<int>())
>> I will go through all the tests and make sure that some nontrivial
>> calculation is done that contributes to a final bool return.?? And clean
>> up the mess.?? I did this in chunk 2 but I guess I left a lot of chunk 1.
> As was the case with the iterator patch, it's not a question of doing
> a nontrivial calculation, but
> a question of initializing a constexpr variable with the result. (Yeah
> sure a non-type template
> argument would also work but eurgh). Then, and only then, have we
> proven that the code
> works in a constexpr context. Initializing a const doesn't do that.
> constinit would, but that's
> C++20.
>
Ok, why isn't

 ?? static_assert(test());

a constexpr context?

The std::exchange test passed originally because, unlike all the other
algo tests I had neglected to call the test function in a constexpr context.

Note that constexpr_iter.c is this:

----

constexpr int
test()
{
 ?? constexpr std::array<int, 3> a1{{1, 2, 3}};
 ?? static_assert(1 == *a1.begin());
 ?? auto n = a1[0] * a1[1]* a1[2];
 ?? static_assert(1 == *a1.cbegin());

 ?? std::array<int, 3> a2{{0, 0, 0}};
 ?? auto a1i = a1.begin();
 ?? auto a1e = a1.end();
 ?? auto a2i = a2.begin();
 ?? while (a1i != a1e)
 ?????? *a2i++ = *a1i++;

 ?? return n;
}

void
run_test()
{
 ?? constexpr int n = test();
}

----

Things inside the function can, and in many cases for this capability
must, be mutable.?? As long as the input and the final output is a
literal we should be good, no?

Also when I assign returned iterators to constexpr I get:

/home/ed/gcc_git/libstdc++-v3/testsuite/25_algorithms/find_if_not/constexpr.cc:36:
error: '(((std::array<int, 12>::const_pointer)(& ca0.std::array<int,
12>::_M_elems)) + 28)' is not a constant expression.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] C++20 constexpr lib part 1/3

Jonathan Wakely-3
On 27/06/19 19:07 -0400, Ed Smith-Rowland wrote:

>On 6/27/19 1:06 PM, Ville Voutilainen wrote:
>>On Thu, 27 Jun 2019 at 19:55, Ed Smith-Rowland via libstdc++
>><[hidden email]> wrote:
>>>>I don't think this will work in a constant expression:
>>>>
>>>>?? /// Assign @p __new_val to @p __obj and return its previous value.
>>>>?? template <typename _Tp, typename _Up = _Tp>
>>>>+?????? _GLIBCXX20_CONSTEXPR
>>>>?????? inline _Tp
>>>>?????? exchange(_Tp& __obj, _Up&& __new_val)
>>>>?????? { return std::__exchange(__obj, std::forward<_Up>(__new_val)); }
>>>>
>>>>Because std::__exchange hasn't been marked constexpr. The test passes
>>>>because it doesn't call it in a context that requires constant
>>>>evaluation:
>>>>
>>>>??const auto x = std::exchange(e, pi);
>>>Derp.
>>>>I see the same problem in other tests too:
>>>>
>>>>+?? constexpr std::array<int, 12> car{{0, 1, 2, 3, 4, 5, 6, 6, 8, 9, 9,
>>>>11}};
>>>>+
>>>>+?? const auto out0x = std::adjacent_find(car.begin(), car.end());
>>>>+
>>>>+?? const auto out1x = std::adjacent_find(car.begin(), car.end(),
>>>>+?????????????????????????????????????? std::equal_to<int>())
>>>I will go through all the tests and make sure that some nontrivial
>>>calculation is done that contributes to a final bool return.?? And clean
>>>up the mess.?? I did this in chunk 2 but I guess I left a lot of chunk 1.
>>As was the case with the iterator patch, it's not a question of doing
>>a nontrivial calculation, but
>>a question of initializing a constexpr variable with the result. (Yeah
>>sure a non-type template
>>argument would also work but eurgh). Then, and only then, have we
>>proven that the code
>>works in a constexpr context. Initializing a const doesn't do that.
>>constinit would, but that's
>>C++20.
>>
>Ok, why isn't
>
>?? static_assert(test());
>
>a constexpr context?

It is, I missed that the array test does that at the bottom of the
file.


>The std::exchange test passed originally because, unlike all the other
>algo tests I had neglected to call the test function in a constexpr
>context.
>
>Note that constexpr_iter.c is this:
>
>----
>
>constexpr int
>test()
>{
>?? constexpr std::array<int, 3> a1{{1, 2, 3}};
>?? static_assert(1 == *a1.begin());
>?? auto n = a1[0] * a1[1]* a1[2];
>?? static_assert(1 == *a1.cbegin());
>
>?? std::array<int, 3> a2{{0, 0, 0}};
>?? auto a1i = a1.begin();
>?? auto a1e = a1.end();
>?? auto a2i = a2.begin();
>?? while (a1i != a1e)
>?????? *a2i++ = *a1i++;
>
>?? return n;
>}
>
>void
>run_test()
>{
>?? constexpr int n = test();
>}
>
>----
>
>Things inside the function can, and in many cases for this capability
>must, be mutable.?? As long as the input and the final output is a
>literal we should be good, no?
>
>Also when I assign returned iterators to constexpr I get:
>
>/home/ed/gcc_git/libstdc++-v3/testsuite/25_algorithms/find_if_not/constexpr.cc:36:
>error: '(((std::array<int, 12>::const_pointer)(& ca0.std::array<int,
>12>::_M_elems)) + 28)' is not a constant expression.

Yes, I tried adding that and got the same error, which is why I
thought the test had the same problem as the std::exchange one.

I'm not sure what causes that error, I'll take a look tomorrow.


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] C++20 constexpr lib part 1/3

Jonathan Wakely-3
In reply to this post by gcc - patches mailing list
On 26/06/19 19:13 -0400, Ed Smith-Rowland via libstdc++ wrote:

>Here is the first of three patches for C++20 constexpr library.
>
>?????? Implement C++20 p0202 - Add constexpr Modifiers to Functions in
><algorithm> and <utility> Headers.
>???? ??Implement C++20 p1023 - constexpr comparison operators for std::array.
>
>I believe I have answered peoples concerns with the last patch
>attempts [https://gcc.gnu.org/ml/libstdc++/2019-03/msg00132.html].
>
>The patch is large because of test cases but really just boils down to
>adding constexpr for c++2a.
>
>The patch passes testing for gnu++2a and pre-gnu++2a on x86_64-linux:
>
>$ make check -k -j4
>
>$ make check RUNTESTFLAGS=--target_board=unix/-std=gnu++2a -k -j4
>
>OK for trunk?

One more comment. In <bits/stl_algobase.h> this:

+#if __cplusplus > 201703L \
+    && defined(_GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED)
+      if (__builtin_is_constant_evaluated())

can be simplified to just:

#ifdef __cpp_lib_is_constant_evaluated
      if (std::is_constant_evaluated())

The feature test macro is exactly the check we want here.


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] C++20 constexpr lib part 1/3

gcc - patches mailing list
On 7/2/19 8:11 AM, Jonathan Wakely wrote:

> One more comment. In <bits/stl_algobase.h> this:
>
> +#if __cplusplus > 201703L \
> +?????? && defined(_GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED)
> +?????????? if (__builtin_is_constant_evaluated())
>
> can be simplified to just:
>
> #ifdef __cpp_lib_is_constant_evaluated
> ???????? if (std::is_constant_evaluated())
>
> The feature test macro is exactly the check we want here
This is done and the test cases for the non-modifying algorithms are done.

I'm was stumped on the modifying algos though.?? Consider std::copy:

-------------------------------------

constexpr bool
test()
{
 ?? constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10,
11}};
 ?? std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};

 ?? constexpr auto out6 = std::copy(ca0.begin(), ca0.begin() + 8,
ma0.begin() + 2);

 ?? return out6 == ma0.begin() + 10;
}

static_assert(test());
-------------------------------------

This is essentially the same as the Boost test case referenced in p0202.

I've also taken the arrays out as globals with the same result either way:

-------------------------------------

ed@bad-horse:~/cxx_constexpr_lib$ ../bin_constexpr_lib/bin/g++
-std=gnu++2a -c testsuite/25_algorithms/copy/constexpr.cc
testsuite/25_algorithms/copy/constexpr.cc: In function ???constexpr bool
test()???:
testsuite/25_algorithms/copy/constexpr.cc:36:34:???? in ???constexpr???
expansion of ???std::copy<const int*, int*>(ca0.std::array<int,
12>::begin(), (ca0.std::array<int, 12>::begin() + 32),
(ma0.std::array<int, 12>::begin() + 8))???
/home/ed/bin_constexpr_lib/include/c++/10.0.0/bits/stl_algobase.h:535:7:
in ???constexpr??? expansion of ???std::__copy_move_a2<false, const int*,
int*>(std::__miter_base<const int*>(__first), std::__miter_base<const
int*>(__last), __result)???
/home/ed/bin_constexpr_lib/include/c++/10.0.0/bits/stl_algobase.h:501:30:
in ???constexpr??? expansion of ???std::__copy_move_a<false, const int*,
int*>(std::__niter_base<const int*>(__first), std::__niter_base<const
int*>(__last), std::__niter_base<int*>(__result))???
/home/ed/bin_constexpr_lib/include/c++/10.0.0/bits/stl_algobase.h:463:30:
in ???constexpr??? expansion of ???std::__copy_move<false, true,
std::random_access_iterator_tag>::__copy_m<int>(__first, __last, __result)???
/home/ed/bin_constexpr_lib/include/c++/10.0.0/bits/stl_algobase.h:445:24:
in ???constexpr??? expansion of ???std::__memmove<false, int>(__result,
__first, ((std::size_t)((std::ptrdiff_t)_Num)))???
testsuite/25_algorithms/copy/constexpr.cc:36:80: error: modification of
???ma0??? is not a constant expression
 ???? 36 |???? constexpr auto out6 = std::copy(ca0.begin(), ca0.begin() + 8,
ma0.begin() + 2);
| ^
testsuite/25_algorithms/copy/constexpr.cc: At global scope:
testsuite/25_algorithms/copy/constexpr.cc:41:19: error: non-constant
condition for static assertion
 ???? 41 | static_assert(test());
 ?????????? |???????????????????????????? ~~~~^~

-------------------------------------

By my reckoning, you have a constexpr source array, an output array that
is initialized as it must be for constexpr.?? You have to have a
deterministic result after the copy.?? In the local array version the
actual iterator is not leaking out - just the results of a calculation
that must return one result.

The only thing that 'helps' is removing the constexpr from the iterator
return and of course that's the whole point of the thing.

Blargh!

So, you can't modify a constexpr sequence. No actual surprise.?? The
returned iterators into non-constexpr sequences can never be literals.??
What you *can* do is make the modifying algorithms so you can make pure
functions with them.?? In this connection my previous testcases for
non-modifying?? were correct, just not complete because there the
returned iterators into constexpr sequences can be (must be) literals.

So here is a new patch for chunk 1 of constexpr library.

Tested with default settings and with -std=gnu++2a on x86_64-linux.

OK?

Ed



CL_constexpr_lib (9K) Download Attachment
patch_constexpr_lib_2.bz2 (20K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] C++20 constexpr lib part 1/3

Ville Voutilainen
On Sat, 6 Jul 2019 at 06:12, Ed Smith-Rowland via libstdc++
<[hidden email]> wrote:

> By my reckoning, you have a constexpr source array, an output array that
> is initialized as it must be for constexpr.?? You have to have a
> deterministic result after the copy.?? In the local array version the
> actual iterator is not leaking out - just the results of a calculation
> that must return one result.
>
> The only thing that 'helps' is removing the constexpr from the iterator
> return and of course that's the whole point of the thing.
>
> Blargh!

But that's fine; the result of copy is not stored in a constexpr
variable, but the function return
is static_asserted so we have sufficiently tested that std::copy is
indeed constexpr-compatible
since it appears in a function that is evaluated at compile-time.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] C++20 constexpr lib part 1/3

gcc - patches mailing list
Here is the patch for

* Implement C++20 p0202 - Add constexpr Modifiers to Functions in
<algorithm> and <utility> Headers.

* Implement C++20 p1023 - constexpr comparison operators for std::array.

Relative to the last effort it is rebased on more recent trunk and I
added to <version>.

There's some chance that I'll have to tweak the macros after the draft
comes in but I'd like to get this moving.?? I've got other chunks of
constexpr lib coming.?? This passes C++20 testing onx86_64-linux.

Ok?



CL_constexpr_lib (9K) Download Attachment
patch_constexpr_lib_3.bz2 (20K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] C++20 constexpr lib part 1/3

Jonathan Wakely-3
On 31/07/19 10:50 -0400, Ed Smith-Rowland via libstdc++ wrote:

>Here is the patch for
>
>* Implement C++20 p0202 - Add constexpr Modifiers to Functions in
><algorithm> and <utility> Headers.
>
>* Implement C++20 p1023 - constexpr comparison operators for std::array.
>
>Relative to the last effort it is rebased on more recent trunk and I
>added to <version>.
>
>There's some chance that I'll have to tweak the macros after the draft
>comes in but I'd like to get this moving.?? I've got other chunks of
>constexpr lib coming.?? This passes C++20 testing onx86_64-linux.
>
>Ok?

Calls to the new __memmove and __memcmp functions need to be qualified
with std:: to prevent ADL. I think we should rename those functions,
but that can happen later.

The new 23_containers/array/comparison_operators/constexpr.cc test
will be UNSUPPORTED by default because it doesn't have the directive
{ dg-options "-std=gnu++2a" }.

The change to the <version> header defines __cpp_lib_constexpr instead
of __cpp_lib_constexpr_algorithms as it should be. For some recent
changes I've added a testcase that does nothing but include <version>
and check the feature test macro, which ensures that it's set
correctly by <version> not just by the other header(s) defining it.
For example, see testsuite/26_numerics/numbers/2.cc added yesterday.

I wonder if the feature test macro should only be defined when
__cpp_lib_is_constant_evaluated is defined, because otherwise some
algos will not be usable constant expressions (e.g. when compiled with
Clang 7.0).

OK for trunk with:

- std:: qualification on the new __mem* functions;
- the dg-options added to the testcase;
- fix the macro in <version> (and ideally add a test for it).

Thanks.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] C++20 constexpr lib part 1/3

gcc - patches mailing list
On 8/1/19 6:56 AM, Jonathan Wakely wrote:

> On 31/07/19 10:50 -0400, Ed Smith-Rowland via libstdc++ wrote:
>> Here is the patch for
>>
>> * Implement C++20 p0202 - Add constexpr Modifiers to Functions in
>> <algorithm> and <utility> Headers.
>>
>> * Implement C++20 p1023 - constexpr comparison operators for std::array.
>>
>> Relative to the last effort it is rebased on more recent trunk and I
>> added to <version>.
>>
>> There's some chance that I'll have to tweak the macros after the
>> draft comes in but I'd like to get this moving.?? I've got other
>> chunks of constexpr lib coming.?? This passes C++20 testing
>> onx86_64-linux.
>>
>> Ok?
>
> Calls to the new __memmove and __memcmp functions need to be qualified
> with std:: to prevent ADL. I think we should rename those functions,
> but that can happen later.
IMHO, these concepts are too important to leave as an implementation detail.

I suspect the committee will come crawling back to specify these with
real names.

memory_copy, memory_compare, memory_move for C++23 anyone?

> The new 23_containers/array/comparison_operators/constexpr.cc test
> will be UNSUPPORTED by default because it doesn't have the directive
> { dg-options "-std=gnu++2a" }.
>
> The change to the <version> header defines __cpp_lib_constexpr instead
> of __cpp_lib_constexpr_algorithms as it should be. For some recent
> changes I've added a testcase that does nothing but include <version>
> and check the feature test macro, which ensures that it's set
> correctly by <version> not just by the other header(s) defining it.
> For example, see testsuite/26_numerics/numbers/2.cc added yesterday.
I'll add a testcase.
> I wonder if the feature test macro should only be defined when
> __cpp_lib_is_constant_evaluated is defined, because otherwise some
> algos will not be usable constant expressions (e.g. when compiled with
> Clang 7.0).
We can do this later if we need to.
>
> OK for trunk with:
>
> - std:: qualification on the new __mem* functions;
Done.
> - the dg-options added to the testcase;
Done.
> - fix the macro in <version> (and ideally add a test for it).
Done.
> Thanks.

Committed with 273975.?? Final patch and CL attached.

Regards,

Ed



CL_constexpr_lib (9K) Download Attachment
patch_constexpr_lib_4.bz2 (20K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/3] C++20 constexpr lib part 2/3 - swappish functions.

gcc - patches mailing list
Greetings,

Here is a patch for C++20 p0879 - Constexpr for swap and swap related
functions.

This essentially constexprifies the rest of <algorithm>.

Built and tested with C++20 (and pre-c++20) on x86_64-linux.

Ok?

Regards,

Ed Smith-Rowland



CL_constexpr_swap (3K) Download Attachment
patch_constexpr_swap (73K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] C++20 constexpr lib part 1/3

gcc - patches mailing list
In reply to this post by gcc - patches mailing list
On 8/1/19 3:45 PM, Jonathan Wakely wrote:

> On 01/08/19 11:47 -0400, Ed Smith-Rowland via libstdc++ wrote:
>> On 8/1/19 6:56 AM, Jonathan Wakely wrote:
>>> On 31/07/19 10:50 -0400, Ed Smith-Rowland via libstdc++ wrote:
>>>> Here is the patch for
>>>>
>>>> * Implement C++20 p0202 - Add constexpr Modifiers to Functions in
>>>> <algorithm> and <utility> Headers.
>>>>
>>>> * Implement C++20 p1023 - constexpr comparison operators for
>>>> std::array.
>>>>
>>>> Relative to the last effort it is rebased on more recent trunk and
>>>> I added to <version>.
>>>>
>>>> There's some chance that I'll have to tweak the macros after the
>>>> draft comes in but I'd like to get this moving.?? I've got other
>>>> chunks of constexpr lib coming.?? This passes C++20 testing
>>>> onx86_64-linux.
>>>>
>>>> Ok?
>>>
>>> Calls to the new __memmove and __memcmp functions need to be qualified
>>> with std:: to prevent ADL. I think we should rename those functions,
>>> but that can happen later.
>>
>> IMHO, these concepts are too important to leave as an implementation
>> detail.
>>
>> I suspect the committee will come crawling back to specify these with
>> real names.
>>
>> memory_copy, memory_compare, memory_move for C++23 anyone?
>
> trivial_copy, trivial_compare, trivial_move

Works for me. I'm sure there will be great bikeshed battles!

Ed


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] C++20 constexpr lib part 1/3

Steve Ellcey-10
In reply to this post by gcc - patches mailing list
Ed,

I have run into an ICE that I tracked down to this patch:

commit 02fefffe6b78c4c39169206aa40fb53f367ecba8
Author: emsr <emsr@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Thu Aug 1 15:25:42 2019 +0000

    2019-08-01  Edward Smith-Rowland  <[hidden email]>

            Implement C++20 p0202 - Add Constexpr Modifiers to Functions
            in <algorithm> and <utility> Headers.
            Implement C++20 p1023 - constexpr comparison operators for std::array.


Before I try to create a smaller test example (the current failure happens
when I build https://github.com/llnl/RAJAPerf.git) I was wondering if you
have already seen this ICE:

/extra/sellcey/raja-build-error/RAJAPerf/src/apps/WIP-COUPLE.cpp: In member function 'virtual void rajaperf::apps::COUPLE::runKernel(rajaperf::VariantID)':
/extra/sellcey/raja-build-error/RAJAPerf/src/apps/WIP-COUPLE.cpp:217:1: internal compiler error: Segmentation fault
  217 | } // end namespace rajaperf
      | ^
0xe81ddf crash_signal
        ../../gcc/gcc/toplev.c:326
0x968d14 lookup_page_table_entry
        ../../gcc/gcc/ggc-page.c:632
0x968d14 ggc_set_mark(void const*)
        ../../gcc/gcc/ggc-page.c:1531
0xbfeadf gt_ggc_mx_symtab_node(void*)
        /extra/sellcey/gcc-tot/obj-gcc/gcc/gtype-desc.c:1302
0xd9d2a7 gt_ggc_ma_order
        ./gt-passes.h:31
0xd9d2a7 gt_ggc_ma_order
        ./gt-passes.h:26
0xb6f49f ggc_mark_root_tab
        ../../gcc/gcc/ggc-common.c:77
0xb6f6c3 ggc_mark_roots()
        ../../gcc/gcc/ggc-common.c:94
0x9696af ggc_collect()
        ../../gcc/gcc/ggc-page.c:2201
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] C++20 constexpr lib part 1/3

Jonathan Wakely-3
On 06/08/19 15:30 +0000, Steve Ellcey wrote:

>Ed,
>
>I have run into an ICE that I tracked down to this patch:
>
>commit 02fefffe6b78c4c39169206aa40fb53f367ecba8
>Author: emsr <emsr@138bc75d-0d04-0410-961f-82ee72b054a4>
>Date:   Thu Aug 1 15:25:42 2019 +0000
>
>    2019-08-01  Edward Smith-Rowland  <[hidden email]>
>
>            Implement C++20 p0202 - Add Constexpr Modifiers to Functions
>            in <algorithm> and <utility> Headers.
>            Implement C++20 p1023 - constexpr comparison operators for std::array.
>
>
>Before I try to create a smaller test example (the current failure happens
>when I build https://github.com/llnl/RAJAPerf.git) I was wondering if you
>have already seen this ICE:
>
>/extra/sellcey/raja-build-error/RAJAPerf/src/apps/WIP-COUPLE.cpp: In member function 'virtual void rajaperf::apps::COUPLE::runKernel(rajaperf::VariantID)':
>/extra/sellcey/raja-build-error/RAJAPerf/src/apps/WIP-COUPLE.cpp:217:1: internal compiler error: Segmentation fault

The RAJAPerf code appears to be built with -std=gnu++11 which means
Ed's patch should make almost no difference at all. 99% of the patch
has no effect unless compiling with -std=gnu++2a.

I don't see any ICE running the libstdc++ testsuite with -std=gnu++11,
so I have no better suggestion than creating a bugzilla report for the
C++ front-end, with the preprocessed source of WIP-COUPLE.cpp

>  217 | } // end namespace rajaperf
>      | ^
>0xe81ddf crash_signal
> ../../gcc/gcc/toplev.c:326
>0x968d14 lookup_page_table_entry
> ../../gcc/gcc/ggc-page.c:632
>0x968d14 ggc_set_mark(void const*)
> ../../gcc/gcc/ggc-page.c:1531
>0xbfeadf gt_ggc_mx_symtab_node(void*)
> /extra/sellcey/gcc-tot/obj-gcc/gcc/gtype-desc.c:1302
>0xd9d2a7 gt_ggc_ma_order
> ./gt-passes.h:31
>0xd9d2a7 gt_ggc_ma_order
> ./gt-passes.h:26
>0xb6f49f ggc_mark_root_tab
> ../../gcc/gcc/ggc-common.c:77
>0xb6f6c3 ggc_mark_roots()
> ../../gcc/gcc/ggc-common.c:94
>0x9696af ggc_collect()
> ../../gcc/gcc/ggc-page.c:2201
>Please submit a full bug report,
>with preprocessed source if appropriate.
>Please include the complete backtrace with any bug report.
>See <https://gcc.gnu.org/bugs/> for instructions.
Reply | Threaded
Open this post in threaded view
|

Re: [EXT] Re: [PATCH 1/3] C++20 constexpr lib part 1/3

Steve Ellcey-10
On Tue, 2019-08-06 at 21:04 +0100, Jonathan Wakely wrote:
>
> The RAJAPerf code appears to be built with -std=gnu++11 which means
> Ed's patch should make almost no difference at all. 99% of the patch
> has no effect unless compiling with -std=gnu++2a.
>
> I don't see any ICE running the libstdc++ testsuite with -std=gnu++11,
> so I have no better suggestion than creating a bugzilla report for the
> C++ front-end, with the preprocessed source of WIP-COUPLE.cpp

I created a preprocessed file.  Unfortunately when I compile that
preprocessed file with the same options as the unpreprocessed file,
it does not ICE.  I compiled the original file with -save-temps and
that compile no longer gives an ICE.  I hate bugs like this.

Steve Ellcey
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [EXT] Re: [PATCH 1/3] C++20 constexpr lib part 1/3

Steve Ellcey-10
On Tue, 2019-08-06 at 16:47 -0400, Marek Polacek wrote:

> On Tue, Aug 06, 2019 at 08:30:14PM +0000, Steve Ellcey wrote:
> > On Tue, 2019-08-06 at 21:04 +0100, Jonathan Wakely wrote:
> > >
> > > The RAJAPerf code appears to be built with -std=gnu++11 which
> > > means
> > > Ed's patch should make almost no difference at all. 99% of the
> > > patch
> > > has no effect unless compiling with -std=gnu++2a.
> > >
> > > I don't see any ICE running the libstdc++ testsuite with
> > > -std=gnu++11,
> > > so I have no better suggestion than creating a bugzilla report
> > > for the
> > > C++ front-end, with the preprocessed source of WIP-COUPLE.cpp
> >
> > I created a preprocessed file.  Unfortunately when I compile that
> > preprocessed file with the same options as the unpreprocessed file,
> > it does not ICE.  I compiled the original file with -save-temps and
> > that compile no longer gives an ICE.  I hate bugs like this.
>
> Does -fdirectives-only make any difference?
>
> Marek

Nope.  I also looked at the preprocessed file compiled with and without
this patch and the two preprocessed files are identical.  I am thinking
that this patch is triggering some unrelated latent bug.

Steve Ellcey
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] C++20 constexpr lib part 1/3

gcc - patches mailing list
In reply to this post by Steve Ellcey-10
On 8/6/19 11:30 AM, Steve Ellcey wrote:

> Ed,
>
> I have run into an ICE that I tracked down to this patch:
>
> commit 02fefffe6b78c4c39169206aa40fb53f367ecba8
> Author: emsr <emsr@138bc75d-0d04-0410-961f-82ee72b054a4>
> Date:   Thu Aug 1 15:25:42 2019 +0000
>
>      2019-08-01  Edward Smith-Rowland  <[hidden email]>
>
>              Implement C++20 p0202 - Add Constexpr Modifiers to Functions
>              in <algorithm> and <utility> Headers.
>              Implement C++20 p1023 - constexpr comparison operators for std::array.
>
>
> Before I try to create a smaller test example (the current failure happens
> when I build https://github.com/llnl/RAJAPerf.git) I was wondering if you
> have already seen this ICE:
>
> /extra/sellcey/raja-build-error/RAJAPerf/src/apps/WIP-COUPLE.cpp: In member function 'virtual void rajaperf::apps::COUPLE::runKernel(rajaperf::VariantID)':
> /extra/sellcey/raja-build-error/RAJAPerf/src/apps/WIP-COUPLE.cpp:217:1: internal compiler error: Segmentation fault
>    217 | } // end namespace rajaperf
>        | ^
> 0xe81ddf crash_signal
> ../../gcc/gcc/toplev.c:326
> 0x968d14 lookup_page_table_entry
> ../../gcc/gcc/ggc-page.c:632
> 0x968d14 ggc_set_mark(void const*)
> ../../gcc/gcc/ggc-page.c:1531
> 0xbfeadf gt_ggc_mx_symtab_node(void*)
> /extra/sellcey/gcc-tot/obj-gcc/gcc/gtype-desc.c:1302
> 0xd9d2a7 gt_ggc_ma_order
> ./gt-passes.h:31
> 0xd9d2a7 gt_ggc_ma_order
> ./gt-passes.h:26
> 0xb6f49f ggc_mark_root_tab
> ../../gcc/gcc/ggc-common.c:77
> 0xb6f6c3 ggc_mark_roots()
> ../../gcc/gcc/ggc-common.c:94
> 0x9696af ggc_collect()
> ../../gcc/gcc/ggc-page.c:2201
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See <https://gcc.gnu.org/bugs/> for instructions.

I've been building C++14 code (and C++17 and C++20 code) with this patch
for half a year and no ICE.

I don't see how the patch could impact pre C++20 code.

Ed


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/3] C++20 constexpr lib part 2/3 - swappish functions.

gcc - patches mailing list
In reply to this post by gcc - patches mailing list
On 8/13/19 7:14 AM, Jonathan Wakely wrote:

> On 01/08/19 13:16 -0400, Ed Smith-Rowland via libstdc++ wrote:
>> Greetings,
>>
>> Here is a patch for C++20 p0879 - Constexpr for swap and swap related
>> functions.
>>
>> This essentially constexprifies the rest of <algorithm>.
>>
>> Built and tested with C++20 (and pre-c++20) on x86_64-linux.
>>
>> Ok?
>>
>> Regards,
>>
>> Ed Smith-Rowland
>>
>>
>
>> 2019-08-01?? Edward Smith-Rowland <[hidden email]>
>>
>> ????????Implement C++20 p0879 - Constexpr for swap and swap related
>> functions.
>> ????????* include/bits/algorithmfwd.h (__cpp_lib_constexpr_swap_algorithms):
>> ????????New macro. (iter_swap, make_heap, next_permutation,
>> partial_sort_copy,
>
> There should be a newline after "New macro." and before the next
> parenthesized list of identifiers.
>
> The parenthesized lists should not span multiple lines, so close and
> reopen the parens, i.e.
>
> ???????????? Implement C++20 p0879 - Constexpr for swap and swap related
> functions.
> ???????????? * include/bits/algorithmfwd.h
> (__cpp_lib_constexpr_swap_algorithms):
> ???????????? New macro.
> ???????????? (iter_swap, make_heap, next_permutation, partial_sort_copy,
> pop_heap)
> ???????????? (prev_permutation, push_heap, reverse, rotate, sort_heap, swap)
> ???????????? (swap_ranges, nth_element, partial_sort, sort): Add constexpr.
>
>> @@ -193,6 +193,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>
>> #if __cplusplus > 201703L
>> #?? define __cpp_lib_constexpr_algorithms 201711L
>> +#?? define __cpp_lib_constexpr_swap_algorithms 201712L
>
> Should this value be 201806L?
Indeed.
>
> The new macro also needs to be added to <version>.
>
Done.

I this OK after it passes testing?

Ed




CL_constexpr_swap (3K) Download Attachment
patch_constexpr_swap (70K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/3] C++20 constexpr lib part 2/3 - swappish functions.

Jonathan Wakely-3
On 14/08/19 11:06 -0400, Ed Smith-Rowland wrote:
>I this OK after it passes testing?
>
>Ed
>
>
>

>2019-08-14  Edward Smith-Rowland  <[hidden email]>
>
> Implement C++20 p0879 - Constexpr for swap and swap related functions.
> * include/bits/algorithmfwd.h (__cpp_lib_constexpr_swap_algorithms):
> New macro.
> * include/std/version: Ditto.

It looks like this line was inserted in the wrong place, as the lines
that follow it are not part of <version>. The entry for
include/std/version should be after include/std/type_traits.


> (iter_swap, make_heap, next_permutation, partial_sort_copy, pop_heap)
> (prev_permutation, push_heap, reverse, rotate, sort_heap, swap)
> (swap_ranges, nth_element, partial_sort, sort): Add constexpr.
> * include/bits/move.h (swap): Add constexpr.
> * include/bits/stl_algo.h (__move_median_to_first, __reverse, reverse)
> (__gcd, __rotate, rotate, __partition, __heap_select)
> (__partial_sort_copy, partial_sort_copy, __unguarded_partition)
> (__unguarded_partition_pivot, __partial_sort, __introsort_loop, __sort)
> (__introselect, __chunk_insertion_sort, next_permutation)
> (prev_permutation, partition, partial_sort, nth_element, sort)
> (__iter_swap::iter_swap, iter_swap, swap_ranges): Add constexpr.
> * include/bits/stl_algobase.h (__iter_swap::iter_swap, iter_swap)
> (swap_ranges): Add constexpr.
> * include/bits/stl_heap.h (__push_heap, push_heap, __adjust_heap,
> __pop_heap, pop_heap, __make_heap, make_heap, __sort_heap, sort_heap):
> Add constexpr.
> * include/std/type_traits (swap): Add constexpr.

i.e. here.

OK for trunk with that change if testing passes. Thanks!