[PATCH v2] gcov: Add GCOV_TYPE_SIZE target macro

[PATCH v2] gcov: Add GCOV_TYPE_SIZE target macro

[Sebastian Huber]

If -fprofile-update=atomic is used, then the target must provide atomic
operations for the counters of the type returned by get_gcov_type().
This is a 64-bit type for targets which have a 64-bit long long type.
On 32-bit targets this could be an issue since they may not provide
64-bit atomic operations.  Allow targets to override the default type
size with the new GCOV_TYPE_SIZE target macro.

If a 32-bit gcov type size is used, then there is currently a warning in
libgcov-driver.c in a dead code block due to
sizeof (counter) == sizeof (gcov_unsigned_t):

libgcc/libgcov-driver.c: In function 'dump_counter':
libgcc/libgcov-driver.c:401:46: warning: right shift count >= width of type [-Wshift-count-overflow]
  401 |     dump_unsigned ((gcov_unsigned_t)(counter >> 32), dump_fn, arg);
      |                                              ^~

gcc/

	* config/sparc/rtemself.h (GCOV_TYPE_SIZE): Define.
	* coverage.c (get_gcov_type): Use GCOV_TYPE_SIZE.
	* defaults.h (GCOV_TYPE_SIZE): Define default.
	* tree-profile.c (gimple_gen_edge_profiler): Use precision of
	gcov_type_node.
	(gimple_gen_time_profiler): Likewise.

libgcc/

	* libgcov.h (gcov_type): Define using GCOV_TYPE_SIZE.
	(gcov_type_unsigned): Likewise.
---
 gcc/config/sparc/rtemself.h | 2 ++
 gcc/coverage.c              | 3 +--
 gcc/defaults.h              | 8 ++++++++
 gcc/tree-profile.c          | 4 ++--
 libgcc/libgcov.h            | 6 +++---
 5 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/gcc/config/sparc/rtemself.h b/gcc/config/sparc/rtemself.h
index fa972af640cc..ac4f70c48c43 100644
--- a/gcc/config/sparc/rtemself.h
+++ b/gcc/config/sparc/rtemself.h
@@ -40,3 +40,5 @@
 
 /* Use the default */
 #undef LINK_GCC_C_SEQUENCE_SPEC
+
+#define GCOV_TYPE_SIZE 32
diff --git a/gcc/coverage.c b/gcc/coverage.c
index ac9a9fdad228..e655c164d664 100644
--- a/gcc/coverage.c
+++ b/gcc/coverage.c
@@ -145,8 +145,7 @@ static void coverage_obj_finish (vec<constructor_elt, va_gc> *);
 tree
 get_gcov_type (void)
 {
-  scalar_int_mode mode
-    = smallest_int_mode_for_size (LONG_LONG_TYPE_SIZE > 32 ? 64 : 32);
+  scalar_int_mode mode = smallest_int_mode_for_size (GCOV_TYPE_SIZE);
   return lang_hooks.types.type_for_mode (mode, false);
 }
 
diff --git a/gcc/defaults.h b/gcc/defaults.h
index ba79a8e48edd..242ae7a75a09 100644
--- a/gcc/defaults.h
+++ b/gcc/defaults.h
@@ -1475,4 +1475,12 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 typedef TARGET_UNIT target_unit;
 #endif
 
+#ifndef GCOV_TYPE_SIZE
+#if LONG_LONG_TYPE_SIZE > 32
+#define GCOV_TYPE_SIZE 64
+#else
+#define GCOV_TYPE_SIZE 32
+#endif
+#endif
+
 #endif  /* ! GCC_DEFAULTS_H */
diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c
index 5a74cc96e132..cf46912631cb 100644
--- a/gcc/tree-profile.c
+++ b/gcc/tree-profile.c
@@ -250,7 +250,7 @@ gimple_gen_edge_profiler (int edgeno, edge e)
     {
       /* __atomic_fetch_add (&counter, 1, MEMMODEL_RELAXED); */
       tree addr = tree_coverage_counter_addr (GCOV_COUNTER_ARCS, edgeno);
-      tree f = builtin_decl_explicit (LONG_LONG_TYPE_SIZE > 32
+      tree f = builtin_decl_explicit (TYPE_PRECISION (gcov_type_node) > 32
 				      ? BUILT_IN_ATOMIC_FETCH_ADD_8:
 				      BUILT_IN_ATOMIC_FETCH_ADD_4);
       gcall *stmt = gimple_build_call (f, 3, addr, one,
@@ -525,7 +525,7 @@ gimple_gen_time_profiler (unsigned tag)
 			  tree_time_profiler_counter);
       gassign *assign = gimple_build_assign (ptr, NOP_EXPR, addr);
       gsi_insert_before (&gsi, assign, GSI_NEW_STMT);
-      tree f = builtin_decl_explicit (LONG_LONG_TYPE_SIZE > 32
+      tree f = builtin_decl_explicit (TYPE_PRECISION (gcov_type_node) > 32
 				      ? BUILT_IN_ATOMIC_ADD_FETCH_8:
 				      BUILT_IN_ATOMIC_ADD_FETCH_4);
       gcall *stmt = gimple_build_call (f, 3, ptr, one,
diff --git a/libgcc/libgcov.h b/libgcc/libgcov.h
index 9c537253293f..ffa93c89cb0d 100644
--- a/libgcc/libgcov.h
+++ b/libgcc/libgcov.h
@@ -52,7 +52,7 @@
 #if __CHAR_BIT__ == 8
 typedef unsigned gcov_unsigned_t __attribute__ ((mode (SI)));
 typedef unsigned gcov_position_t __attribute__ ((mode (SI)));
-#if LONG_LONG_TYPE_SIZE > 32
+#if GCOV_TYPE_SIZE > 32
 typedef signed gcov_type __attribute__ ((mode (DI)));
 typedef unsigned gcov_type_unsigned __attribute__ ((mode (DI)));
 #else
@@ -63,7 +63,7 @@ typedef unsigned gcov_type_unsigned __attribute__ ((mode (SI)));
 #if __CHAR_BIT__ == 16
 typedef unsigned gcov_unsigned_t __attribute__ ((mode (HI)));
 typedef unsigned gcov_position_t __attribute__ ((mode (HI)));
-#if LONG_LONG_TYPE_SIZE > 32
+#if GCOV_TYPE_SIZE > 32
 typedef signed gcov_type __attribute__ ((mode (SI)));
 typedef unsigned gcov_type_unsigned __attribute__ ((mode (SI)));
 #else
@@ -73,7 +73,7 @@ typedef unsigned gcov_type_unsigned __attribute__ ((mode (HI)));
 #else
 typedef unsigned gcov_unsigned_t __attribute__ ((mode (QI)));
 typedef unsigned gcov_position_t __attribute__ ((mode (QI)));
-#if LONG_LONG_TYPE_SIZE > 32
+#if GCOV_TYPE_SIZE > 32
 typedef signed gcov_type __attribute__ ((mode (HI)));
 typedef unsigned gcov_type_unsigned __attribute__ ((mode (HI)));
 #else
-- 
2.26.2

Re: [PATCH v2] gcov: Add GCOV_TYPE_SIZE target macro

[Joseph Myers]

On Tue, 10 Aug 2021, Sebastian Huber wrote:

> If -fprofile-update=atomic is used, then the target must provide atomic
> operations for the counters of the type returned by get_gcov_type().
> This is a 64-bit type for targets which have a 64-bit long long type.
> On 32-bit targets this could be an issue since they may not provide
> 64-bit atomic operations.  Allow targets to override the default type
> size with the new GCOV_TYPE_SIZE target macro.

Any target macro needs to be documented in tm.texi.in (and tm.texi 
regenerated).

Please don't add new target macros that are used in both host and target 
code; every such macro is an obstruction to removing inclusion of the host 
tm.h from libgcc.  The appropriate way to handle sharing such 
configuration information is to provide a target hook in the host compiler 
code, and then make c-cppbuiltin.c:c_cpp_builtins define a corresponding 
predefined macro under the "if (flag_building_libgcc)" conditional.  
(Although a target hook is generally preferable to a target macro for 
host-side configuration, when a target macro is used on the host it's 
still desirable not to use it directly in target libraries but to use a 
predefined macro instead.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH v2] gcov: Add GCOV_TYPE_SIZE target macro

[Sebastian Huber]

On 10/08/2021 19:30, Joseph Myers wrote:
> On Tue, 10 Aug 2021, Sebastian Huber wrote:
> 
>> If -fprofile-update=atomic is used, then the target must provide atomic
>> operations for the counters of the type returned by get_gcov_type().
>> This is a 64-bit type for targets which have a 64-bit long long type.
>> On 32-bit targets this could be an issue since they may not provide
>> 64-bit atomic operations.  Allow targets to override the default type
>> size with the new GCOV_TYPE_SIZE target macro.
> Any target macro needs to be documented in tm.texi.in (and tm.texi
> regenerated).
> 
> Please don't add new target macros that are used in both host and target
> code; every such macro is an obstruction to removing inclusion of the host
> tm.h from libgcc.  The appropriate way to handle sharing such
> configuration information is to provide a target hook in the host compiler
> code, and then make c-cppbuiltin.c:c_cpp_builtins define a corresponding
> predefined macro under the "if (flag_building_libgcc)" conditional.
> (Although a target hook is generally preferable to a target macro for
> host-side configuration, when a target macro is used on the host it's
> still desirable not to use it directly in target libraries but to use a
> predefined macro instead.)

Ok, I understood how I can add a compiler provided define for libgcov. 
What is not clear to me is how I can add a target hook, set a default 
value, and customize it for a target/system. Is this described here

https://gcc.gnu.org/onlinedocs/gccint/Target-Structure.html

?

-- 
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/

Re: [PATCH v2] gcov: Add GCOV_TYPE_SIZE target macro

[Joseph Myers]

On Tue, 10 Aug 2021, Sebastian Huber wrote:

> Ok, I understood how I can add a compiler provided define for libgcov. What is
> not clear to me is how I can add a target hook, set a default value, and
> customize it for a target/system. Is this described here
> 
> https://gcc.gnu.org/onlinedocs/gccint/Target-Structure.html

Yes.

It's simplest when the hook definition depends only on the target 
architecture and not the target OS, but you can work around that when the 
OS affects the definition (have an architecture-specific function used as 
the hook definition, with a #if condition inside that function based on 
definitions from architecture-and-OS-specific headers, for example).

-- 
Joseph S. Myers
joseph@codesourcery.com