riscv/RTEMS: Add RISCV_GCOV_TYPE_SIZE

  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.
---
 gcc/config/riscv/riscv.cc | 11 +++++++++++
 gcc/config/riscv/rtems.h  |  2 ++
 2 files changed, 13 insertions(+)
  

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.


jeff
  

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.

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

On rv64 we got some amoadds, which are sane.
  

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.
  

On 10/27/22 23:47, Sebastian Huber wrote:
> 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.

Right, but if you're limited by RV32A, then we're architecturally 
limited to 32bit atomics.  So something has to give.


I'm not objecting to this for rtems.  I'm just noting that if we're 
dealing with an architectural limitation, then the issue is likely to 
show up in other operating systems, so we should at least ponder if we 
want to do an OS specific change or something more general.


Jeff