| Message ID | bf02c66f-a911-8ce0-9249-45bef80b418c@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
François Dumont via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > Following confirmation of the fix by TC here is the patch where I'm > simply adding a 'constexpr' on _M_next(). > > Please let me know this ChangeLog entry is correct. I would prefer > this patch to be assigned to 'TC' with me as co-author but I don't > know how to do such a thing. Unless I need to change my user git > identity to do so ? git commit --author="TC <email.here>" --amend > > libstdc++: Add constexpr qualification to _Hash_node::_M_next() > > https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=1b6f0476837205932613ddb2b3429a55c26c409d > changed _Hash_node_value_base to no longer derive from > _Hash_node_base, which means > that its member functions expect _M_storage to be at a different > offset. So explosions > result if an out-of-line definition is emitted for any of the > member functions (say, > in a non-optimized build) and the resulting object file is then > linked with code built > using older version of GCC/libstdc++. > > libstdc++-v3/ChangeLog: > > * include/bits/hashtable_policy.h > (_Hash_node_value_base<>::_M_valptr(), > _Hash_node_value_base<>::_M_v()) > Add [[__gnu__::__always_inline__]]. > (_Hash_node<>::_M_next()): Add constexpr. > > Co-authored-by: TC <rs2740@gmail.com> > > Ok to commit and backport to GCC 11, 12, 13 branches ? > > François > > [2. text/x-patch; pr111050.patch]...
On Sun, 10 Sept 2023 at 14:57, François Dumont via Libstdc++ <libstdc++@gcc.gnu.org> wrote: > > Following confirmation of the fix by TC here is the patch where I'm > simply adding a 'constexpr' on _M_next(). > > Please let me know this ChangeLog entry is correct. I would prefer this > patch to be assigned to 'TC' with me as co-author but I don't know how > to do such a thing. Unless I need to change my user git identity to do so ? Sam already explained that, but please check with Tim how he wants to be credited, if at all. He doesn't have a copyright assignment, and hasn't added a DCO sign-off to the patch, but it's small enough to not need it as this is the first contribution credited to him. > > libstdc++: Add constexpr qualification to _Hash_node::_M_next() What has this constexpr addition got to do with the ABI change and the always_inline attributes? It certainly doesn't seem like it should be the summary line of the git commit message. > > https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=1b6f0476837205932613ddb2b3429a55c26c409d > changed _Hash_node_value_base to no longer derive from > _Hash_node_base, which means > that its member functions expect _M_storage to be at a different > offset. So explosions > result if an out-of-line definition is emitted for any of the > member functions (say, > in a non-optimized build) and the resulting object file is then > linked with code built > using older version of GCC/libstdc++. > > libstdc++-v3/ChangeLog: > > * include/bits/hashtable_policy.h > (_Hash_node_value_base<>::_M_valptr(), > _Hash_node_value_base<>::_M_v()) > Add [[__gnu__::__always_inline__]]. > (_Hash_node<>::_M_next()): Add constexpr. > > Co-authored-by: TC <rs2740@gmail.com> > > Ok to commit and backport to GCC 11, 12, 13 branches ? > > François >
On 11/09/2023 13:51, Jonathan Wakely wrote: > On Sun, 10 Sept 2023 at 14:57, François Dumont via Libstdc++ > <libstdc++@gcc.gnu.org> wrote: >> Following confirmation of the fix by TC here is the patch where I'm >> simply adding a 'constexpr' on _M_next(). >> >> Please let me know this ChangeLog entry is correct. I would prefer this >> patch to be assigned to 'TC' with me as co-author but I don't know how >> to do such a thing. Unless I need to change my user git identity to do so ? > Sam already explained that, but please check with Tim how he wants to > be credited, if at all. He doesn't have a copyright assignment, and > hasn't added a DCO sign-off to the patch, but it's small enough to not > need it as this is the first contribution credited to him. > > >> libstdc++: Add constexpr qualification to _Hash_node::_M_next() > What has this constexpr addition got to do with the ABI change and the > always_inline attributes? > > It certainly doesn't seem like it should be the summary line of the > git commit message. Oops, sorry, that's what I had started to do before Tim submitted anything. Here is latest version: Author: TC <rs2740@gmail.com> Date: Wed Sep 6 19:31:55 2023 +0200 libstdc++: Force inline on _Hash_node_value_base methods to fix abi (PR111050) https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=1b6f0476837205932613ddb2b3429a55c26c409d changed _Hash_node_value_base to no longer derive from _Hash_node_base, which means that its member functions expect _M_storage to be at a different offset. So explosions result if an out-of-line definition is emitted for any of the member functions (say, in a non-optimized build) and the resulting object file is then linked with code built using older version of GCC/libstdc++. libstdc++-v3/ChangeLog: PR libstdc++/111050 * include/bits/hashtable_policy.h (_Hash_node_value_base<>::_M_valptr(), _Hash_node_value_base<>::_M_v()) Add [[__gnu__::__always_inline__]]. (_Hash_node<>::_M_next()): Add constexpr. Co-authored-by: François Dumont <fdumont@gcc.gnu.org> Ok for you TC (Tim ?) ?
On Mon, 11 Sept 2023 at 18:19, François Dumont <frs.dumont@gmail.com> wrote: > > > On 11/09/2023 13:51, Jonathan Wakely wrote: > > On Sun, 10 Sept 2023 at 14:57, François Dumont via Libstdc++ > > <libstdc++@gcc.gnu.org> wrote: > >> Following confirmation of the fix by TC here is the patch where I'm > >> simply adding a 'constexpr' on _M_next(). > >> > >> Please let me know this ChangeLog entry is correct. I would prefer this > >> patch to be assigned to 'TC' with me as co-author but I don't know how > >> to do such a thing. Unless I need to change my user git identity to do so ? > > Sam already explained that, but please check with Tim how he wants to > > be credited, if at all. He doesn't have a copyright assignment, and > > hasn't added a DCO sign-off to the patch, but it's small enough to not > > need it as this is the first contribution credited to him. > > > > > >> libstdc++: Add constexpr qualification to _Hash_node::_M_next() > > What has this constexpr addition got to do with the ABI change and the > > always_inline attributes? > > > > It certainly doesn't seem like it should be the summary line of the > > git commit message. > > Oops, sorry, that's what I had started to do before Tim submitted anything. > > Here is latest version: No patch attached, and the ChangeLog below still mentions the constexpr. I've pinged Tim via another channel to ask him about the author attribution. > > Author: TC <rs2740@gmail.com> > Date: Wed Sep 6 19:31:55 2023 +0200 > > libstdc++: Force inline on _Hash_node_value_base methods to fix abi > (PR111050) > > https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=1b6f0476837205932613ddb2b3429a55c26c409d > changed _Hash_node_value_base to no longer derive from > _Hash_node_base, which means > that its member functions expect _M_storage to be at a different > offset. So explosions > result if an out-of-line definition is emitted for any of the > member functions (say, > in a non-optimized build) and the resulting object file is then > linked with code built > using older version of GCC/libstdc++. > > libstdc++-v3/ChangeLog: > > PR libstdc++/111050 > * include/bits/hashtable_policy.h > (_Hash_node_value_base<>::_M_valptr(), > _Hash_node_value_base<>::_M_v()) > Add [[__gnu__::__always_inline__]]. > (_Hash_node<>::_M_next()): Add constexpr. > > Co-authored-by: François Dumont <fdumont@gcc.gnu.org> > > Ok for you TC (Tim ?) ? > >
Author: TC <rs2740@gmail.com> Date: Wed Sep 6 19:31:55 2023 +0200 libstdc++: Force _Hash_node_value_base methods inline to fix abi (PR111050) https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=1b6f0476837205932613ddb2b3429a55c26c409d changed _Hash_node_value_base to no longer derive from _Hash_node_base, which means that its member functions expect _M_storage to be at a different offset. So explosions result if an out-of-line definition is emitted for any of the member functions (say, in a non-optimized build) and the resulting object file is then linked with code built using older version of GCC/libstdc++. libstdc++-v3/ChangeLog: PR libstdc++/111050 * include/bits/hashtable_policy.h (_Hash_node_value_base<>::_M_valptr(), _Hash_node_value_base<>::_M_v()) Add [[__gnu__::__always_inline__]]. Ok to commit ? On 12/09/2023 18:09, Jonathan Wakely wrote: > On Mon, 11 Sept 2023 at 18:19, François Dumont <frs.dumont@gmail.com> wrote: >> >> On 11/09/2023 13:51, Jonathan Wakely wrote: >>> On Sun, 10 Sept 2023 at 14:57, François Dumont via Libstdc++ >>> <libstdc++@gcc.gnu.org> wrote: >>>> Following confirmation of the fix by TC here is the patch where I'm >>>> simply adding a 'constexpr' on _M_next(). >>>> >>>> Please let me know this ChangeLog entry is correct. I would prefer this >>>> patch to be assigned to 'TC' with me as co-author but I don't know how >>>> to do such a thing. Unless I need to change my user git identity to do so ? >>> Sam already explained that, but please check with Tim how he wants to >>> be credited, if at all. He doesn't have a copyright assignment, and >>> hasn't added a DCO sign-off to the patch, but it's small enough to not >>> need it as this is the first contribution credited to him. >>> >>> >>>> libstdc++: Add constexpr qualification to _Hash_node::_M_next() >>> What has this constexpr addition got to do with the ABI change and the >>> always_inline attributes? >>> >>> It certainly doesn't seem like it should be the summary line of the >>> git commit message. >> Oops, sorry, that's what I had started to do before Tim submitted anything. >> >> Here is latest version: > No patch attached, and the ChangeLog below still mentions the constexpr. > > I've pinged Tim via another channel to ask him about the author attribution. > > >> Author: TC <rs2740@gmail.com> >> Date: Wed Sep 6 19:31:55 2023 +0200 >> >> libstdc++: Force inline on _Hash_node_value_base methods to fix abi >> (PR111050) >> >> https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=1b6f0476837205932613ddb2b3429a55c26c409d >> changed _Hash_node_value_base to no longer derive from >> _Hash_node_base, which means >> that its member functions expect _M_storage to be at a different >> offset. So explosions >> result if an out-of-line definition is emitted for any of the >> member functions (say, >> in a non-optimized build) and the resulting object file is then >> linked with code built >> using older version of GCC/libstdc++. >> >> libstdc++-v3/ChangeLog: >> >> PR libstdc++/111050 >> * include/bits/hashtable_policy.h >> (_Hash_node_value_base<>::_M_valptr(), >> _Hash_node_value_base<>::_M_v()) >> Add [[__gnu__::__always_inline__]]. >> (_Hash_node<>::_M_next()): Add constexpr. >> >> Co-authored-by: François Dumont <fdumont@gcc.gnu.org> >> >> Ok for you TC (Tim ?) ? >> >>
Still no chance to get feedback from TC ? Maybe I can commit the below then ? AFAICS on gcc mailing list several gcc releases were done recently, too late. On 14/09/2023 06:46, François Dumont wrote: > Author: TC <rs2740@gmail.com> > Date: Wed Sep 6 19:31:55 2023 +0200 > > libstdc++: Force _Hash_node_value_base methods inline to fix abi > (PR111050) > > https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=1b6f0476837205932613ddb2b3429a55c26c409d > > changed _Hash_node_value_base to no longer derive from > _Hash_node_base, which means > that its member functions expect _M_storage to be at a different > offset. So explosions > result if an out-of-line definition is emitted for any of the > member functions (say, > in a non-optimized build) and the resulting object file is then > linked with code built > using older version of GCC/libstdc++. > > libstdc++-v3/ChangeLog: > > PR libstdc++/111050 > * include/bits/hashtable_policy.h > (_Hash_node_value_base<>::_M_valptr(), > _Hash_node_value_base<>::_M_v()) > Add [[__gnu__::__always_inline__]]. > > Ok to commit ? > > On 12/09/2023 18:09, Jonathan Wakely wrote: >> On Mon, 11 Sept 2023 at 18:19, François Dumont <frs.dumont@gmail.com> >> wrote: >>> >>> On 11/09/2023 13:51, Jonathan Wakely wrote: >>>> On Sun, 10 Sept 2023 at 14:57, François Dumont via Libstdc++ >>>> <libstdc++@gcc.gnu.org> wrote: >>>>> Following confirmation of the fix by TC here is the patch where I'm >>>>> simply adding a 'constexpr' on _M_next(). >>>>> >>>>> Please let me know this ChangeLog entry is correct. I would prefer >>>>> this >>>>> patch to be assigned to 'TC' with me as co-author but I don't know >>>>> how >>>>> to do such a thing. Unless I need to change my user git identity >>>>> to do so ? >>>> Sam already explained that, but please check with Tim how he wants to >>>> be credited, if at all. He doesn't have a copyright assignment, and >>>> hasn't added a DCO sign-off to the patch, but it's small enough to not >>>> need it as this is the first contribution credited to him. >>>> >>>> >>>>> libstdc++: Add constexpr qualification to >>>>> _Hash_node::_M_next() >>>> What has this constexpr addition got to do with the ABI change and the >>>> always_inline attributes? >>>> >>>> It certainly doesn't seem like it should be the summary line of the >>>> git commit message. >>> Oops, sorry, that's what I had started to do before Tim submitted >>> anything. >>> >>> Here is latest version: >> No patch attached, and the ChangeLog below still mentions the constexpr. >> >> I've pinged Tim via another channel to ask him about the author >> attribution. >> >> >>> Author: TC <rs2740@gmail.com> >>> Date: Wed Sep 6 19:31:55 2023 +0200 >>> >>> libstdc++: Force inline on _Hash_node_value_base methods to >>> fix abi >>> (PR111050) >>> >>> https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=1b6f0476837205932613ddb2b3429a55c26c409d >>> >>> changed _Hash_node_value_base to no longer derive from >>> _Hash_node_base, which means >>> that its member functions expect _M_storage to be at a different >>> offset. So explosions >>> result if an out-of-line definition is emitted for any of the >>> member functions (say, >>> in a non-optimized build) and the resulting object file is then >>> linked with code built >>> using older version of GCC/libstdc++. >>> >>> libstdc++-v3/ChangeLog: >>> >>> PR libstdc++/111050 >>> * include/bits/hashtable_policy.h >>> (_Hash_node_value_base<>::_M_valptr(), >>> _Hash_node_value_base<>::_M_v()) >>> Add [[__gnu__::__always_inline__]]. >>> (_Hash_node<>::_M_next()): Add constexpr. >>> >>> Co-authored-by: François Dumont <fdumont@gcc.gnu.org> >>> >>> Ok for you TC (Tim ?) ? >>> >>>
On Wed, 27 Sept 2023 at 05:44, François Dumont <frs.dumont@gmail.com> wrote: > > Still no chance to get feedback from TC ? Maybe I can commit the below > then ? I've heard back from Tim now. Please use "Tim Song <t.canens.cpp@gmail.com>" as the author. You can change the commit again using git commit --amend --author "Tim Song <t.canens.cpp@gmail.com>" OK for trunk with that change - thanks for waiting. > > AFAICS on gcc mailing list several gcc releases were done recently, too > late. There have been no releases this month, so the delay hasn't caused any problems. > > > On 14/09/2023 06:46, François Dumont wrote: > > Author: TC <rs2740@gmail.com> > > Date: Wed Sep 6 19:31:55 2023 +0200 > > > > libstdc++: Force _Hash_node_value_base methods inline to fix abi > > (PR111050) > > > > https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=1b6f0476837205932613ddb2b3429a55c26c409d > > > > changed _Hash_node_value_base to no longer derive from > > _Hash_node_base, which means > > that its member functions expect _M_storage to be at a different > > offset. So explosions > > result if an out-of-line definition is emitted for any of the > > member functions (say, > > in a non-optimized build) and the resulting object file is then > > linked with code built > > using older version of GCC/libstdc++. > > > > libstdc++-v3/ChangeLog: > > > > PR libstdc++/111050 > > * include/bits/hashtable_policy.h > > (_Hash_node_value_base<>::_M_valptr(), > > _Hash_node_value_base<>::_M_v()) > > Add [[__gnu__::__always_inline__]]. > > > > Ok to commit ? > > > > On 12/09/2023 18:09, Jonathan Wakely wrote: > >> On Mon, 11 Sept 2023 at 18:19, François Dumont <frs.dumont@gmail.com> > >> wrote: > >>> > >>> On 11/09/2023 13:51, Jonathan Wakely wrote: > >>>> On Sun, 10 Sept 2023 at 14:57, François Dumont via Libstdc++ > >>>> <libstdc++@gcc.gnu.org> wrote: > >>>>> Following confirmation of the fix by TC here is the patch where I'm > >>>>> simply adding a 'constexpr' on _M_next(). > >>>>> > >>>>> Please let me know this ChangeLog entry is correct. I would prefer > >>>>> this > >>>>> patch to be assigned to 'TC' with me as co-author but I don't know > >>>>> how > >>>>> to do such a thing. Unless I need to change my user git identity > >>>>> to do so ? > >>>> Sam already explained that, but please check with Tim how he wants to > >>>> be credited, if at all. He doesn't have a copyright assignment, and > >>>> hasn't added a DCO sign-off to the patch, but it's small enough to not > >>>> need it as this is the first contribution credited to him. > >>>> > >>>> > >>>>> libstdc++: Add constexpr qualification to > >>>>> _Hash_node::_M_next() > >>>> What has this constexpr addition got to do with the ABI change and the > >>>> always_inline attributes? > >>>> > >>>> It certainly doesn't seem like it should be the summary line of the > >>>> git commit message. > >>> Oops, sorry, that's what I had started to do before Tim submitted > >>> anything. > >>> > >>> Here is latest version: > >> No patch attached, and the ChangeLog below still mentions the constexpr. > >> > >> I've pinged Tim via another channel to ask him about the author > >> attribution. > >> > >> > >>> Author: TC <rs2740@gmail.com> > >>> Date: Wed Sep 6 19:31:55 2023 +0200 > >>> > >>> libstdc++: Force inline on _Hash_node_value_base methods to > >>> fix abi > >>> (PR111050) > >>> > >>> https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=1b6f0476837205932613ddb2b3429a55c26c409d > >>> > >>> changed _Hash_node_value_base to no longer derive from > >>> _Hash_node_base, which means > >>> that its member functions expect _M_storage to be at a different > >>> offset. So explosions > >>> result if an out-of-line definition is emitted for any of the > >>> member functions (say, > >>> in a non-optimized build) and the resulting object file is then > >>> linked with code built > >>> using older version of GCC/libstdc++. > >>> > >>> libstdc++-v3/ChangeLog: > >>> > >>> PR libstdc++/111050 > >>> * include/bits/hashtable_policy.h > >>> (_Hash_node_value_base<>::_M_valptr(), > >>> _Hash_node_value_base<>::_M_v()) > >>> Add [[__gnu__::__always_inline__]]. > >>> (_Hash_node<>::_M_next()): Add constexpr. > >>> > >>> Co-authored-by: François Dumont <fdumont@gcc.gnu.org> > >>> > >>> Ok for you TC (Tim ?) ? > >>> > >>> >
On 28/09/2023 18:18, Jonathan Wakely wrote: > On Wed, 27 Sept 2023 at 05:44, François Dumont <frs.dumont@gmail.com> wrote: >> Still no chance to get feedback from TC ? Maybe I can commit the below >> then ? > I've heard back from Tim now. Please use "Tim Song > <t.canens.cpp@gmail.com>" as the author. > > You can change the commit again using git commit --amend --author "Tim > Song <t.canens.cpp@gmail.com>" Sure :-) > > OK for trunk with that change - thanks for waiting. Committed to trunk, let me know for backports. >> AFAICS on gcc mailing list several gcc releases were done recently, too >> late. > There have been no releases this month, so the delay hasn't caused any problems. I was confused by emails like this one: https://gcc.gnu.org/pipermail/gcc/2023-September/242429.html I just subscribed to gcc mailing list, I had no idea there were regular snapshots like this.
On Thu, 28 Sept 2023 at 18:25, François Dumont <frs.dumont@gmail.com> wrote: > > > On 28/09/2023 18:18, Jonathan Wakely wrote: > > On Wed, 27 Sept 2023 at 05:44, François Dumont <frs.dumont@gmail.com> wrote: > >> Still no chance to get feedback from TC ? Maybe I can commit the below > >> then ? > > I've heard back from Tim now. Please use "Tim Song > > <t.canens.cpp@gmail.com>" as the author. > > > > You can change the commit again using git commit --amend --author "Tim > > Song <t.canens.cpp@gmail.com>" > > Sure :-) > > > > > OK for trunk with that change - thanks for waiting. > > Committed to trunk, let me know for backports. Please wait a few days in case of problems on the trunk (although I don't expect any) and then OK for 11/12/13 too - thanks! > > >> AFAICS on gcc mailing list several gcc releases were done recently, too > >> late. > > There have been no releases this month, so the delay hasn't caused any problems. > > I was confused by emails like this one: > > https://gcc.gnu.org/pipermail/gcc/2023-September/242429.html > > I just subscribed to gcc mailing list, I had no idea there were regular > snapshots like this. >
Now backport to gcc-11/12/13 branches. On 29/09/2023 11:53, Jonathan Wakely wrote: > On Thu, 28 Sept 2023 at 18:25, François Dumont <frs.dumont@gmail.com> wrote: >> >> On 28/09/2023 18:18, Jonathan Wakely wrote: >>> On Wed, 27 Sept 2023 at 05:44, François Dumont <frs.dumont@gmail.com> wrote: >>>> Still no chance to get feedback from TC ? Maybe I can commit the below >>>> then ? >>> I've heard back from Tim now. Please use "Tim Song >>> <t.canens.cpp@gmail.com>" as the author. >>> >>> You can change the commit again using git commit --amend --author "Tim >>> Song <t.canens.cpp@gmail.com>" >> Sure :-) >> >>> OK for trunk with that change - thanks for waiting. >> Committed to trunk, let me know for backports. > Please wait a few days in case of problems on the trunk (although I > don't expect any) and then OK for 11/12/13 too - thanks! > > >>>> AFAICS on gcc mailing list several gcc releases were done recently, too >>>> late. >>> There have been no releases this month, so the delay hasn't caused any problems. >> I was confused by emails like this one: >> >> https://gcc.gnu.org/pipermail/gcc/2023-September/242429.html >> >> I just subscribed to gcc mailing list, I had no idea there were regular >> snapshots like this. >>
diff --git a/libstdc++-v3/include/bits/hashtable_policy.h b/libstdc++-v3/include/bits/hashtable_policy.h index 347d468ea86..101c5eb639c 100644 --- a/libstdc++-v3/include/bits/hashtable_policy.h +++ b/libstdc++-v3/include/bits/hashtable_policy.h @@ -327,18 +327,22 @@ namespace __detail __gnu_cxx::__aligned_buffer<_Value> _M_storage; + [[__gnu__::__always_inline__]] _Value* _M_valptr() noexcept { return _M_storage._M_ptr(); } + [[__gnu__::__always_inline__]] const _Value* _M_valptr() const noexcept { return _M_storage._M_ptr(); } + [[__gnu__::__always_inline__]] _Value& _M_v() noexcept { return *_M_valptr(); } + [[__gnu__::__always_inline__]] const _Value& _M_v() const noexcept { return *_M_valptr(); } @@ -372,7 +376,7 @@ namespace __detail : _Hash_node_base , _Hash_node_value<_Value, _Cache_hash_code> { - _Hash_node* + constexpr _Hash_node* _M_next() const noexcept { return static_cast<_Hash_node*>(this->_M_nxt); } };