Re: [PATCH] optab fixes (PR middle-end/33335)

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

On Nov 6, 2007 3:52 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> PR middle-end/33335 is a PCH related failure with optabs.  The problem
> was introduced by the lazy optab initialization patches by Jan.
> http://gcc.gnu.org/ml/gcc-patches/2007-09/msg00288.html
> attempted to fix PCH failures by not storing optab values (== struct optab *)
> but some difference between them in the GTY marked libfunc_hash.
> But this is not enough, because each optab (resp. convert_optab) is
> malloced separately, so it is saving differences between pointers returned
> by different calls to malloc.  Besides this being undefined pointer
> arithmetics, we really have no guarantee in each invocation all the almost
> 200 malloc calls will return the same pointers or at least pointers which
> are all biased by some value (with the same differences between each pair).
>
> All these allocations is done inside of init_optabs and all have same size
> (all optab_table have one and all convert_optab_table have a different one).
>
> So the shortest fix would be to change init_optabs to malloc two (or one)
> big chunk of memory and let new_optab resp. new_convert_optab return the
> consecutive subchunks from that array - then the pointers would be into
> the same object and thus with defined behavior on pointer arithmetics
> and the differences between any two such pointers couldn't change between
> different invocations of the same compiler.
>
> This leads to another question, if all the optabs are stored consecutively
> in memory, why do we need pointers to it when pointer arithmetics can
> avoid one extra memory read.  And, at least when compiled with recent GCC
> we can use designated range initializers to avoid initializing the handlers
> at runtime.  This is what the following patch does.  And it even speeds
> up cc1 startup a little bit.
>
> Haven't benchmarked this with --enable-checking=release, but normal
> --enable-checking=yes; on x86_64 I get:
> cat U.c
> ;
> ~/timing -c 500 ./cc1.vanilla -quiet U.c -o - > /dev/null
> Strip out best and worst 10 realtime results
> minimum: 0.007994958 sec real / 0.000023252 sec CPU
> maximum: 0.008563484 sec real / 0.000050906 sec CPU
> average: 0.008411853 sec real / 0.000035306 sec CPU
> stdev  : 0.000082964 sec real / 0.000002534 sec CPU
> ~/timing -c 500 ./cc1.patched -quiet U.c -o - > /dev/null
> Strip out best and worst 10 realtime results
> minimum: 0.007698567 sec real / 0.000023930 sec CPU
> maximum: 0.008185790 sec real / 0.000046379 sec CPU
> average: 0.008002160 sec real / 0.000034471 sec CPU
> stdev  : 0.000094689 sec real / 0.000002084 sec CPU
>
> It is not much, but still almost 5% improvement.  So far only lightly tested
> on x86_64-linux, but will soon bootstrap/regtest this on ppc64-linux where
> this bug caused a bunch of libstdc++-v3 testsuite failures.
> The patch is large, but most of the changes are mechanical.
>
> Ok if testing passes?

This is ok.

Thanks,
Richard.

> 2007-11-06  Jakub Jelinek  <jakub@redhat.com>
>
>         PR middle-end/33335
>         * optabs.h (optab_table): Change type to struct optab from optab.
>         (convert_optab_table): Similarly.
>         Adjust all _optab macros to expand to addresses of optab_table
>         resp. convert_optab_table elements rather than the values of the
>         elements.
>         * optabs.c (optab_table): Change typype to struct optab from optab.
>         If compiled by recent GCC, initialize all the insn_code fields in
>         the array using designated range initializers.
>         (convert_optab_table): Similarly.
>         (convert_optab_libfunc, optab_libfunc, set_optab_libfunc,
>         set_conv_libfunc): Into libfunc_entry's optab field store index within
>         optab_table resp. convert_optab_table array instead of difference
>         between two separately malloced addresses.
>         (new_optab): Add op argument, don't return anything.  Initialize
>         just the insn_code field in handlers array and only when needed.
>         (init_optab, init_optabv): Add op argument, don't return anything.
>         Adjust new_optab caller.
>         (new_convert_optab): Removed.
>         (init_convert_optab): Add op argument, don't return anything, don't
>         call new_convert_optab.  If needed, initialize insn_code field in
>         handlers array.
>         (init_optabs): Adjust init_optab, init_optabv and init_convert_optab
>         callers.
>         (debug_optab_libfuncs): Adjust uses of optab_table and
>         convert_optab_table arrays.