Issue 6980044: [PATCH] Fix PR gcov-profile/55734 for bootstrapping with older compilers
|
Created:
11 years, 4 months ago by tejohnson Modified:
9 years, 8 months ago Reviewers:
jakub, rguenther CC:
gcc-patches_gcc.gnu.org Base URL:
svn+ssh://gcc.gnu.org/svn/gcc/trunk/gcc/ Visibility:
Public. |
Patch Set 1 #MessagesTotal messages: 4
Fix PR gcov-profile/55734 by using methods from hwint.c instead of builtins, to handle non-GCC and older versions of GCC. When building libgcov.a, however, hwint.c is not available, but we are always using the bootstrapped compiler and can therefore use the builtins. Use __builtin_popcount instead of __builtin_popcountll, since we are operating on an int. Use floor_log2 directly, instead of clz_hwi for the non-libgcov case, and handle situations where the size of the gcov_type is bigger than HOST_WIDE_INT. Verified that the various cases compiled by forcing different HOST_BITS_PER_WIDE_INT values. Bootstrapped and tested on x86_64-unknown-linux-gnu. Ok for trunk? Thanks, Teresa 2012-12-19 Teresa Johnson <tejohnson@google.com> Jakub Jelinek <jakub@redhat.com> PR gcov-profile/55734 * gcov-io.c (gcov_read_summary): Use __builtin_popcount instead of __builtin_popcountll when building libgcov.a, otherwise use popcount_hwi. (gcov_histo_index): When not building libgcov.a, use floor_log2 instead of __builtin_clzll. Index: gcov-io.c =================================================================== --- gcov-io.c (revision 194585) +++ gcov-io.c (working copy) @@ -538,7 +538,15 @@ gcov_read_summary (struct gcov_summary *summary) for (bv_ix = 0; bv_ix < GCOV_HISTOGRAM_BITVECTOR_SIZE; bv_ix++) { histo_bitvector[bv_ix] = gcov_read_unsigned (); - h_cnt += __builtin_popcountll (histo_bitvector[bv_ix]); +#if IN_LIBGCOV + /* When building libgcov we don't include system.h, which includes + hwint.h (where popcount_hwi is declared). However, libgcov.a + is built by the bootstrapped compiler and therefore the builtins + are always available. */ + h_cnt += __builtin_popcount (histo_bitvector[bv_ix]); +#else + h_cnt += popcount_hwi (histo_bitvector[bv_ix]); +#endif } bv_ix = 0; h_ix = 0; @@ -642,7 +650,33 @@ gcov_histo_index (gcov_type value) /* Find the place of the most-significant bit set. */ if (v > 0) - r = 63 - __builtin_clzll (v); + { +#if IN_LIBGCOV + /* When building libgcov we don't include system.h, which includes + hwint.h (where floor_log2 is declared). However, libgcov.a + is built by the bootstrapped compiler and therefore the builtins + are always available. */ + r = 63 - __builtin_clzll (v); +#else + /* We use floor_log2 from hwint.c, which takes a HOST_WIDE_INT + that is either 32 or 64 bits, and gcov_type_unsigned may be 64 bits. + Need to check for the case where gcov_type_unsigned is 64 bits + and HOST_WIDE_INT is 32 bits and handle it specially. */ +#if LONG_LONG_TYPE_SIZE <= HOST_BITS_PER_WIDE_INT + r = floor_log2 (v); +#else +#if LONG_LONG_TYPE_SIZE == 2 * HOST_BITS_PER_WIDE_INT + HOST_WIDE_INT hwi_v = v >> HOST_BITS_PER_WIDE_INT; + if (hwi_v) + r = floor_log2 (hwi_v) + HOST_BITS_PER_WIDE_INT; + else + r = floor_log2 ((HOST_WIDE_INT)v); +#else + gcc_unreachable (); +#endif +#endif +#endif + } /* If at most the 2 least significant bits are set (value is 0 - 3) then that value is our index into the lowest set of -- This patch is available for review at http://codereview.appspot.com/6980044
Sign in to reply to this message.
On Wed, Dec 19, 2012 at 01:34:34PM -0800, Teresa Johnson wrote:
> +#if IN_LIBGCOV
> + /* When building libgcov we don't include system.h, which includes
> + hwint.h (where floor_log2 is declared). However, libgcov.a
> + is built by the bootstrapped compiler and therefore the builtins
> + are always available. */
> + r = 63 - __builtin_clzll (v);
Perhaps it would be more portable to use
r = sizeof (long long) * __CHAR_BIT__ - 1 - __builtin_clzll (v);
here.
> +#else
> + /* We use floor_log2 from hwint.c, which takes a HOST_WIDE_INT
> + that is either 32 or 64 bits, and gcov_type_unsigned may be 64
bits.
> + Need to check for the case where gcov_type_unsigned is 64 bits
> + and HOST_WIDE_INT is 32 bits and handle it specially. */
> +#if LONG_LONG_TYPE_SIZE <= HOST_BITS_PER_WIDE_INT
If not in libgcov.a, we have
typedef unsigned HOST_WIDEST_INT gcov_type_unsigned;
in gcov-io.h. LONG_LONG_TYPE_SIZE is target long long size, you are
interested about the size of host gcov_type_unsigned.
So perhaps test HOST_BITS_PER_WIDEST_INT instead of LONG_LONG_TYPE_SIZE?
> + r = floor_log2 (v);
> +#else
> +#if LONG_LONG_TYPE_SIZE == 2 * HOST_BITS_PER_WIDE_INT
Likewise. Can't you use #elif above and get rid of one of the #endif lines?
> + HOST_WIDE_INT hwi_v = v >> HOST_BITS_PER_WIDE_INT;
> + if (hwi_v)
> + r = floor_log2 (hwi_v) + HOST_BITS_PER_WIDE_INT;
> + else
> + r = floor_log2 ((HOST_WIDE_INT)v);
> +#else
> + gcc_unreachable ();
> +#endif
> +#endif
> +#endif
> + }
>
Jakub
Sign in to reply to this message.
On Wed, Dec 19, 2012 at 5:19 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Wed, Dec 19, 2012 at 01:34:34PM -0800, Teresa Johnson wrote: > > +#if IN_LIBGCOV > > + /* When building libgcov we don't include system.h, which includes > > + hwint.h (where floor_log2 is declared). However, libgcov.a > > + is built by the bootstrapped compiler and therefore the > builtins > > + are always available. */ > > + r = 63 - __builtin_clzll (v); > > Perhaps it would be more portable to use > r = sizeof (long long) * __CHAR_BIT__ - 1 - __builtin_clzll (v); > here. > Ok. > > > +#else > > + /* We use floor_log2 from hwint.c, which takes a HOST_WIDE_INT > > + that is either 32 or 64 bits, and gcov_type_unsigned may be 64 > bits. > > + Need to check for the case where gcov_type_unsigned is 64 bits > > + and HOST_WIDE_INT is 32 bits and handle it specially. */ > > +#if LONG_LONG_TYPE_SIZE <= HOST_BITS_PER_WIDE_INT > > If not in libgcov.a, we have > typedef unsigned HOST_WIDEST_INT gcov_type_unsigned; > in gcov-io.h. LONG_LONG_TYPE_SIZE is target long long size, you are > interested about the size of host gcov_type_unsigned. > So perhaps test HOST_BITS_PER_WIDEST_INT instead of LONG_LONG_TYPE_SIZE? > Yes, that is better. I left it as a <= test, but perhaps it only makes sense to check here if HOST_BITS_PER_WIDEST_INT == HOST_BITS_PER_WIDE_INT, since I don't think it could be smaller. Made that change too. > > > + r = floor_log2 (v); > > +#else > > +#if LONG_LONG_TYPE_SIZE == 2 * HOST_BITS_PER_WIDE_INT > > Likewise. Can't you use #elif above and get rid of one of the #endif > lines? > Merged this pair into an #elif, but left the outer one (from the IN_LIBGCOV check) since it looks clearer. New patch: 2012-12-19 Teresa Johnson <tejohnson@google.com> Jakub Jelinek <jakub@redhat.com> PR gcov-profile/55734 * gcov-io.c (gcov_read_summary): Use __builtin_popcount instead of __builtin_popcountll when building libgcov.a, otherwise use popcount_hwi. (gcov_histo_index): When not building libgcov.a, use floor_log2 instead of __builtin_clzll. Index: gcov-io.c =================================================================== --- gcov-io.c (revision 194585) +++ gcov-io.c (working copy) @@ -538,7 +538,15 @@ gcov_read_summary (struct gcov_summary *summary) for (bv_ix = 0; bv_ix < GCOV_HISTOGRAM_BITVECTOR_SIZE; bv_ix++) { histo_bitvector[bv_ix] = gcov_read_unsigned (); - h_cnt += __builtin_popcountll (histo_bitvector[bv_ix]); +#if IN_LIBGCOV + /* When building libgcov we don't include system.h, which includes + hwint.h (where popcount_hwi is declared). However, libgcov.a + is built by the bootstrapped compiler and therefore the builtins + are always available. */ + h_cnt += __builtin_popcount (histo_bitvector[bv_ix]); +#else + h_cnt += popcount_hwi (histo_bitvector[bv_ix]); +#endif } bv_ix = 0; h_ix = 0; @@ -642,7 +650,31 @@ gcov_histo_index (gcov_type value) /* Find the place of the most-significant bit set. */ if (v > 0) - r = 63 - __builtin_clzll (v); + { +#if IN_LIBGCOV + /* When building libgcov we don't include system.h, which includes + hwint.h (where floor_log2 is declared). However, libgcov.a + is built by the bootstrapped compiler and therefore the builtins + are always available. */ + r = sizeof (long long) * __CHAR_BIT__ - 1 - __builtin_clzll (v); +#else + /* We use floor_log2 from hwint.c, which takes a HOST_WIDE_INT + that is either 32 or 64 bits, and gcov_type_unsigned may be 64 bits. + Need to check for the case where gcov_type_unsigned is 64 bits + and HOST_WIDE_INT is 32 bits and handle it specially. */ +#if HOST_BITS_PER_WIDEST_INT == HOST_BITS_PER_WIDE_INT + r = floor_log2 (v); +#elif HOST_BITS_PER_WIDEST_INT == 2 * HOST_BITS_PER_WIDE_INT + HOST_WIDE_INT hwi_v = v >> HOST_BITS_PER_WIDE_INT; + if (hwi_v) + r = floor_log2 (hwi_v) + HOST_BITS_PER_WIDE_INT; + else + r = floor_log2 ((HOST_WIDE_INT)v); +#else + gcc_unreachable (); +#endif +#endif + } /* If at most the 2 least significant bits are set (value is 0 - 3) then that value is our index into the lowest set of Thanks, Teresa > > > + HOST_WIDE_INT hwi_v = v >> HOST_BITS_PER_WIDE_INT; > > + if (hwi_v) > > + r = floor_log2 (hwi_v) + HOST_BITS_PER_WIDE_INT; > > + else > > + r = floor_log2 ((HOST_WIDE_INT)v); > > +#else > > + gcc_unreachable (); > > +#endif > > +#endif > > +#endif > > + } > > > > Jakub > -- Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
Sign in to reply to this message.
On Wed, Dec 19, 2012 at 10:14:26PM -0800, Teresa Johnson wrote: > Merged this pair into an #elif, but left the outer one (from the IN_LIBGCOV > check) since it looks clearer. > > New patch: > > 2012-12-19 Teresa Johnson <tejohnson@google.com> > Jakub Jelinek <jakub@redhat.com> > > PR gcov-profile/55734 > * gcov-io.c (gcov_read_summary): Use __builtin_popcount instead > of __builtin_popcountll when building libgcov.a, otherwise use > popcount_hwi. > (gcov_histo_index): When not building libgcov.a, use floor_log2 > instead of __builtin_clzll. Okay, thanks. Jakub
Sign in to reply to this message.
|
