[PATCH] annotate vector::_M_default_append fo better codegen (PR 83229)

[Jonathan Wakely]

On 11/12/17 12:35 +0000, Jonathan Wakely wrote:
>On 10/12/17 22:13 +0100, François Dumont wrote:
>>On 06/12/2017 04:44, Martin Sebor wrote:
>>>
>>>Since it's possible that there may be other opportunities for
>>>similar annotation in libstdc++, if changes along these lines
>>>become more than isolated instances, it may be worth considering
>>>adding a macro to make the conditions more readable.  E.g., to
>>>follow the example of __glibcxx_assert, something like:
>>>
>>>Â  #define __glibcxx_assume(expr) \
>>>Â Â Â  ((expr) ? (void)0 : __builtin_unreachable())
>>>
>>
>>Or perhaps do something like that:
>>
>>@@ -485,7 +485,9 @@ namespace std
>>Â #if defined(_GLIBCXX_ASSERTIONS)
>>Â # define __glibcxx_assert(_Condition) __glibcxx_assert_impl(_Condition)
>>Â #else
>>-# define __glibcxx_assert(_Condition)
>>+# define __glibcxx_assert(_Condition)Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  \
>>+Â Â Â  ((_Condition) ? (void)0 : __builtin_unreachable())
>>Â #endif
>>
>>So that assertions are automatically transform into optimizer hints 
>>on release builds.
>
>That's an interesting idea, but quite scary to make that change at
>this point in Stage 3, as it would affect dozens of places in the
>code. We should consider it in Stage 1 though.
>
>This might be a slightly better way to do it:
>
>--- a/libstdc++-v3/include/bits/c++config
>+++ b/libstdc++-v3/include/bits/c++config
>@@ -458,6 +458,9 @@ namespace std
>
>#if defined(_GLIBCXX_ASSERTIONS)
># define __glibcxx_assert(_Condition) __glibcxx_assert_impl(_Condition)
>+#elif defined(__OPTIMIZE__)
>+# define __glibcxx_assert(_Condition) \
>+  do { if (! (_Condition)) __builtin_unreachable(); } while (false)
>#else
># define __glibcxx_assert(_Condition)
>#endif
>
>Since the __builtin_unreachable won't be used when not optimizing, so
>there's no point evaluating the condition.
>
>All our tests pass with this change.

Thinking about it further, there are places we'd want to say "assume"
but don't want to "assert" (e.g. where Martin's patch adds
__builtin_unreachable) because we want to tell the compiler about
invariants, not just check user input. We wouldn't want to use
__glibcxx_assert for the change Martin is making.

So it would be better to have the __glibcxx_assume macro for those
situations, and then make __glibcxx_assert use __glibcxx_assume when
assertions are disabled.