On Sun, Dec 5, 2021 at 11:02 PM Richard Sandiford via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> In self-compilations, GCC doesn't realise that gimple_seq_last
> always returns nonnull when the argument is nonnull. Although
> it's a small thing in itself, the function is used (indirectly)
> many times and the extra null checks bloat what are otherwise
> simple loops.
>
> This patch adds an optimisation hint to tell the compiler that
> s && !s->prev is impossible.
> This gives a small (<1%) but
> measureable improvement in compile time. The original intention
> was to make a loop small enough that it could be turned into an
> inline function.
>
> The form of assertion in the patch:
>
> __extension__ ({ if (!(EXPR)) __builtin_unreachable (); (void) 0; })
>
> seems to work more reliably than the form used by release-checking
> gcc_asserts:
>
> ((void)(__builtin_expect (!(EXPR), 0) ? __builtin_unreachable (), 0 : 0))
>
> For that reason, I wondered about making gcc_assert use the
> new macro too. However, gcc_assert is currently inconsistent:
> it preserves side-effects even in release compilers when compiled
> with GCC, but it doesn't when compiled by other compilers.
> This difference dates back to 2013 (g:2df77822ee0974d84b2)
> and it looks like we now have several gcc_asserts that assume
> that side effects will be preserved. It therefore seemed better
> to deal with that separately.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
>
> Richard
>
>
> gcc/
> * system.h (optimization_assert): New macro.
> * gimple.h (gimple_seq_last): Use it to assert that s->prev is
> never null.
> ---
> gcc/gimple.h | 2 ++
> gcc/system.h | 10 ++++++++++
> 2 files changed, 12 insertions(+)
>
> diff --git a/gcc/gimple.h b/gcc/gimple.h
> index f7fdefc5362..8162c1ea507 100644
> --- a/gcc/gimple.h
> +++ b/gcc/gimple.h
> @@ -1723,6 +1723,8 @@ gimple_seq_first_stmt_as_a_bind (gimple_seq s)
> static inline gimple_seq_node
> gimple_seq_last (gimple_seq s)
> {
> + /* Helps to avoid a double branch in callers. */
> + optimization_assert (!s || s->prev);
> return s ? s->prev : NULL;
Does it work when you write it in a more legible way like
if (s)
{
/* gimple_seq is linked cyclic in prev. */
gcc_assert (s->prev);
return s->prev;
}
return NULL;
?
> }
>
> diff --git a/gcc/system.h b/gcc/system.h
> index 4ac656c9c3c..6e27b3270b9 100644
> --- a/gcc/system.h
> +++ b/gcc/system.h
> @@ -778,6 +778,16 @@ extern void fancy_abort (const char *, int, const char *)
> ATTRIBUTE_NORETURN ATTRIBUTE_COLD;
> #define abort() fancy_abort (__FILE__, __LINE__, __FUNCTION__)
>
> +/* Tell the compiler of GCC that EXPR is known to be true. This is purely
> + an optimization hint and EXPR must not have any side effects. */
> +#if (GCC_VERSION >= 4005)
> +#define optimization_assert(EXPR) \
> + __extension__ ({ if (!(EXPR)) __builtin_unreachable (); (void) 0; })
> +#else
> +/* Include EXPR, so that unused variable warnings do not occur. */
> +#define optimization_assert(EXPR) ((void)(0 && (EXPR)))
> +#endif
> +
> /* Use gcc_assert(EXPR) to test invariants. */
> #if ENABLE_ASSERT_CHECKING
> #define gcc_assert(EXPR) \
> --
> 2.31.1
>
