| Message ID | 20121219213435.2E8FE6112F@tjsboxrox.mtv.corp.google.com |
|---|---|
| State | New |
| Headers | show |
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
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
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
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. -- This patch is available for review at http://codereview.appspot.com/6980044