On Thu, 27 Oct 2022 16:05:19 PDT (-0700), gcc-patches@gcc.gnu.org wrote: > > On 10/27/22 06:49, Xiongchuan Tan via Gcc-patches wrote: >> libitm/ChangeLog: >> >> * configure.tgt: Add riscv support. >> * config/riscv/asm.h: New file. >> * config/riscv/sjlj.S: New file. >> * config/riscv/target.h: New file. >> --- >> v2: Change HW_CACHELINE_SIZE to 64 (in accordance with the RVA profiles, see >> https://github.com/riscv/riscv-profiles/blob/main/profiles.adoc) >> >> libitm/config/riscv/asm.h | 52 +++++++++++++ >> libitm/config/riscv/sjlj.S | 144 +++++++++++++++++++++++++++++++++++ >> libitm/config/riscv/target.h | 50 ++++++++++++ >> libitm/configure.tgt | 2 + >> 4 files changed, 248 insertions(+) >> create mode 100644 libitm/config/riscv/asm.h >> create mode 100644 libitm/config/riscv/sjlj.S >> create mode 100644 libitm/config/riscv/target.h > > Not objecting or even reviewing.... But hasn't transactional memory > largely fallen out of favor these days? Intel has pulled it and I think > IBM did as well.   Should we be investing in extending libitm at all?
I think we didn't get the memo: https://github.com/riscv/riscv-isa-manual/pull/906 The code looks fine to me, so Reviewed-by: Palmer Dabbelt <pal...@rivosinc.com> Acked-by: Palmer Dabbelt <pal...@rivosinc.com> though I don't have an opinion on whether libitm should be taking ports to new targets, I'd never even heard of it before. Some minor comments: > +_ITM_beginTransaction: > + cfi_startproc > + mv a1, sp > + addi sp, sp, -(14*SZ_GPR+12*SZ_FPR) > + cfi_adjust_cfa_offset(14*SZ_GPR+12*SZ_FPR) Many of the ABIs require 16-byte stack alignment. Also: it doesn't hurt anything to use the extra stack, but we only stricly need space for the FPRs if we're going to bother saving them. > +/* ??? The size of one line in hardware caches (in bytes). */ > +#define HW_CACHELINE_SIZE 64 Maybe we should have a placeholder libc/vdso routine for the cache line size? The specs are sort of just a suggestion for that sort of thing. > +static inline void > +cpu_relax (void) > +{ > + __asm__ volatile ("" : : : "memory"); > +} We have Zihintpause now, but that's a pretty minor optimization.
