From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-x42e.google.com (mail-pf1-x42e.google.com [IPv6:2607:f8b0:4864:20::42e]) by sourceware.org (Postfix) with ESMTPS id BBCD33858CDB for ; Fri, 10 Nov 2023 15:05:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org BBCD33858CDB Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=vrull.eu Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=vrull.eu ARC-Filter: OpenARC Filter v1.0.0 sourceware.org BBCD33858CDB Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::42e ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699628752; cv=none; b=SAoJCydUbCWtX+zkxqO2RxdiEbBytjX12jsGttkk//AyT3YhcArN+AF0NiSwQOUByLquwENQohT2w8FAH9y00z18VwEFsxIuJMYDyqWpjYoSyjtWrZIh44OPTzXLrUXBSdvmBHhItnJUFcVlhuzeoGUCKvp+M0cgoW7YO01MjHg= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699628752; c=relaxed/simple; bh=FGhXpffkcMerKqcNgHpluRNbZjPUKQ2D2l9Cqp2pS04=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=AfZxHWxr9ySKOK3slXl9rq0JMf3ObEyN6pRgAi8FtjZVpAiNBiWKBPlg9aLTlfpZMLaMOzdtGdMMuZgtKvjzqEX6uknC6TeLW7VMlLVXNnEjUee+cu4/vWnK54HD5stcUvV6eQiSsjsG+IB+Gf954NUTA1Z21aq81wPRcQLBeEI= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pf1-x42e.google.com with SMTP id d2e1a72fcca58-692c02adeefso1977295b3a.3 for ; Fri, 10 Nov 2023 07:05:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=vrull.eu; s=google; t=1699628748; x=1700233548; darn=gcc.gnu.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=88KLc8awJ/UeIIq+yKUXZyT6y+mFxqf2UQQ0eJwsKsI=; b=KVQeT5c0+PgJIy2U5TIx+V+L9ZQJi9Ehx5lR29Q6STnyINDs2ug52/Xhru58z7soYP C4mdILCiVwHZxihCaVO33hhpBq1xIBq8FIMK/6ZKZMUhOumzra/0HwORqTNRrnTEQ/b2 ddeg5IMz5S00aOK7KuSwESAOqCiGbNHOjtvYy8iNnt8gE3dsJPfwq5fxcIfLUO+REA55 5BEdvM5MDRwF89no4XlLsQCWIYbJW5nUstL0AFTgh9Ity+qc831gK5/tSFOq2w6/txpu e0exL3/10HoHqOH8GFfYmPJFkaQjOS10m6EYtN4DQg2O5i+cJV/+R5kFh2lwN+Ii9BNg gVYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699628748; x=1700233548; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=88KLc8awJ/UeIIq+yKUXZyT6y+mFxqf2UQQ0eJwsKsI=; b=tx/l0UJybQfahi4jTwVjw6qORP3o3yMOIs+gOnfDKuGhvD87IGAC7L183XC//vplMv jBFDWbWgUqkCRiiDToSM+2xEIoFpETHmI0cEkaFSY/ZX4s8jAAYXc7pl1vhgpYoQiDEA xluhuVse9f1UYI4VGg26lhEyUIQA16PJ292ToCyJs/rUrUJt+KM8UINDnQ6FtuTO730d atQWcRUCArJ6zbhCS1FDrfgI9gjwlbYAMZjAYW8paIG4jgW6V3h51HhoNX8Qb069D8cF I7KMoOeSwVorXGco7Swryv+EflXocSde5Aek+xEg8pROhYxwK7A5hL6z+4JddOOcIK76 OK8Q== X-Gm-Message-State: AOJu0YwQfaVtFMuF9MQ1x5N+KKr7Y/q6Jpu8iJCZkouDs6YiwKeFhMCa zm0CFmcsewf9zLHBmv5yYkucJ2gKxKJggLiLnHtre3vBsXIwarvi7/0= X-Google-Smtp-Source: AGHT+IFv0e8i3TbKQBpkHnd/4lFK8guTH1o3X9YCjSnTxlyZlxLn61WTgpws/LrDKC9KmqUv50kAK0/c2TwpbL50258= X-Received: by 2002:a17:90a:4888:b0:280:1df1:cbc7 with SMTP id b8-20020a17090a488800b002801df1cbc7mr5088929pjh.19.1699628748273; Fri, 10 Nov 2023 07:05:48 -0800 (PST) MIME-Version: 1.0 References: <20221113214636.2747737-8-christoph.muellner@vrull.eu> <20231107030415.1105-1-jinma@linux.alibaba.com> In-Reply-To: <20231107030415.1105-1-jinma@linux.alibaba.com> From: =?UTF-8?Q?Christoph_M=C3=BCllner?= Date: Fri, 10 Nov 2023 16:05:36 +0100 Message-ID: Subject: Re: [PATCH] riscv: thead: Add support for the XTheadInt ISA extension To: Jin Ma Cc: gcc-patches@gcc.gnu.org, jinma.contrib@gmail.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-9.7 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,JMQ_SPF_NEUTRAL,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Tue, Nov 7, 2023 at 4:04=E2=80=AFAM Jin Ma wro= te: > > The XTheadInt ISA extension provides acceleration interruption > instructions as defined in T-Head-specific: > > * th.ipush > * th.ipop Overall, it looks ok to me. There are just a few small issues to clean up (see below). > > gcc/ChangeLog: > > * config/riscv/riscv-protos.h (th_int_get_mask): New prototype. > (th_int_get_save_adjustment): Likewise. > (th_int_adjust_cfi_prologue): Likewise. > * config/riscv/riscv.cc (TH_INT_INTERRUPT): New macro. > (riscv_expand_prologue): Add the processing of XTheadInt. > (riscv_expand_epilogue): Likewise. > * config/riscv/riscv.md: New unspec. > * config/riscv/thead.cc (BITSET_P): New macro. > * config/riscv/thead.md (th_int_push): New pattern. > (th_int_pop): New pattern. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/xtheadint-push-pop.c: New test. > --- > gcc/config/riscv/riscv-protos.h | 3 + > gcc/config/riscv/riscv.cc | 58 +++++++++++++- > gcc/config/riscv/riscv.md | 4 + > gcc/config/riscv/thead.cc | 78 +++++++++++++++++++ > gcc/config/riscv/thead.md | 67 ++++++++++++++++ > .../gcc.target/riscv/xtheadint-push-pop.c | 36 +++++++++ > 6 files changed, 245 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadint-push-pop.c > > diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-pro= tos.h > index 85d4f6ed9ea..05d1fc2b3a0 100644 > --- a/gcc/config/riscv/riscv-protos.h > +++ b/gcc/config/riscv/riscv-protos.h > @@ -627,6 +627,9 @@ extern void th_mempair_prepare_save_restore_operands = (rtx[4], bool, > int, HOST_WIDE_INT, > int, HOST_WIDE_INT)= ; > extern void th_mempair_save_restore_regs (rtx[4], bool, machine_mode); > +extern unsigned int th_int_get_mask(unsigned int); Space between function name and parenthesis. > +extern unsigned int th_int_get_save_adjustment(); Space between function name and parenthesis. An empty parameter list should be written as "(void)". > +extern rtx th_int_adjust_cfi_prologue (unsigned int); > #ifdef RTX_CODE > extern const char* > th_mempair_output_move (rtx[4], bool, machine_mode, RTX_CODE); > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc > index 08ff05dcc3f..c623101b05e 100644 > --- a/gcc/config/riscv/riscv.cc > +++ b/gcc/config/riscv/riscv.cc > @@ -101,6 +101,16 @@ along with GCC; see the file COPYING3. If not see > /* True the mode switching has static frm, or false. */ > #define STATIC_FRM_P(c) ((c)->machine->mode_sw_info.static_frm_p) > > +/* True if we can use the instructions in the XTheadInt extension > + to handle interrupts, or false. */ > +#define TH_INT_INTERRUPT(c) \ > + (TARGET_XTHEADINT \ > + /* The XTheadInt extension only supports rv32. */ \ > + && !TARGET_64BIT \ > + && (c)->machine->interrupt_handler_p \ > + /* This instruction can be executed in M-mode only.*/ \ Dot, space, space, end of comment. Maybe better: /* The XTheadInt instructions can only be executed in M-mode. */ > + && (c)->machine->interrupt_mode =3D=3D MACHINE_MODE) > + > /* Information about a function's frame layout. */ > struct GTY(()) riscv_frame_info { > /* The size of the frame in bytes. */ > @@ -6703,6 +6713,7 @@ riscv_expand_prologue (void) > unsigned fmask =3D frame->fmask; > int spimm, multi_push_additional, stack_adj; > rtx insn, dwarf =3D NULL_RTX; > + unsigned th_int_mask =3D 0; > > if (flag_stack_usage_info) > current_function_static_stack_size =3D constant_lower_bound (remaini= ng_size); > @@ -6771,6 +6782,28 @@ riscv_expand_prologue (void) > REG_NOTES (insn) =3D dwarf; > } > > + th_int_mask =3D th_int_get_mask(frame->mask); There should be exactly one space between function name and parenthesis. > + if (th_int_mask && TH_INT_INTERRUPT (cfun)) > + { > + frame->mask &=3D ~th_int_mask; > + > + /* RISCV_PROLOGUE_TEMP may be used to handle some CSR for > + interrupts, such as fcsr. */ Dot, space, space, end of comment. > + if ((TARGET_HARD_FLOAT && frame->fmask) > + || (TARGET_ZFINX && frame->mask)) > + frame->mask |=3D (1 << RISCV_PROLOGUE_TEMP_REGNUM); > + > + unsigned save_adjustment =3D th_int_get_save_adjustment (); > + frame->gp_sp_offset -=3D save_adjustment; > + remaining_size -=3D save_adjustment; > + > + insn =3D emit_insn (gen_th_int_push ()); > + > + rtx dwarf =3D th_int_adjust_cfi_prologue (th_int_mask); > + RTX_FRAME_RELATED_P (insn) =3D 1; > + REG_NOTES (insn) =3D dwarf; > + } > + > /* Save the GP, FP registers. */ > if ((frame->mask | frame->fmask) !=3D 0) > { > @@ -6999,6 +7032,7 @@ riscv_expand_epilogue (int style) > =3D use_multi_pop ? frame->multi_push_adj_base + frame->multi_push_a= dj_addi > : 0; > rtx ra =3D gen_rtx_REG (Pmode, RETURN_ADDR_REGNUM); > + unsigned th_int_mask =3D 0; > rtx insn; > > /* We need to add memory barrier to prevent read from deallocated stac= k. */ > @@ -7161,12 +7195,32 @@ riscv_expand_epilogue (int style) > else if (use_restore_libcall) > frame->mask =3D 0; /* Temporarily fib that we need not restore GPRs.= */ > > + th_int_mask =3D th_int_get_mask(frame->mask); There should be exactly one space between function name and parenthesis. > + if (th_int_mask && TH_INT_INTERRUPT (cfun)) > + { > + frame->mask &=3D ~th_int_mask; > + > + /* RISCV_PROLOGUE_TEMP may be used to handle some CSR for > + interrupts, such as fcsr. */ Dot, space, space, end of comment. > + if ((TARGET_HARD_FLOAT && frame->fmask) > + || (TARGET_ZFINX && frame->mask)) > + frame->mask |=3D (1 << RISCV_PROLOGUE_TEMP_REGNUM); > + } > + > /* Restore the registers. */ > riscv_for_each_saved_v_reg (step2, riscv_restore_reg, false); > riscv_for_each_saved_reg (frame->total_size - step2 - libcall_size > - multipop_size, > riscv_restore_reg, true, style =3D=3D EXCEPTI= ON_RETURN); > > + if (th_int_mask && TH_INT_INTERRUPT (cfun)) > + { > + frame->mask =3D mask; /* Undo the above fib. */ > + unsigned save_adjustment =3D th_int_get_save_adjustment ();; Double semi-colon. > + gcc_assert (step2.to_constant () >=3D save_adjustment); > + step2 -=3D save_adjustment; > + } > + > if (use_restore_libcall) > frame->mask =3D mask; /* Undo the above fib. */ > > @@ -7229,7 +7283,9 @@ riscv_expand_epilogue (int style) > > gcc_assert (mode !=3D UNKNOWN_MODE); > > - if (mode =3D=3D MACHINE_MODE) > + if (th_int_mask && TH_INT_INTERRUPT (cfun)) > + emit_jump_insn (gen_th_int_pop ()); > + else if (mode =3D=3D MACHINE_MODE) > emit_jump_insn (gen_riscv_mret ()); > else if (mode =3D=3D SUPERVISOR_MODE) > emit_jump_insn (gen_riscv_sret ()); > diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md > index ae2217d0907..d4147d08838 100644 > --- a/gcc/config/riscv/riscv.md > +++ b/gcc/config/riscv/riscv.md > @@ -126,6 +126,10 @@ (define_c_enum "unspecv" [ > ;; XTheadFmv unspec > UNSPEC_XTHEADFMV > UNSPEC_XTHEADFMV_HW > + > + ;; XTheadInt unspec > + UNSPECV_XTHEADINT_PUSH > + UNSPECV_XTHEADINT_POP > ]) > > (define_constants > diff --git a/gcc/config/riscv/thead.cc b/gcc/config/riscv/thead.cc > index a485fb1fba6..5b437c9a7a2 100644 > --- a/gcc/config/riscv/thead.cc > +++ b/gcc/config/riscv/thead.cc > @@ -36,6 +36,9 @@ > #include "regs.h" > #include "riscv-protos.h" > > +/* True if bit BIT is set in VALUE. */ > +#define BITSET_P(VALUE, BIT) (((VALUE) & (1ULL << (BIT))) !=3D 0) > + This is copied from riscv.cc. Let's better move it to riscv.h instead of duplicating it. > /* If MEM is in the form of "base+offset", extract the two parts > of address and set to BASE and OFFSET, otherwise return false > after clearing BASE and OFFSET. */ > @@ -945,3 +948,78 @@ th_print_operand_address (FILE *file, machine_mode m= ode, rtx x) > > gcc_unreachable (); > } > + > +/* Number array of registers X1, X5-X7, X10-X17, X28-X31, to be > + operated on by instruction th.ipush/th.ipop in XTheadInt. */ > + > +int th_int_regs[] =3D{ > + RETURN_ADDR_REGNUM, > + T0_REGNUM, T1_REGNUM, T2_REGNUM, > + A0_REGNUM, A1_REGNUM, A2_REGNUM, A3_REGNUM, > + A4_REGNUM, A5_REGNUM, A6_REGNUM, A7_REGNUM, > + T3_REGNUM, T4_REGNUM, T5_REGNUM, T6_REGNUM, > +}; > + > +/* If MASK contains registers X1, X5-X7, X10-X17, X28-X31, then > + return the mask composed of these registers, otherwise return > + zero. */ > + > +unsigned int > +th_int_get_mask(unsigned int mask) There should be exactly one space between function name and parenthesis. > +{ > + unsigned int xtheadint_mask =3D 0; > + > + if (!TARGET_XTHEADINT || TARGET_64BIT) > + return 0; > + > + for (unsigned int i =3D 0; i < ARRAY_SIZE (th_int_regs); i++) > + { > + if (!BITSET_P (mask, th_int_regs[i])) > + return 0; > + > + xtheadint_mask |=3D (1 << th_int_regs[i]); > + } > + > + return xtheadint_mask; /* Usually 0xf003fce2. */ > +} > + > +/* Returns the occupied frame needed to save registers X1, X5-X7, > + X10-X17, X28-X31.*/ Dot, space, space, end of comment. > + > +unsigned int > +th_int_get_save_adjustment () "()" -> "(void)" > +{ > + gcc_assert (TARGET_XTHEADINT && !TARGET_64BIT); > + return ARRAY_SIZE (th_int_regs) * UNITS_PER_WORD; > +} > + > +rtx > +th_int_adjust_cfi_prologue (unsigned int mask) > +{ > + gcc_assert (TARGET_XTHEADINT && !TARGET_64BIT); > + > + rtx dwarf =3D NULL_RTX; > + rtx adjust_sp_rtx, reg, mem, insn; > + int saved_size =3D ARRAY_SIZE (th_int_regs) * UNITS_PER_WORD; > + int offset =3D saved_size; > + > + for (int regno =3D GP_REG_FIRST; regno <=3D GP_REG_LAST; regno++) > + if (BITSET_P (mask, regno - GP_REG_FIRST)) > + { > + offset -=3D UNITS_PER_WORD; > + reg =3D gen_rtx_REG (SImode, regno); > + mem =3D gen_frame_mem (SImode, plus_constant (Pmode, > + stack_pointer_rtx, > + offset)); > + > + insn =3D gen_rtx_SET (mem, reg); > + dwarf =3D alloc_reg_note (REG_CFA_OFFSET, insn, dwarf); > + } > + > + /* Debug info for adjust sp. */ > + adjust_sp_rtx =3D gen_rtx_SET (stack_pointer_rtx, > + gen_rtx_PLUS (GET_MODE(stack_pointer_rtx), stack_pointer_rtx, GEN= _INT (-saved_size))); There should be exactly one space between function name and parenthesis. > + dwarf =3D alloc_reg_note (REG_CFA_ADJUST_CFA, adjust_sp_rtx, dwarf); > + > + return dwarf; > +} > diff --git a/gcc/config/riscv/thead.md b/gcc/config/riscv/thead.md > index 2babfafb23c..4d6e16c0edc 100644 > --- a/gcc/config/riscv/thead.md > +++ b/gcc/config/riscv/thead.md > @@ -210,6 +210,73 @@ (define_insn "th_fmv_x_hw" > (set_attr "type" "fmove") > (set_attr "mode" "DF")]) > > +;; XTheadInt > + > +(define_constants > + [(T0_REGNUM 5) > + (T1_REGNUM 6) > + (T2_REGNUM 7) > + (A0_REGNUM 10) > + (A1_REGNUM 11) > + (A2_REGNUM 12) > + (A3_REGNUM 13) > + (A4_REGNUM 14) > + (A5_REGNUM 15) > + (A6_REGNUM 16) > + (A7_REGNUM 17) > + (T3_REGNUM 28) > + (T4_REGNUM 29) > + (T5_REGNUM 30) > + (T6_REGNUM 31) > +]) > + > +(define_insn "th_int_push" > + [(unspec_volatile [(const_int 0)] UNSPECV_XTHEADINT_PUSH) > + (use (reg:SI RETURN_ADDR_REGNUM)) > + (use (reg:SI T0_REGNUM)) > + (use (reg:SI T1_REGNUM)) > + (use (reg:SI T2_REGNUM)) > + (use (reg:SI A0_REGNUM)) > + (use (reg:SI A1_REGNUM)) > + (use (reg:SI A2_REGNUM)) > + (use (reg:SI A3_REGNUM)) > + (use (reg:SI A4_REGNUM)) > + (use (reg:SI A5_REGNUM)) > + (use (reg:SI A6_REGNUM)) > + (use (reg:SI A7_REGNUM)) > + (use (reg:SI T3_REGNUM)) > + (use (reg:SI T4_REGNUM)) > + (use (reg:SI T5_REGNUM)) > + (use (reg:SI T6_REGNUM))] > + "TARGET_XTHEADINT && !TARGET_64BIT" > + "th.ipush" > + [(set_attr "type" "store") > + (set_attr "mode" "SI")]) > + > +(define_insn "th_int_pop" > + [(unspec_volatile [(const_int 0)] UNSPECV_XTHEADINT_POP) > + (clobber (reg:SI RETURN_ADDR_REGNUM)) > + (clobber (reg:SI T0_REGNUM)) > + (clobber (reg:SI T1_REGNUM)) > + (clobber (reg:SI T2_REGNUM)) > + (clobber (reg:SI A0_REGNUM)) > + (clobber (reg:SI A1_REGNUM)) > + (clobber (reg:SI A2_REGNUM)) > + (clobber (reg:SI A3_REGNUM)) > + (clobber (reg:SI A4_REGNUM)) > + (clobber (reg:SI A5_REGNUM)) > + (clobber (reg:SI A6_REGNUM)) > + (clobber (reg:SI A7_REGNUM)) > + (clobber (reg:SI T3_REGNUM)) > + (clobber (reg:SI T4_REGNUM)) > + (clobber (reg:SI T5_REGNUM)) > + (clobber (reg:SI T6_REGNUM)) > + (return)] > + "TARGET_XTHEADINT && !TARGET_64BIT" > + "th.ipop" > + [(set_attr "type" "ret") > + (set_attr "mode" "SI")]) > + > ;; XTheadMac > > (define_insn "*th_mula" > diff --git a/gcc/testsuite/gcc.target/riscv/xtheadint-push-pop.c b/gcc/te= stsuite/gcc.target/riscv/xtheadint-push-pop.c > new file mode 100644 > index 00000000000..3383431aa30 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/xtheadint-push-pop.c > @@ -0,0 +1,36 @@ > +/* { dg-do compile } */ > +/* { dg-options "-march=3Drv32gc_xtheadint -mabi=3Dilp32d" { target { rv= 32 } } } */ > +/* { dg-options "-march=3Drv64gc_xtheadint -mabi=3Dlp64d" { target { rv6= 4 } } } */ > + > +extern void f(void); > + > +__attribute__((interrupt)) > +void func_default(void) There should be exactly one space between function name and parenthesis. > +{ > + f (); > +} > + > +__attribute__ ((interrupt ("machine"))) > +void func_machine(void) There should be exactly one space between function name and parenthesis. > +{ > + f (); > +} > + > +/* { dg-final { scan-assembler-times {\tth\.ipush\n} 2 { target { rv32 }= } } } */ > +/* { dg-final { scan-assembler-times {\tth\.ipop\n} 2 { target { rv32 } = } } } */ > + > + > +__attribute__ ((interrupt ("user"))) > +void func_usr(void) > +{ > + f (); > +} > + > +__attribute__ ((interrupt ("supervisor"))) > +void func_supervisor(void) > +{ > + f (); > +} > + > +/* { dg-final { scan-assembler-not {\tth\.ipush\n} { target { rv64 } } }= } */ > +/* { dg-final { scan-assembler-not {\tth\.ipop\n} { target { rv64 } } } = } */ > \ No newline at end of file > > base-commit: 2cca6ae615f3fb083d3a1e5e9dffcefd54fed990 > -- > 2.17.1 >