gimple: Optimise inlined gimple_seq_last

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(+)

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
>