[PATCH] PR libstdc++/91788 improve codegen for std::variant<T...>::index()

Jonathan Wakely jwakely@redhat.com
Tue Sep 24 08:57:00 GMT 2019


On 23/09/19 19:39 +0200, Marc Glisse wrote:
>On Mon, 23 Sep 2019, Jonathan Wakely wrote:
>
>>If __index_type is a smaller type than size_t, then the result of
>>size_t(__index_type(-1)) is not equal to size_t(-1), but to an incorrect
>>value such as size_t(255) or size_t(65535). The old implementation of
>>variant<T...>::index() uses (size_t(__index_type(_M_index + 1)) - 1)
>>which is always correct, but generates suboptimal code for many common
>>cases.
>>
>>When the __index_type is size_t or valueless variants are not possible
>>we can just return the value directly.
>>
>>When the number of alternatives is sufficiently small the result of
>>converting the _M_index value to the corresponding signed type will be
>>either non-negative or -1. In those cases converting to the signed type
>>and then to size_t will either produce the correct positive value or
>>will sign extend -1 to (size_t)-1 as desired.
>>
>>For the remaining case we keep the existing arithmetic operations to
>>ensure the correct result.
>>
>>	PR libstdc++/91788 (partial)
>>	* include/std/variant (variant::index()): Improve codegen for cases
>>	where conversion to size_t already works correctly.
>>
>>Tested x86_64-linux, committed to trunk.
>
>Thanks.
>
>+       if constexpr (is_same_v<__index_type, size_t>)
>+         return this->_M_index;
>
>I don't think this special case is useful, gcc has no trouble 
>optimizing the other 2 versions to nothing when the types are the 
>same. Of course it won't hurt either.

My rationale was that it's much cheaper to instantiate is_same_v than
the __never_valueless<T...>() check (and will be even cheaper after
the concepts-cxx2a branch merges, as I plan to make is_same_v use the
__is_same_as built-in to avoid instantiating the std::is_same class
template).

That's probably not a big saving, as the __never_valueless function
template will almost certainly be used by some other member function
anyway.



More information about the Libstdc++ mailing list