On 16/05/19 12:29 +0100, Jonathan Wakely wrote:
These two changes both result in smaller code for std::variant.
The first one means smaller tables of function pointers, because we
don't generate an instantiation for the valueless state. Instead we do
a runtime branch, marked [[unlikely]] to make _M_reset() a no-op if
it's already valueless. In a microbenchmark I couldn't measure any
performance difference due to the extra branch, so the code size
reduction seems worthwhile.
The second one removes a branch from the index() member by relying on
unsigned arithmetic. That also results in smaller code and I can't see
any downside.
* include/std/variant (_Variant_storage<false, _Types...>::_M_reset):
Replace raw visitation with a runtime check for the valueless state
and a non-raw visitor.
(_Variant_storage<false, _Types...>::_M_reset_impl): Remove.
(variant::index()): Remove branch.
We might also want:
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -1503,7 +1503,7 @@ namespace __variant
}
else
{
- if (this->index() != variant_npos)
+ if (!this->valueless_by_exception()) [[__likely__]]
{
auto __tmp(std::move(__rhs_mem));
__rhs = std::move(*this);
@@ -1520,7 +1520,7 @@ namespace __variant
}
else
{
- if (this->index() != variant_npos)
+ if (!this->valueless_by_exception()) [[__likely__]]
{
__rhs = std::move(*this);
this->_M_reset();
This results in smaller code too, because for some specializations
valueless_by_exception() always returns false, so the branch can be
removed.
(This suggests that it's generally better to ask the yes/no question
"are you valid?" rather than "what is your index, and does it equal
this magic number?")
For specializations where a valueless state is possible we still
expect it to be very unlikely in practice, so the attribute should
help there.
