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 <[email protected]>
---
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
