Hello Jeff,
thanks for your comment.
----- Am 10. Jul 2025 um 16:01 schrieb Jeff Law jeffreya...@gmail.com:
> On 7/9/25 11:53 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 into two parts.
>>
>> 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
>>
>> the code generation to 64-bit vs. 32-bit RISC-V looks like:
>>
>> addi a5,a5,%lo(.LANCHOR0)
>> beq a0,zero,.L2
>> li a4,1
>> - amoor.d zero,a4,0(a5)
>> - addi a5,a5,8
>> - amoor.d zero,zero,0(a5)
>> + amoor.w zero,a4,0(a5)
>> + addi a4,a5,4
>> + amoor.w zero,zero,0(a4)
>> + addi a4,a5,8
>> + amoor.w zero,zero,0(a4)
>> + addi a5,a5,12
>> + amoor.w zero,zero,0(a5)
>> tail a
>> .L2:
>> - amoor.d zero,zero,0(a5)
>> + amoor.w zero,zero,0(a5)
>> + addi a4,a5,4
>> + amoor.w zero,zero,0(a4)
>> li a4,1
>> - addi a5,a5,8
>> - amoor.d zero,a4,0(a5)
>> + addi a3,a5,8
>> + amoor.w zero,a4,0(a3)
>> + addi a5,a5,12
>> + amoor.w zero,zero,0(a5)
>> tail b
>>
>> Not related to this patch, even with -O2 the compiler generates
>> no-operations like
>>
>> amoor.d zero,zero,0(a5)
>>
>> and
>>
>> amoor.w zero,zero,0(a5)
>>
>> Would this be possible to filter out in instrument_decisions()?
> Just to touch on the last issue. We don't generally try that hard to
> optimize atomics, so I'm not terribly surprised to see that kind of dumb
> code.
In instrument_decisions() we have
/* _true &= ~mask, _false &= ~mask */
counters next;
next[2] = emit_bitwise_op (e, prev[2], BIT_NOT_EXPR);
next[0] = emit_bitwise_op (e, prev[0], BIT_AND_EXPR, next[2]);
next[1] = emit_bitwise_op (e, prev[1], BIT_AND_EXPR, next[2]);
/* _global_true |= _true, _global_false |= _false */
for (size_t k = 0; k != 2; ++k)
{
tree ref = tree_coverage_counter_ref (GCOV_COUNTER_CONDS,
2*condno + k);
...
else if (use_atomic_split)
{
split_update_decision_counter (e, ref, next[k],
atomic_ior_32, relaxed);
}
...
}
Somehow GCC knows that the next[k] is a compile-time constant (results in "li
a4, 1"). I don't have enough GCC knowledge, the figure out the compile-time
value of next[k]. Based on the compile-time value, we could reduce the number
of 32-bit atomic operations.
Also for the 64-bit case, the
addi a5,a5,8
amoor.d zero,zero,0(a5)
could be left out, if we could check the compile-time value of next[k].
>
> If you've got a testcase for the nop-atomics, definitely get it filed as
> a bug. My worry with those cases is we may not have the semantics
> exposed in RTL to allow for optimization, though it's more likely we
> have them in gimple.
I think these nop-atomics are specific to this coverage instrumentation case.
--
embedded brains GmbH & Co. KG
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.hu...@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/
