[v3] gcov: Split atomic bitwise-or for some targets
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gcc_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_simplebootstrap_build--master-aarch64-bootstrap |
success
|
Build passed
|
Commit Message
There are targets which only offer 32-bit atomic operations (for example
32-bit RISC-V). For these targets, split the 64-bit atomic bitwise-or
operation used to update condition/decision coverage counters into up to
two 32-bit atomic bitwise-or operations, one per 32-bit half of the
counter. If a half folds to a compile-time constant zero, then the
atomic bitwise-or operation for that half is a no-op and is omitted, the
same optimization already applied to whole-counter updates.
For this test case
int a(int i);
int b(int i);
int f(int i)
{
if (i) {
return a(i);
} else {
return b(i);
}
}
with options
-O2 -fprofile-update=atomic -fcondition-coverage
on a 32-bit RISC-V target with the atomic extension but no 64-bit atomic
instructions, each counter update now generates a single inlined
amoor.w zero,a4,0(a5)
instead of a call into libatomic. The always-zero high 32-bit half of
the counter is folded away at compile time, so its atomic bitwise-or
operation is omitted. On a target with 64-bit atomic instructions, such
as 64-bit RISC-V, a single
amoor.d zero,a4,0(a5)
is generated, unchanged from before this patch.
Tested on x86_64-pc-linux-gnu (gcov.exp, gcc.dg gcov*.c, tree-prof.exp)
with no regressions, and cross-checked code generation on 32-bit and
64-bit RISC-V (rv32ima/ilp32 and rv64ima/lp64).
gcc/ChangeLog:
* tree-profile.cc (split_update_decision_counter): New.
(instrument_decisions): Use counter_update to determine which
atomic operations are available. Use
split_update_decision_counter() if 64-bit atomic operations can
be split up into two 32-bit atomic operations.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/amo/gcov-condition-coverage-split-ior.c: New
test.
Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
---
v2:
* Properly use a right shift (RSHIFT_EXPR) to get the higher order
32-bits of the counter.
* Fix style issue.
v3:
* Update to take into account the recent counter optimization changes.
* Add test case.
.../amo/gcov-condition-coverage-split-ior.c | 27 ++++++++
gcc/tree-profile.cc | 66 +++++++++++++++++--
2 files changed, 88 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/amo/gcov-condition-coverage-split-ior.c
Comments
On 7/16/2026 5:59 PM, Sebastian Huber wrote:
> There are targets which only offer 32-bit atomic operations (for example
> 32-bit RISC-V). For these targets, split the 64-bit atomic bitwise-or
> operation used to update condition/decision coverage counters into up to
> two 32-bit atomic bitwise-or operations, one per 32-bit half of the
> counter. If a half folds to a compile-time constant zero, then the
> atomic bitwise-or operation for that half is a no-op and is omitted, the
> same optimization already applied to whole-counter updates.
>
> For this test case
>
> int a(int i);
> int b(int i);
>
> int f(int i)
> {
> if (i) {
> return a(i);
> } else {
> return b(i);
> }
> }
>
> with options
>
> -O2 -fprofile-update=atomic -fcondition-coverage
>
> on a 32-bit RISC-V target with the atomic extension but no 64-bit atomic
> instructions, each counter update now generates a single inlined
>
> amoor.w zero,a4,0(a5)
>
> instead of a call into libatomic. The always-zero high 32-bit half of
> the counter is folded away at compile time, so its atomic bitwise-or
> operation is omitted. On a target with 64-bit atomic instructions, such
> as 64-bit RISC-V, a single
>
> amoor.d zero,a4,0(a5)
>
> is generated, unchanged from before this patch.
>
> Tested on x86_64-pc-linux-gnu (gcov.exp, gcc.dg gcov*.c, tree-prof.exp)
> with no regressions, and cross-checked code generation on 32-bit and
> 64-bit RISC-V (rv32ima/ilp32 and rv64ima/lp64).
>
> gcc/ChangeLog:
>
> * tree-profile.cc (split_update_decision_counter): New.
> (instrument_decisions): Use counter_update to determine which
> atomic operations are available. Use
> split_update_decision_counter() if 64-bit atomic operations can
> be split up into two 32-bit atomic operations.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/amo/gcov-condition-coverage-split-ior.c: New
> test.
>
> Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
> ---
>
> v2:
>
> * Properly use a right shift (RSHIFT_EXPR) to get the higher order
> 32-bits of the counter.
>
> * Fix style issue.
>
> v3:
>
> * Update to take into account the recent counter optimization changes.
>
> * Add test case.
>
>
> +static void
> +split_update_decision_counter (gimple_seq *seq, tree ref, tree counter,
> + tree atomic_ior_32, tree relaxed)
> +{
> + ref = unshare_expr (ref);
> +
> + /* Get the low and high address of the referenced counter */
Make sure your comments are full sentences. End them with a period, two
spaces and the close comment the same nit is repeated a few times in
this function.
> @@ -1191,6 +1244,9 @@ instrument_decisions (array_slice<basic_block> expr, size_t condno,
> next[k], relaxed);
> gimple_seq_add_stmt (&seq, flush);
> }
> + else if (use_atomic_split)
> + split_update_decision_counter (&seq, ref, next[k],
> + atomic_ior_32, relaxed);
Another nit. It's a bit incconsistent, but our coding guidelines would
have the call to split_udpate_decision_counter indented 2 positions
inside the else-if, not 4 positions like you've done. 4 positions would
be right if you had enclosing curly braces, but those would be redundant
here (and it does get called out regularly). So adjust the indention to
just two positions inside the else-if.
OK with the nits fixed. No need for another review round.
jeff
new file mode 100644
@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=rv32gc -mabi=ilp32 -fprofile-update=atomic -fcondition-coverage" { target { rv32 } } } */
+/* { dg-options "-O2 -march=rv64gc -mabi=lp64 -fprofile-update=atomic -fcondition-coverage" { target { rv64 } } } */
+
+/* rv32 only has 32-bit atomic instructions, so the 64-bit gcov decision
+ counter update is split into up to two 32-bit atomic bitwise-or
+ operations, one per 32-bit half. Here each counter update is a small
+ compile-time constant, so the always-zero high half is folded away and
+ only one atomic bitwise-or per branch remains. rv64 has 64-bit atomic
+ instructions, so a single atomic bitwise-or suffices there too. */
+
+int a (int);
+int b (int);
+
+int
+f (int i)
+{
+ if (i)
+ return a (i);
+ else
+ return b (i);
+}
+
+/* { dg-final { scan-assembler-times "\tamoor\\.w\t" 2 { target { rv32 } } } } */
+/* { dg-final { scan-assembler-not "\tamoor\\.d\t" { target { rv32 } } } } */
+/* { dg-final { scan-assembler-times "\tamoor\\.d\t" 2 { target { rv64 } } } } */
+/* { dg-final { scan-assembler-not "\tamoor\\.w\t" { target { rv64 } } } } */
@@ -1033,6 +1033,53 @@ resolve_counters (vec<counters> &cands)
}
+/* Append statements to SEQ that update the decision counter referenced by
+ REF with the COUNTER. Generate two separate 32-bit atomic bitwise-or
+ operations specified by ATOMIC_IOR_32 in the RELAXED memory order. If a
+ 32-bit part of COUNTER folds to a constant zero, then the atomic
+ bitwise-or operation for this part is a no-op and is omitted. */
+static void
+split_update_decision_counter (gimple_seq *seq, tree ref, tree counter,
+ tree atomic_ior_32, tree relaxed)
+{
+ ref = unshare_expr (ref);
+
+ /* Get the low and high address of the referenced counter */
+ tree addr_low = build_addr (ref);
+ tree four = build_int_cst (size_type_node, 4);
+ tree addr_high = gimple_build (seq, POINTER_PLUS_EXPR,
+ TREE_TYPE (addr_low), addr_low, four);
+ if (WORDS_BIG_ENDIAN)
+ std::swap (addr_low, addr_high);
+
+ /* Get the low and high 32-bit parts of the counter. Fold to a
+ constant if COUNTER is a compile-time constant, so that a part
+ which is known to be zero can be recognized below. */
+ tree counter_low_32 = gimple_build (seq, NOP_EXPR, uint32_type_node,
+ counter);
+ tree shift_32 = build_int_cst (integer_type_node, 32);
+ tree counter_high_64 = gimple_build (seq, RSHIFT_EXPR, gcov_type_node,
+ counter, shift_32);
+ tree counter_high_32 = gimple_build (seq, NOP_EXPR, uint32_type_node,
+ counter_high_64);
+
+ /* Atomically bitwise-or the low 32-bit counter parts */
+ if (!integer_zerop (counter_low_32))
+ {
+ gcall *call1 = gimple_build_call (atomic_ior_32, 3, addr_low,
+ counter_low_32, relaxed);
+ gimple_seq_add_stmt (seq, call1);
+ }
+
+ /* Atomically bitwise-or the high 32-bit counter parts */
+ if (!integer_zerop (counter_high_32))
+ {
+ gcall *call2 = gimple_build_call (atomic_ior_32, 3, addr_high,
+ counter_high_32, relaxed);
+ gimple_seq_add_stmt (seq, call2);
+ }
+}
+
/* Add instrumentation to a decision subgraph. EXPR should be the
(topologically sorted) block of nodes returned by cov_blocks, MAPS the
bitmaps returned by cov_maps, and MASKS the block of bitsets returned by
@@ -1138,11 +1185,17 @@ instrument_decisions (array_slice<basic_block> expr, size_t condno,
gcc_assert (xi == bitmap_count_bits (core));
const tree relaxed = build_int_cst (integer_type_node, MEMMODEL_RELAXED);
- const bool atomic = flag_profile_update == PROFILE_UPDATE_ATOMIC;
+ const bool use_atomic_builtin
+ = counter_update == COUNTER_UPDATE_ATOMIC_BUILTIN;
+ const bool use_atomic_split
+ = counter_update == COUNTER_UPDATE_ATOMIC_SPLIT
+ || counter_update == COUNTER_UPDATE_ATOMIC_PARTIAL;
+ const tree atomic_ior_32
+ = builtin_decl_explicit (BUILT_IN_ATOMIC_FETCH_OR_4);
const tree atomic_ior
- = builtin_decl_explicit (TYPE_PRECISION (gcov_type_node) > 32
- ? BUILT_IN_ATOMIC_FETCH_OR_8
- : BUILT_IN_ATOMIC_FETCH_OR_4);
+ = TYPE_PRECISION (gcov_type_node) > 32
+ ? builtin_decl_explicit (BUILT_IN_ATOMIC_FETCH_OR_8)
+ : atomic_ior_32;
/* Flush to the gcov accumulators. */
for (const basic_block b : expr)
@@ -1183,7 +1236,7 @@ instrument_decisions (array_slice<basic_block> expr, size_t condno,
continue;
tree ref = tree_coverage_counter_ref (GCOV_COUNTER_CONDS,
2 * condno + k);
- if (atomic)
+ if (use_atomic_builtin)
{
ref = unshare_expr (ref);
gcall *flush = gimple_build_call (atomic_ior, 3,
@@ -1191,6 +1244,9 @@ instrument_decisions (array_slice<basic_block> expr, size_t condno,
next[k], relaxed);
gimple_seq_add_stmt (&seq, flush);
}
+ else if (use_atomic_split)
+ split_update_decision_counter (&seq, ref, next[k],
+ atomic_ior_32, relaxed);
else
{
tree get = emit_assign (&seq, ref);