Re: [PATCH] riscv/RTEMS: Add RISCV_GCOV_TYPE_SIZE

[Sebastian Huber <sebastian.huber@embedded-brains.de>]

On 28/10/2022 01:05, Palmer Dabbelt wrote:
> On Thu, 27 Oct 2022 15:56:17 PDT (-0700), gcc-patches@gcc.gnu.org wrote:
>>
>> On 10/26/22 01:49, Sebastian Huber wrote:
>>> The RV32A extension does not support 64-bit atomic operations.  For 
>>> RTEMS, use
>>> a 32-bit gcov type for RV32.
>>>
>>> gcc/ChangeLog:
>>>
>>>     * config/riscv/riscv.cc (riscv_gcov_type_size): New.
>>>     (TARGET_GCOV_TYPE_SIZE): Likewise.
>>>     * config/riscv/rtems.h (RISCV_GCOV_TYPE_SIZE): New.
>>
>> Why make this specific to rtems?  ISTM the logic behind this change
>> would apply independently of the os.

Reducing the gcov type to 32-bit has the drawback that the program 
runtime is reduced. I am not sure if this is generally acceptable.

> 
> Looks like rv32gc is just broken here:
> 
> $ cat test.s
> int func(int x) { return x + 1; }
> $ gcc -march=rv32gc -O3 -fprofile-update=atomic -fprofile-arcs test.c -S 
> -o-
> func(int):
>         lui     a4,%hi(__gcov0.func(int))
>         lw      a5,%lo(__gcov0.func(int))(a4)
>         lw      a2,%lo(__gcov0.func(int)+4)(a4)
>         addi    a0,a0,1
>         addi    a3,a5,1
>         sltu    a5,a3,a5
>         add     a5,a5,a2
>         sw      a3,%lo(__gcov0.func(int))(a4)
>         sw      a5,%lo(__gcov0.func(int)+4)(a4)
>         ret
> _sub_I_00100_0:
>         lui     a0,%hi(.LANCHOR0)
>         addi    a0,a0,%lo(.LANCHOR0)
>         tail    __gcov_init
> _sub_D_00100_1:
>         tail    __gcov_exit
> __gcov0.func(int):
>         .zero   8
> 
> Those are not atomic...

Well, you get at least a warning:

test.c:1:1: warning: target does not support atomic profile update, 
single mode is selected

With the patch you get:

riscv-rtems6-gcc -march=rv32gc -O3 -fprofile-update=atomic 
-fprofile-arcs test.c -S -o-
func:
         lui     a5,%hi(__gcov0.func)
         li      a4,1
         addi    a5,a5,%lo(__gcov0.func)
         amoadd.w zero,a4,0(a5)
         addi    a0,a0,1
         ret
         .size   func, .-func

The Armv7-A doesn't have an issue with 64-bit atomics:

arm-rtems6-gcc -march=armv7-a -O3 -fprofile-update=atomic -fprofile-arcs 
test.c -S -o-
func:
         @ args = 0, pretend = 0, frame = 0
         @ frame_needed = 0, uses_anonymous_args = 0
         @ link register save eliminated.
         movw    r3, #:lower16:.LANCHOR0
         movt    r3, #:upper16:.LANCHOR0
         push    {r4, r5, r6, r7}
         mov     r4, #1
         mov     r5, #0
.L2:
         ldrexd  r6, r7, [r3]
         adds    r6, r6, r4
         adc     r7, r7, r5
         strexd  r1, r6, r7, [r3]
         cmp     r1, #0
         bne     .L2
         add     r0, r0, #1
         pop     {r4, r5, r6, r7}
         bx      lr

Maybe RV32 should also support LL/SC instructions with two 32-bit registers.

Another option would be to split the atomic increment into two parts as 
suggested by Jakub Jelinek:

https://patchwork.ozlabs.org/project/gcc/patch/19c4a81d-6ecd-8c6e-b641-e257c1959baf@suse.cz/#1447334

Another option would be to use library calls if hardware atomics are not 
available.

-- 
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/