Re: Disable bounds checking for edge iterators in release compiler

[Richard Guenther <richard dot guenther at gmail dot com>]

On Tue, Jun 8, 2010 at 9:17 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> oprofile shows as one of highest sample count the following sanity check:
> ? ? ? ? ? ? ? :/* Advance the iterator to the next element. ?*/
> ? ? ? ? ? ? ? :static inline void
> ? ? ? ? ? ? ? :ei_next (edge_iterator *i)
> ? ? ? ? ? ? ? :{
> ?6578 ?0.4915 : ?gcc_assert (i->index < EDGE_COUNT (ei_container (*i)));
> ? 201 ?0.0150 : ?i->index++;
> ? ? ? ? ? ? ? :}
>
> I guess it is mostly cache miss that would be later attributed elsewhere, but
> it also seems to me that this kind of bounds checking is something we want to
> omit in release compiler (same was as we disable VEC range checking and such).
> The probability that we will get useful error here is relatively low compared
> to amount of checks we spread across compiler by inlining the function.
>
> I tested the attached patch. ?It reduce size of stripped cc1 binary (with WHOPR
> build) from 11944192 to 11939616 bytes (4.5KB) and time needed to build cc1
> binary from 10m29s to 10m19s. With non-WHOPR this should be more effective
> since we are worse on optimizing out unnecesary checks, but I didn't verified.
>
> Bootstrapped/regtested x86_64-linux. If this seems to make sense, I will look at the
> other inline functions sanity checks. ?I think in gimple.h we also have few quite
> good candidates for ENABLE_CHECKING.

I thought I've gone over gimple.h at some point ...

Ok.

Thanks,
Richard.

> Honza
>
> ? ? ? ?* basic-block.h (single_succ_edge, single_pred_edge, ei_container,
> ? ? ? ?ei_next, ei_prev): Do sanity checking with ENABLE_CHECKING only.
> Index: basic-block.h
> ===================================================================
> --- basic-block.h ? ? ? (revision 160382)
> +++ basic-block.h ? ? ? (working copy)
> @@ -554,7 +554,9 @@ single_pred_p (const_basic_block bb)
> ?static inline edge
> ?single_succ_edge (const_basic_block bb)
> ?{
> +#ifdef ENABLE_CHECKING
> ? gcc_assert (single_succ_p (bb));
> +#endif
> ? return EDGE_SUCC (bb, 0);
> ?}
>
> @@ -564,7 +566,9 @@ single_succ_edge (const_basic_block bb)
> ?static inline edge
> ?single_pred_edge (const_basic_block bb)
> ?{
> +#ifdef ENABLE_CHECKING
> ? gcc_assert (single_pred_p (bb));
> +#endif
> ? return EDGE_PRED (bb, 0);
> ?}
>
> @@ -596,7 +600,9 @@ typedef struct {
> ?static inline VEC(edge,gc) *
> ?ei_container (edge_iterator i)
> ?{
> +#ifdef ENABLE_CHECKING
> ? gcc_assert (i.container);
> +#endif
> ? return *i.container;
> ?}
>
> @@ -647,7 +653,9 @@ ei_one_before_end_p (edge_iterator i)
> ?static inline void
> ?ei_next (edge_iterator *i)
> ?{
> +#ifdef ENABLE_CHECKING
> ? gcc_assert (i->index < EDGE_COUNT (ei_container (*i)));
> +#endif
> ? i->index++;
> ?}
>
> @@ -655,7 +663,9 @@ ei_next (edge_iterator *i)
> ?static inline void
> ?ei_prev (edge_iterator *i)
> ?{
> +#ifdef ENABLE_CHECKING
> ? gcc_assert (i->index > 0);
> +#endif
> ? i->index--;
> ?}
>
>