From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vk1-xa36.google.com (mail-vk1-xa36.google.com [IPv6:2607:f8b0:4864:20::a36]) by sourceware.org (Postfix) with ESMTPS id 1B54B3858C74 for ; Thu, 13 Jul 2023 07:34:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1B54B3858C74 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-vk1-xa36.google.com with SMTP id 71dfb90a1353d-48138949fb4so129251e0c.1 for ; Thu, 13 Jul 2023 00:34:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1689233683; x=1691825683; 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=fU0bF48ec8lJM+j6iTQQjC+83TQxi4Wkkn+ODMEOEH0=; b=PXF9buJ2ZSP7ENjvXLAfIHTtLODCTwz4DNVakz2z6WNQD5ggL7G//aXhqe/XQQRRJM qefXDMek9zwmB3m5sW1qAliak/4sEj5wT00rcMz9wPO1PfSTFAysH2PR6qM1ZLDi7elA uXdNnjodCmy0DXySNbw3i7fjokJxflJETjgMKX80lQN4uNLUA7sEXyWNmBdw8Yczhfkh r2IXAY4vm3tVAQSvXa5ZE7xGLuTZLgUs5yXObiEutqo6ybULF7ejFYwMcpcUri9ZTL8N K3wxB8wDNJavRjSZqDOmvfnyMMOpcDxiz8jQUeFXeXX/7nyVHkqlrDUXPmVP7iPWlFCU dx7Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689233683; x=1691825683; 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=fU0bF48ec8lJM+j6iTQQjC+83TQxi4Wkkn+ODMEOEH0=; b=a/Vex49wYK2oUiUmdat1+zOOJgijpQyoIg24j1+qrA1AFyX+Xs5t3L1gzfX7NI6xxV fVwG3i4DULM+Utshc5Z85tu28JtH/bSsl7IROvhiIKYMs5g65kQH3iTY/SV+I3HBqCKY avt25a8x5u3hvircbSsRskqAlakEeDUxxYQ8MS0VLs/iOq44bQfTSvU3w3MvqMMRq8sq V1itQpJlx0VzJ5J3e8dUKzSAqG5IvBWFJUFBfGrTaTmbqhIJ82rEmGnEbuqGzx/Jw3Ty 2/tavDwXWRxTfnIKUFNz7GRUNYQNfB2qJiSmCLqH07C4vkltWLghW2P8llQV3GYNVOW8 5NkQ== X-Gm-Message-State: ABy/qLbI3fk2hE5fCPkAgAc9s/qzZeoRN7QdQFLGlDGM3/NZgNDVwYP+ wsSSV7I0CKMaruLcfGZePNaeDTtMaU13h2KfQio= X-Google-Smtp-Source: APBJJlET4/1tYVSvpRKW3JAsLSz/oozyAPe5LLMJvdh6hOa7y5VhzYiU9jMF+G9RF7R/HctY6BRgRggsZP0Fyf5mfEY= X-Received: by 2002:a1f:5c8c:0:b0:481:4092:991 with SMTP id q134-20020a1f5c8c000000b0048140920991mr389104vkb.13.1689233683236; Thu, 13 Jul 2023 00:34:43 -0700 (PDT) MIME-Version: 1.0 References: <20230712054609.3958442-1-pan2.li@intel.com> <20230712055053.4016796-1-pan2.li@intel.com> <7da64431-255f-699b-e0a3-761844c08b76@gmail.com> In-Reply-To: From: Kito Cheng Date: Thu, 13 Jul 2023 15:34:31 +0800 Message-ID: Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM To: "Li, Pan2" Cc: Jeff Law , "gcc-patches@gcc.gnu.org" , "juzhe.zhong@rivai.ai" , "rdapp.gcc@gmail.com" , "Wang, Yanzhang" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-8.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,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: oh, I know why you failed on that, you need to put it within the function, not global static, function static variable will construct when first invoked rather than construct at program start. Could you try to apply my diff in the last mail and try again? On Thu, Jul 13, 2023 at 3:29=E2=80=AFPM Li, Pan2 via Gcc-patches wrote: > > Thanks Kito for review. Sorry didn't involve the code result in self test= error in PATCH v3, but it can be reproduced with below diff based on PATCH= v3. Let me know if I didn't get the point of your comments. > > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc > index 6ed735d6983..76689eaf8d5 100644 > --- a/gcc/config/riscv/riscv.cc > +++ b/gcc/config/riscv/riscv.cc > @@ -233,6 +233,9 @@ static int epilogue_cfa_sp_offset; > /* Which tuning parameters to use. */ > static const struct riscv_tune_param *tune_param; > > +static const_rtx vxrm_rtx =3D gen_rtx_REG (SImode, VXRM_REGNUM); > +static const_rtx frm_rtx =3D gen_rtx_REG (SImode, FRM_REGNUM); > + > /* Which automaton to use for tuning. */ > enum riscv_microarchitecture_type riscv_microarchitecture; > > @@ -7717,7 +7720,7 @@ static bool > vxrm_unknown_p (rtx_insn *insn) > { > /* Return true if there is a definition of VXRM. */ > - if (reg_set_p (gen_rtx_REG (SImode, VXRM_REGNUM), insn)) > + if (reg_set_p (vxrm_rtx, insn)) > return true; > > /* A CALL function may contain an instruction that modifies the VXRM, > @@ -7739,7 +7742,7 @@ static bool > frm_unknown_dynamic_p (rtx_insn *insn) > { > /* Return true if there is a definition of FRM. */ > - if (reg_set_p (gen_rtx_REG (SImode, FRM_REGNUM), insn)) > + if (reg_set_p (frm_rtx, insn)) > return true; > > /* A CALL function may contain an instruction that modifies the FRM, > @@ -7761,7 +7764,7 @@ riscv_vxrm_mode_after (rtx_insn *insn, int mode) > if (recog_memoized (insn) < 0) > return mode; > > - if (reg_mentioned_p (gen_rtx_REG (SImode, VXRM_REGNUM), PATTERN (insn)= )) > + if (reg_mentioned_p (vxrm_rtx, PATTERN (insn))) > return get_attr_vxrm_mode (insn); > else > return mode; > @@ -7778,7 +7781,7 @@ riscv_frm_mode_after (rtx_insn *insn, int mode) > if (recog_memoized (insn) < 0) > return mode; > > - if (reg_mentioned_p (gen_rtx_REG (SImode, FRM_REGNUM), PATTERN (insn))= ) > + if (reg_mentioned_p (frm_rtx, PATTERN (insn))) > return get_attr_frm_mode (insn); > else > return mode; > > Pan > > -----Original Message----- > From: Kito Cheng > Sent: Thursday, July 13, 2023 2:19 PM > To: Li, Pan2 > Cc: Jeff Law ; gcc-patches@gcc.gnu.org; juzhe.zhon= g@rivai.ai; rdapp.gcc@gmail.com; Wang, Yanzhang > Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FR= M > > Hmmm? I didn't get that error on selftest? > > my diff with your v2: > > $ git diff > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc > index 12655f7fdc65..466e1aed91c7 100644 > --- a/gcc/config/riscv/riscv.cc > +++ b/gcc/config/riscv/riscv.cc > @@ -8058,8 +8058,9 @@ asm_insn_p (rtx_insn *insn) > static bool > vxrm_unknown_p (rtx_insn *insn) > { > + static const_rtx vxrm_reg =3D gen_rtx_REG (SImode, VXRM_REGNUM); > /* Return true if there is a definition of VXRM. */ > - if (reg_set_p (gen_rtx_REG (SImode, VXRM_REGNUM), insn)) > + if (reg_set_p (vxrm_reg, insn)) > return true; > > /* A CALL function may contain an instruction that modifies the VXRM, > @@ -8080,8 +8081,9 @@ vxrm_unknown_p (rtx_insn *insn) > static bool > frm_unknown_dynamic_p (rtx_insn *insn) > { > + static const_rtx frm_reg =3D gen_rtx_REG (SImode, FRM_REGNUM); > /* Return true if there is a definition of FRM. */ > - if (reg_set_p (gen_rtx_REG (SImode, FRM_REGNUM), insn)) > + if (reg_set_p (frm_reg, insn)) > return true; > > /* A CALL function may contain an instruction that modifies the FRM, > > > On Thu, Jul 13, 2023 at 1:07=E2=80=AFPM Li, Pan2 via Gcc-patches > wrote: > > > > Thanks Jeff and Kito for comments, update the V3 version as below. > > > > https://gcc.gnu.org/pipermail/gcc-patches/2023-July/624347.html > > > > > Extract vxrm reg to a local static variable to prevent construct that= again and again. > > > > The "static const_rtx vxrm_rtx =3D gen_rtx_REG (SImode, VXRM_REGMU)" re= sults in some error when selftest like below, thus patch v3 doesn't include= this change. > > > > /home/pli/repos/gcc/111/riscv-gnu-toolchain/build-gcc-newlib-stage1/./g= cc/xgcc -B/home/pli/repos/gcc/111/riscv-gnu-toolchain/build-gcc-newlib-stag= e1/./gcc/ -xc -nostdinc /dev/null -S -o /dev/null -fself-test=3D../.././gc= c/gcc/testsuite/selftests > > virtual memory exhausted: Invalid argument > > make[2]: *** [../.././gcc/gcc/c/Make-lang.in:153: s-selftest-c] Error 1 > > > > Pan > > > > -----Original Message----- > > From: Jeff Law > > Sent: Wednesday, July 12, 2023 11:31 PM > > To: Li, Pan2 ; gcc-patches@gcc.gnu.org > > Cc: juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; Wang, Yanzhang ; kito.cheng@gmail.com > > Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and = FRM > > > > > > > > On 7/11/23 23:50, pan2.li@intel.com wrote: > > > From: Pan Li > > > > > > When investigate the FRM dynmaic rounding mode, we find the global > > > unknown status is quite different between the fixed-point and > > > floating-point. Thus, we separate the unknown function with extractin= g > > > some inner common functions. > > > > > > We will also prepare more test cases in another PATCH. > > > > > > Signed-off-by: Pan Li > > > > > > gcc/ChangeLog: > > > > > > * config/riscv/riscv.cc (regnum_definition_p): New function. > > > (insn_asm_p): Ditto. > > > (riscv_vxrm_mode_after): New function for fixed-point. > > > (global_vxrm_state_unknown_p): Ditto. > > > (riscv_frm_mode_after): New function for floating-point. > > > (global_frm_state_unknown_p): Ditto. > > > (riscv_mode_after): Leverage new functions. > > > (riscv_entity_mode_after): Removed. > > > --- > > > gcc/config/riscv/riscv.cc | 96 +++++++++++++++++++++++++++++++++---= --- > > > 1 file changed, 82 insertions(+), 14 deletions(-) > > > > > > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc > > > index 38d8eb2fcf5..553fbb4435a 100644 > > > --- a/gcc/config/riscv/riscv.cc > > > +++ b/gcc/config/riscv/riscv.cc > > > @@ -7742,19 +7742,91 @@ global_state_unknown_p (rtx_insn *insn, unsig= ned int regno) > > > return false; > > > } > > > > > > +static bool > > > +regnum_definition_p (rtx_insn *insn, unsigned int regno) > > Needs a function comment. This is true for each new function added. I= n > > this specific case somethign like this might be appropriate > > > > /* Return TRUE if REGNO is set in INSN, FALSE otherwise. */ > > > > Which begs the question, is there some reason why we're not using the > > existing reg_set_p or simple_regno_set from rtlanal.cc? > > > > > > > > Jeff