gamescope
gamescope copied to clipboard
CloseAllFds: use ranges::any_of() instead of find() for better codegen
@Joshua-Ashton
I heard you were annoyed by how std::find()
was getting compiled for CloseAllFds()
I looked into this issue, and found that using std::ranges::find()
or std::ranges::any_of()
results in way simpler codegen compared to their non-range counterparts:
https://godbolt.org/z/dEnMcq1Kf
(note: for all versions of CloseAllFds()
given in the godbolt link, I put in
if (pBeginExclude == pEndExclude) {
__builtin_unreachable();
}
the sole purpose of this is to eliminate one conditional branch, to simplify control flow, in order to make it easier to compare all of the versions)
Now for a tidbit that's more for satisfying your curiosity as to what could possibly be causing std::find()
to lead to such convoluted assembly/machine code:
It turns out that it wasn't the compiler that was causing this, but actually the implementation of std::find()
.
On my archlinux system, the implementation of std::find()
is in /usr/include/14.1.1/bits/stl_algo.h
but it actually just calls std::__find_if
, which is in /usr/include/14.1.1/bits/stl_algobase.h
the version of __find_if
that's called in the godbolt examples, and in the 'real' version of CloseAllFds()
is:
/// This is an overload used by find algos for the RAI case.
template<typename _RandomAccessIterator, typename _Predicate>
_GLIBCXX20_CONSTEXPR
_RandomAccessIterator
__find_if(_RandomAccessIterator __first, _RandomAccessIterator __last,
_Predicate __pred, random_access_iterator_tag)
{
typename iterator_traits<_RandomAccessIterator>::difference_type
__trip_count = (__last - __first) >> 2;
for (; __trip_count > 0; --__trip_count)
{
if (__pred(__first))
return __first;
++__first;
if (__pred(__first))
return __first;
++__first;
if (__pred(__first))
return __first;
++__first;
if (__pred(__first))
return __first;
++__first;
}
switch (__last - __first)
{
case 3:
if (__pred(__first))
return __first;
++__first;
// FALLTHRU
case 2:
if (__pred(__first))
return __first;
++__first;
// FALLTHRU
case 1:
if (__pred(__first))
return __first;
++__first;
// FALLTHRU
case 0:
default:
return __last;
}
}
I'm not sure why exactly, but I guess ranges::find()
and ranges::any_of()
don't use that convoluted implementation...