Patchwork Fix PR gcov-profile/55734 for bootstrapping with older compilers (issue6980044)

login
register
mail settings
Submitter Teresa Johnson
Date Dec. 19, 2012, 9:34 p.m.
Message ID <20121219213435.2E8FE6112F@tjsboxrox.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/207519/
State New
Headers show

Comments

Teresa Johnson - Dec. 19, 2012, 9:34 p.m.
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
Jakub Jelinek - Dec. 20, 2012, 1:19 a.m.
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
Jakub Jelinek - Dec. 20, 2012, 10:37 a.m.
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

Patch

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