Re: Patch: Remove unneeded double operation in libstdc++-v3/src/c++17/fs_path.cc
On Fri, 5 Jan 2024 at 13:00, Martin Küttler wrote: > > > >>This is a small change to libstdc++ which does not change any behavior. > > > > Please CC the libstd...@gcc.gnu.org list on all libstdc++ patches, as > > documented at https://gcc.gnu.org/lists.html > > Acknowledged. Sorry. > > >>This change has two, ihmo positive, implications: > >> > >> - The implicit conversion from double to int is avoided (Avoiding a > >> warning). > > > > I don't see any warning here. What do you see? > > I see "warning: conversion from ‘double’ to ‘int’ may change value > [-Wfloat-conversion]" This appears to be a specifically enabled warning. > > > Looking at path::_List::reserve now, we probably also want to avoid > > overflow. Although a path with INT_MAX/1.5 components seems > > implausible for 32-bit and 64-bit targets, it could be a problem for > > 16-bit targets. I'll take care of that too. > > Nice catch. We also have some redundant code in path::operator/= which can just be removed, because _List::reserve does it anyway: if (orig_type == _Type::_Multi) { const int curcap = _M_cmpts._M_impl->capacity(); if (capacity > curcap) capacity = std::max(capacity, (int) (curcap * 1.5)); }
Re: Patch: Remove unneeded double operation in libstdc++-v3/src/c++17/fs_path.cc
>>This is a small change to libstdc++ which does not change any behavior. > > Please CC the libstd...@gcc.gnu.org list on all libstdc++ patches, as > documented at https://gcc.gnu.org/lists.html Acknowledged. Sorry. >>This change has two, ihmo positive, implications: >> >> - The implicit conversion from double to int is avoided (Avoiding a >> warning). > > I don't see any warning here. What do you see? I see "warning: conversion from ‘double’ to ‘int’ may change value [-Wfloat-conversion]" This appears to be a specifically enabled warning. > Looking at path::_List::reserve now, we probably also want to avoid > overflow. Although a path with INT_MAX/1.5 components seems > implausible for 32-bit and 64-bit targets, it could be a problem for > 16-bit targets. I'll take care of that too. Nice catch. Martin -- Kernkonzept GmbH at Dresden, Germany, HRB 31129, CEO Dr.-Ing. Michael Hohmuth
Re: Patch: Remove unneeded double operation in libstdc++-v3/src/c++17/fs_path.cc
On 18/12/23 09:36 +0100, Martin Küttler wrote: This is a small change to libstdc++ which does not change any behavior. Please CC the libstd...@gcc.gnu.org list on all libstdc++ patches, as documented at https://gcc.gnu.org/lists.html Otherwise I won't see the patches unless I happen to glance at the gcc-patches archive by chance. This change has two, ihmo positive, implications: - The implicit conversion from double to int is avoided (Avoiding a warning). I don't see any warning here. What do you see? - No floating point number is used at all, which could be significant in some scenarios. Yes, it seems worth doing for this reason. I'll test+push the patch, thanks. Looking at path::_List::reserve now, we probably also want to avoid overflow. Although a path with INT_MAX/1.5 components seems implausible for 32-bit and 64-bit targets, it could be a problem for 16-bit targets. I'll take care of that too. diff --git a/libstdc++-v3/src/c++17/fs_path.cc b/libstdc++-v3/src/c++17/fs_path.cc index d65b5482e8b..b47ed0aa7aa 100644 --- a/libstdc++-v3/src/c++17/fs_path.cc +++ b/libstdc++-v3/src/c++17/fs_path.cc @@ -447,8 +447,9 @@ path::_List::reserve(int newcap, bool exact = false) if (curcap < newcap) { - if (!exact && newcap < int(1.5 * curcap)) - newcap = 1.5 * curcap; + const int nextcap = curcap + curcap / 2; + if (!exact && newcap < nextcap) + newcap = nextcap; void* p = ::operator new(sizeof(_Impl) + newcap * sizeof(value_type)); std::unique_ptr<_Impl, _Impl_deleter> newptr(::new(p) _Impl{newcap});
Patch: Remove unneeded double operation in libstdc++-v3/src/c++17/fs_path.cc
This is a small change to libstdc++ which does not change any behavior. This change has two, ihmo positive, implications: - The implicit conversion from double to int is avoided (Avoiding a warning). - No floating point number is used at all, which could be significant in some scenarios. diff --git a/libstdc++-v3/src/c++17/fs_path.cc b/libstdc++-v3/src/c++17/fs_path.cc index d65b5482e8b..b47ed0aa7aa 100644 --- a/libstdc++-v3/src/c++17/fs_path.cc +++ b/libstdc++-v3/src/c++17/fs_path.cc @@ -447,8 +447,9 @@ path::_List::reserve(int newcap, bool exact = false) if (curcap < newcap) { - if (!exact && newcap < int(1.5 * curcap)) - newcap = 1.5 * curcap; + const int nextcap = curcap + curcap / 2; + if (!exact && newcap < nextcap) + newcap = nextcap; void* p = ::operator new(sizeof(_Impl) + newcap * sizeof(value_type)); std::unique_ptr<_Impl, _Impl_deleter> newptr(::new(p) _Impl{newcap}); -- Kernkonzept GmbH at Dresden, Germany, HRB 31129, CEO Dr.-Ing. Michael Hohmuth