* [PATCH] riscv/RTEMS: Add RISCV_GCOV_TYPE_SIZE @ 2022-10-26 7:49 Sebastian Huber 2022-10-27 22:56 ` Jeff Law 0 siblings, 1 reply; 5+ messages in thread From: Sebastian Huber @ 2022-10-26 7:49 UTC (permalink / raw) To: gcc-patches 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(+) diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc index 4e18a43539a..1b7f4fb1981 100644 --- a/gcc/config/riscv/riscv.cc +++ b/gcc/config/riscv/riscv.cc @@ -6637,6 +6637,17 @@ riscv_vector_alignment (const_tree type) #undef TARGET_VECTOR_ALIGNMENT #define TARGET_VECTOR_ALIGNMENT riscv_vector_alignment +#ifdef RISCV_GCOV_TYPE_SIZE +static HOST_WIDE_INT +riscv_gcov_type_size (void) +{ + return RISCV_GCOV_TYPE_SIZE; +} + +#undef TARGET_GCOV_TYPE_SIZE +#define TARGET_GCOV_TYPE_SIZE riscv_gcov_type_size +#endif + struct gcc_target targetm = TARGET_INITIALIZER; #include "gt-riscv.h" diff --git a/gcc/config/riscv/rtems.h b/gcc/config/riscv/rtems.h index 14e5e59caaa..3982b24382f 100644 --- a/gcc/config/riscv/rtems.h +++ b/gcc/config/riscv/rtems.h @@ -29,3 +29,5 @@ builtin_define ("__USE_INIT_FINI__"); \ builtin_assert ("system=rtems"); \ } while (0) + +#define RISCV_GCOV_TYPE_SIZE (TARGET_64BIT ? 64 : 32) -- 2.35.3 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] riscv/RTEMS: Add RISCV_GCOV_TYPE_SIZE 2022-10-26 7:49 [PATCH] riscv/RTEMS: Add RISCV_GCOV_TYPE_SIZE Sebastian Huber @ 2022-10-27 22:56 ` Jeff Law 2022-10-27 23:05 ` Palmer Dabbelt 0 siblings, 1 reply; 5+ messages in thread From: Jeff Law @ 2022-10-27 22:56 UTC (permalink / raw) To: Sebastian Huber, gcc-patches 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] riscv/RTEMS: Add RISCV_GCOV_TYPE_SIZE 2022-10-27 22:56 ` Jeff Law @ 2022-10-27 23:05 ` Palmer Dabbelt 2022-10-28 5:47 ` Sebastian Huber 0 siblings, 1 reply; 5+ messages in thread From: Palmer Dabbelt @ 2022-10-27 23:05 UTC (permalink / raw) To: gcc-patches; +Cc: sebastian.huber, gcc-patches 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. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] riscv/RTEMS: Add RISCV_GCOV_TYPE_SIZE 2022-10-27 23:05 ` Palmer Dabbelt @ 2022-10-28 5:47 ` Sebastian Huber 2022-11-01 15:52 ` Jeff Law 0 siblings, 1 reply; 5+ messages in thread From: Sebastian Huber @ 2022-10-28 5:47 UTC (permalink / raw) To: Palmer Dabbelt, gcc-patches, Jeff Law 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/ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] riscv/RTEMS: Add RISCV_GCOV_TYPE_SIZE 2022-10-28 5:47 ` Sebastian Huber @ 2022-11-01 15:52 ` Jeff Law 0 siblings, 0 replies; 5+ messages in thread From: Jeff Law @ 2022-11-01 15:52 UTC (permalink / raw) To: Sebastian Huber, Palmer Dabbelt, gcc-patches 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-11-01 15:52 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-10-26 7:49 [PATCH] riscv/RTEMS: Add RISCV_GCOV_TYPE_SIZE Sebastian Huber 2022-10-27 22:56 ` Jeff Law 2022-10-27 23:05 ` Palmer Dabbelt 2022-10-28 5:47 ` Sebastian Huber 2022-11-01 15:52 ` Jeff Law
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).