Message ID | 20230719101156.21771-1-zengxiao@eswincomputing.com |
---|---|
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 186093854178 for <patchwork@sourceware.org>; Wed, 19 Jul 2023 10:12:26 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from zg8tmtyylji0my4xnjqumte4.icoremail.net (zg8tmtyylji0my4xnjqumte4.icoremail.net [162.243.164.118]) by sourceware.org (Postfix) with ESMTP id 2E4D43857BB3 for <gcc-patches@gcc.gnu.org>; Wed, 19 Jul 2023 10:12:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2E4D43857BB3 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=eswincomputing.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=eswincomputing.com Received: from localhost.localdomain (unknown [10.12.130.38]) by app1 (Coremail) with SMTP id EwgMCgDHZMTttrdkrKE0AA--.63411S4; Wed, 19 Jul 2023 18:11:57 +0800 (CST) From: Xiao Zeng <zengxiao@eswincomputing.com> To: gcc-patches@gcc.gnu.org Cc: jeffreyalaw@gmail.com, research_trasio@irq.a4lg.com, kito.cheng@gmail.com, zhengyu@eswincomputing.com, eri-sw-toolchain@eswincomputing.com, Xiao Zeng <zengxiao@eswincomputing.com> Subject: [PATCH 0/5] Recognize Zicond extension Date: Wed, 19 Jul 2023 18:11:51 +0800 Message-Id: <20230719101156.21771-1-zengxiao@eswincomputing.com> X-Mailer: git-send-email 2.17.1 X-CM-TRANSID: EwgMCgDHZMTttrdkrKE0AA--.63411S4 X-Coremail-Antispam: 1UD129KBjvJXoW3XFWrWrWDKF18WFW3tF1fWFg_yoW7Ar47pr 4kCrW5Ary7AF9xJrWrtF1kZF4rAr4Sk3yF934Igrn2v3y7t3y3KF4ktF13Zr15JF98Wr4r ur4q9F1rua4UtFDanT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDU0xBIdaVrnRJUUUkI14x267AKxVW8JVW5JwAFc2x0x2IEx4CE42xK8VAvwI8IcIk0 rVWrJVCq3wAFIxvE14AKwVWUJVWUGwA2ocxC64kIII0Yj41l84x0c7CEw4AK67xGY2AK02 1l84ACjcxK6xIIjxv20xvE14v26w1j6s0DM28EF7xvwVC0I7IYx2IY6xkF7I0E14v26r4U JVWxJr1l84ACjcxK6I8E87Iv67AKxVW0oVCq3wA2z4x0Y4vEx4A2jsIEc7CjxVAFwI0_Gc CE3s1le2I262IYc4CY6c8Ij28IcVAaY2xG8wAqx4xG64xvF2IEw4CE5I8CrVC2j2WlYx0E 2Ix0cI8IcVAFwI0_JrI_JrylYx0Ex4A2jsIE14v26r1j6r4UMcvjeVCFs4IE7xkEbVWUJV W8JwACjcxG0xvY0x0EwIxGrwACjI8F5VA0II8E6IAqYI8I648v4I1lc2xSY4AK6svPMxAI w28IcxkI7VAKI48JMxC20s026xCaFVCjc4AY6r1j6r4UMI8I3I0E5I8CrVAFwI0_Jr0_Jr 4lx2IqxVCjr7xvwVAFwI0_JrI_JrWlx4CE17CEb7AF67AKxVWUAVWUtwCIc40Y0x0EwIxG rwCI42IY6xIIjxv20xvE14v26r1j6r1xMIIF0xvE2Ix0cI8IcVCY1x0267AKxVWUJVW8Jw CI42IY6xAIw20EY4v20xvaj40_Jr0_JF4lIxAIcVC2z280aVAFwI0_Jr0_Gr1lIxAIcVC2 z280aVCY1x0267AKxVW8JVW8JrUvcSsGvfC2KfnxnUUI43ZEXa7VU1a9aPUUUUU== X-CM-SenderInfo: p2hqw5xldrqvxvzl0uprps33xlqjhudrp/ X-Spam-Status: No, score=-5.5 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, KAM_SHORT, RCVD_IN_MSPIKE_H2, 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 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
Recognize Zicond extension
|
|
Message
Xiao Zeng
July 19, 2023, 10:11 a.m. UTC
Hi all RISC-V folks: This series of patches completes support for the riscv architecture's Zicond standard extension instruction set. Currently, Zicond is in a frozen state. See the Zicond specification for details: https://github.com/riscv/riscv-zicond/releases/download/v1.0-rc2/riscv-zicond-v1.0-rc2.pdf Prior to this, other community members have also done related work, as shown in: https://gcc.gnu.org/pipermail/gcc-patches/2023-February/611767.html https://sourceware.org/pipermail/binutils/2023-January/125773.html Xiao Zeng (5): [RISC-V] Recognize Zicond extension [RISC-V] Generate Zicond instruction for basic semantics [RISC-V] Generate Zicond instruction for select pattern with condition eq or neq to 0 [RISC-V] Generate Zicond instruction for select pattern with condition eq or neq to non-zero [RISC-V] Generate Zicond instruction for conditional execution gcc/common/config/riscv/riscv-common.cc | 3 + gcc/config/riscv/riscv-opts.h | 3 + gcc/config/riscv/riscv.cc | 141 +++++ gcc/config/riscv/riscv.md | 3 +- gcc/config/riscv/zicond.md | 84 +++ gcc/ifcvt.cc | 251 ++++++++ gcc/testsuite/gcc.target/riscv/attribute-20.c | 6 + gcc/testsuite/gcc.target/riscv/attribute-21.c | 6 + ...ionalArithmetic_compare_0_return_imm_reg.c | 553 +++++++++++++++++ ...ionalArithmetic_compare_0_return_reg_reg.c | 585 ++++++++++++++++++ ...nalArithmetic_compare_imm_return_imm_reg.c | 297 +++++++++ ...nalArithmetic_compare_imm_return_reg_reg.c | 297 +++++++++ ...nalArithmetic_compare_reg_return_imm_reg.c | 297 +++++++++ ...nalArithmetic_compare_reg_return_reg_reg.c | 329 ++++++++++ .../riscv/zicond-primitiveSemantics.c | 49 ++ .../zicond-primitiveSemantics_compare_imm.c | 57 ++ ...mitiveSemantics_compare_imm_return_0_imm.c | 73 +++ ...tiveSemantics_compare_imm_return_imm_imm.c | 73 +++ ...tiveSemantics_compare_imm_return_imm_reg.c | 65 ++ ...tiveSemantics_compare_imm_return_reg_reg.c | 65 ++ .../zicond-primitiveSemantics_compare_reg.c | 65 ++ ...mitiveSemantics_compare_reg_return_0_imm.c | 73 +++ ...tiveSemantics_compare_reg_return_imm_imm.c | 73 +++ ...tiveSemantics_compare_reg_return_imm_reg.c | 65 ++ ...tiveSemantics_compare_reg_return_reg_reg.c | 77 +++ .../zicond-primitiveSemantics_return_0_imm.c | 65 ++ ...zicond-primitiveSemantics_return_imm_imm.c | 73 +++ ...zicond-primitiveSemantics_return_imm_reg.c | 65 ++ ...zicond-primitiveSemantics_return_reg_reg.c | 65 ++ 29 files changed, 3857 insertions(+), 1 deletion(-) create mode 100644 gcc/config/riscv/zicond.md create mode 100644 gcc/testsuite/gcc.target/riscv/attribute-20.c create mode 100644 gcc/testsuite/gcc.target/riscv/attribute-21.c create mode 100644 gcc/testsuite/gcc.target/riscv/zicond-conditionalArithmetic_compare_0_return_imm_reg.c create mode 100644 gcc/testsuite/gcc.target/riscv/zicond-conditionalArithmetic_compare_0_return_reg_reg.c create mode 100644 gcc/testsuite/gcc.target/riscv/zicond-conditionalArithmetic_compare_imm_return_imm_reg.c create mode 100644 gcc/testsuite/gcc.target/riscv/zicond-conditionalArithmetic_compare_imm_return_reg_reg.c create mode 100644 gcc/testsuite/gcc.target/riscv/zicond-conditionalArithmetic_compare_reg_return_imm_reg.c create mode 100644 gcc/testsuite/gcc.target/riscv/zicond-conditionalArithmetic_compare_reg_return_reg_reg.c create mode 100644 gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics.c create mode 100644 gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_imm.c create mode 100644 gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_imm_return_0_imm.c create mode 100644 gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_imm_return_imm_imm.c create mode 100644 gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_imm_return_imm_reg.c create mode 100644 gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_imm_return_reg_reg.c create mode 100644 gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_reg.c create mode 100644 gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_reg_return_0_imm.c create mode 100644 gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_reg_return_imm_imm.c create mode 100644 gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_reg_return_imm_reg.c create mode 100644 gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_reg_return_reg_reg.c create mode 100644 gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_return_0_imm.c create mode 100644 gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_return_imm_imm.c create mode 100644 gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_return_imm_reg.c create mode 100644 gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_return_reg_reg.c
Comments
On 7/19/23 04:11, Xiao Zeng wrote: > Hi all RISC-V folks: > > This series of patches completes support for the riscv architecture's > Zicond standard extension instruction set. > > Currently, Zicond is in a frozen state. > > See the Zicond specification for details: > https://github.com/riscv/riscv-zicond/releases/download/v1.0-rc2/riscv-zicond-v1.0-rc2.pdf > > Prior to this, other community members have also done related work, as shown in: > https://gcc.gnu.org/pipermail/gcc-patches/2023-February/611767.html > https://sourceware.org/pipermail/binutils/2023-January/125773.html > > Xiao Zeng (5): > [RISC-V] Recognize Zicond extension > [RISC-V] Generate Zicond instruction for basic semantics > [RISC-V] Generate Zicond instruction for select pattern with condition > eq or neq to 0 > [RISC-V] Generate Zicond instruction for select pattern with condition > eq or neq to non-zero > [RISC-V] Generate Zicond instruction for conditional execution [ ... ] So what I'm thinking for the overall kit is to stage it in a bit differently given we have some bits which clearly can go forward as-is or with very minor changes and others that are going to need some iteration/refinement. So I'm going to suggest a few changes so that bits which are non controversial can move forward immediately. 1/5 looked fine as-is. I would split 2/5. The first two patterns you added are non-controversial and could go in immediately. The other 4 patterns (which require some operand matching) will likely need at least one round of iteration and should be a distinct patch. I would split 3/5 as well. 3a would be the costing which I think just needs to use COSTS_N_INSNS (1) rather than 0 for the cost of a conditional move and could then move forward immediately. The bits to wire everything up into the conditional move pattern would be a distinct patch. We did something similar internally in Ventana and I'd like to take the time to make sure the issues we ran into are addressed in your version then do an evaluation of the two approaches. I think patch 4 is probably going to need some work too. I *think* what we did internally at Ventana will work better (utilizing scc for a non-trivial condition). Let's defer patch #5 initially as well. It's going to get tangled up in a whole bunch of changes I think we need to make to ifcvt.cc. The point being that with the bits from #1, #2 and #3 we can get some initial support in immediately. eswincomputing and ventana can both reduce our divergence from the trunk and work together on the rest of the bits. Does that work for you? jeff
On Wed, Jul 26, 2023 at 01:51:00 AM Jeff Law <jeffreyalaw@gmail.com> wrote: > > > >On 7/19/23 04:11, Xiao Zeng wrote: >> Hi all RISC-V folks: >> >> This series of patches completes support for the riscv architecture's >> Zicond standard extension instruction set. >> >> Currently, Zicond is in a frozen state. >> >> See the Zicond specification for details: >> https://github.com/riscv/riscv-zicond/releases/download/v1.0-rc2/riscv-zicond-v1.0-rc2.pdf >> >> Prior to this, other community members have also done related work, as shown in: >> https://gcc.gnu.org/pipermail/gcc-patches/2023-February/611767.html >> https://sourceware.org/pipermail/binutils/2023-January/125773.html >> >> Xiao Zeng (5): >> [RISC-V] Recognize Zicond extension >> [RISC-V] Generate Zicond instruction for basic semantics >> [RISC-V] Generate Zicond instruction for select pattern with condition >> eq or neq to 0 >> [RISC-V] Generate Zicond instruction for select pattern with condition >> eq or neq to non-zero >> [RISC-V] Generate Zicond instruction for conditional execution >[ ... ] >So what I'm thinking for the overall kit is to stage it in a bit >differently given we have some bits which clearly can go forward as-is >or with very minor changes and others that are going to need some >iteration/refinement. > >So I'm going to suggest a few changes so that bits which are non >controversial can move forward immediately. > >1/5 looked fine as-is. > >I would split 2/5. The first two patterns you added are >non-controversial and could go in immediately. The other 4 patterns >(which require some operand matching) will likely need at least one >round of iteration and should be a distinct patch. > > >I would split 3/5 as well. 3a would be the costing which I think just >needs to use COSTS_N_INSNS (1) rather than 0 for the cost of a >conditional move and could then move forward immediately. The bits to >wire everything up into the conditional move pattern would be a distinct >patch. We did something similar internally in Ventana and I'd like to >take the time to make sure the issues we ran into are addressed in your >version then do an evaluation of the two approaches. > >I think patch 4 is probably going to need some work too. I *think* what >we did internally at Ventana will work better (utilizing scc for a >non-trivial condition). > >Let's defer patch #5 initially as well. It's going to get tangled up in >a whole bunch of changes I think we need to make to ifcvt.cc. > >The point being that with the bits from #1, #2 and #3 we can get some >initial support in immediately. eswincomputing and ventana can both >reduce our divergence from the trunk and work together on the rest of >the bits. > >Does that work for you? > >jeff 1 Thanks Jeff for your code review feedback. 2. According to your opinions, I have modified the code, but out of caution for upstream, I conducted a complete regression tests on patch V2, which took some time. I was unable to reply to emails and upload patch V2 in a timely manner. 3 After you and other maintainers made minor modifications to my patch[1/5] and patch[2/5], it has been merged into the master, so I will no longer upload patch V2. 4 patch[1/5] and patch[2/5], which have been merged into the master, have only completed basic support for Zicond, and further optimization work needs to be completed. These further optimization reactions are reflected in my patch[3/5] patch[4/5] and patch[5/5]. 5 As you mentioned in your previous email https://gcc.gnu.org/pipermail/gcc-patches/2023-July/625427.html "eswincomputing and ventana can both reduce our divergence from the trunk and work together on the rest of the bits...". I will reorganize patch[3/5] patch[4/5] and patch[5/5], provide more detailed explanations, and submit them as an alternative solution for further optimization of Zicond. Does that work for you? Xiao Zeng
On 7/27/23 02:43, Xiao Zeng wrote: > > 2. According to your opinions, I have modified the code, but out of caution > for upstream, I conducted a complete regression tests on patch V2, which took > some time. I was unable to reply to emails and upload patch V2 in a timely manner. Sorry to have wasted your time -- zicond/xventanacondops has lingered for quite a while and I had a bit of free time yesterday. I felt it was most useful to try and move this stuff forward. > > 3 After you and other maintainers made minor modifications to my patch[1/5] > and patch[2/5], it has been merged into the master, so I will no longer upload patch V2. Agreed. > > 4 patch[1/5] and patch[2/5], which have been merged into the master, have only > completed basic support for Zicond, and further optimization work needs to be > completed. These further optimization reactions are reflected in my patch[3/5] > patch[4/5] and patch[5/5]. Agreed. > > 5 As you mentioned in your previous email https://gcc.gnu.org/pipermail/gcc-patches/2023-July/625427.html > "eswincomputing and ventana can both reduce our divergence from the trunk > and work together on the rest of the bits...". I will reorganize patch[3/5] patch[4/5] > and patch[5/5], provide more detailed explanations, and submit them as an alternative > solution for further optimization of Zicond. > > Does that work for you? I'm going to look at 3/5 today pretty closely. Exposing zicond to mov<node>cc is something we had implemented inside Ventana and I want to compare/contrast your work with ours. What I like about yours is it keeps all the logic in riscv.cc rather than scattering it across riscv.cc and riscv.md. What I like about the internal Ventana bits is its ability to support arbitrary comparisons by utilizing sCC if the original is not an eq/ne comparison. Ideally we'll be able to get the best of both. Jeff
On Thu, Jul 27, 2023 at 10:43:00 PM Jeff Law <jeffreyalaw@gmail.com> wrote: > > > >On 7/27/23 02:43, Xiao Zeng wrote: > >> >> 2. According to your opinions, I have modified the code, but out of caution >> for upstream, I conducted a complete regression tests on patch V2, which took >> some time. I was unable to reply to emails and upload patch V2 in a timely manner. >Sorry to have wasted your time It's okay I am very willing to accept opinions from the gcc community. >-- zicond/xventanacondops has lingered >for quite a while and I had a bit of free time yesterday. I felt it was >most useful to try and move this stuff forward. > > > >> >> 3 After you and other maintainers made minor modifications to my patch[1/5] >> and patch[2/5], it has been merged into the master, so I will no longer upload patch V2. >Agreed. > >> >> 4 patch[1/5] and patch[2/5], which have been merged into the master, have only >> completed basic support for Zicond, and further optimization work needs to be >> completed. These further optimization reactions are reflected in my patch[3/5] >> patch[4/5] and patch[5/5]. >Agreed. > >> >> 5 As you mentioned in your previous email https://gcc.gnu.org/pipermail/gcc-patches/2023-July/625427.html >> "eswincomputing and ventana can both reduce our divergence from the trunk >> and work together on the rest of the bits...". I will reorganize patch[3/5] patch[4/5] >> and patch[5/5], provide more detailed explanations, and submit them as an alternative >> solution for further optimization of Zicond. >> >> Does that work for you? >I'm going to look at 3/5 today pretty closely. Exposing zicond to >mov<node>cc is something we had implemented inside Ventana and I want to >compare/contrast your work with ours. What a coincidence! > >What I like about yours is it keeps all the logic in riscv.cc rather >than scattering it across riscv.cc and riscv.md. Yes, when I use enough test cases, I cannot find a concise way to optimize all test cases. When I enumerated all possible cases in the mov<mode>cc function of the RISC-V backend, I found a method that satisfied me, which is the method in patch [3/5]. >What I like about the >internal Ventana bits is its ability to support arbitrary comparisons by >utilizing sCC if the original is not an eq/ne comparison. > If it's just for the Zicond instruction set, is it necessary to make judgments outside of eq/ne? After all, it does not support comparison actions other than eq/ne. Of course, it is also possible to use a special technique to use Zicond in non eq/ne comparisons. >Ideally we'll be able to get the best of both. Of course, it is best to unify all situations in one framework. > >Jeff Now that the code on the master has preliminary support for Zicond, I will still submit the optimization patches for Zicond to the community for the convenience of finding the ideal method. Thanks Xiao Zeng
On 7/28/23 00:34, Xiao Zeng wrote: >>> >>> Does that work for you? >> I'm going to look at 3/5 today pretty closely. Exposing zicond to >> mov<node>cc is something we had implemented inside Ventana and I want to >> compare/contrast your work with ours. > > What a coincidence! Zicond is a direct descendant of xventanacondops. The only notable difference is in their encodings. > >> >> What I like about yours is it keeps all the logic in riscv.cc rather >> than scattering it across riscv.cc and riscv.md. > > Yes, when I use enough test cases, I cannot find a concise way to optimize > all test cases. When I enumerated all possible cases in the mov<mode>cc > function of the RISC-V backend, I found a method that satisfied me, which > is the method in patch [3/5]. I got pulled away to another task yesterday, so didn't get as far as I wanted. The biggest inight from yesterday was determining that some of the cases you're handling in riscv_expand_conditional_move were things we were doing inside ifcvt.cc. The difference is likely because the initial work on zicond here was primarily driven by changes to ifcvt. It was only after evaluating that initial implementation that we started to the effort to use zicond at RTL expansion time. I could make a case for either approach, but the more I ponder them the more I'm inclined to go with something like yours. We want to capture the cases implementable as a conditional move as early as possible in the RTL pipeline rather than relying on ifcvt to catch it later. It also avoids polluting ifcvt with transformations that are only likely needed on risc-v. >> > > If it's just for the Zicond instruction set, is it necessary to make judgments > outside of eq/ne? After all, it does not support comparison actions other > than eq/ne. Of course, it is also possible to use a special technique to use > Zicond in non eq/ne comparisons. It's not necessary, but it's definitely helpful to cover the other conditions. In fact, we can even cover a variety of fp conditions by utilizing the sCC type insns. So what I'm looking at for patch #3 is to split out the costing bits into its own patch which can go forward immediately. THen continue evaluating the best way to handle unifying the expander/canonicalization code. Your testcases in patch #3 are particularly helpful to make sure we're not missing cases. Jeff
On Fri, Jul 28, 2023 at 11:03:00 PM Jeff Law <jeffreyalaw@gmail.com> wrote: > > > >On 7/28/23 00:34, Xiao Zeng wrote: > >>>> >>>> Does that work for you? >>> I'm going to look at 3/5 today pretty closely. Exposing zicond to >>> mov<node>cc is something we had implemented inside Ventana and I want to >>> compare/contrast your work with ours. >> >> What a coincidence! >Zicond is a direct descendant of xventanacondops. The only notable >difference is in their encodings. It explains the matter. > >> >>> >>> What I like about yours is it keeps all the logic in riscv.cc rather >>> than scattering it across riscv.cc and riscv.md. >> >> Yes, when I use enough test cases, I cannot find a concise way to optimize >> all test cases. When I enumerated all possible cases in the mov<mode>cc >> function of the RISC-V backend, I found a method that satisfied me, which >> is the method in patch [3/5]. >I got pulled away to another task yesterday, so didn't get as far as I >wanted. The biggest inight from yesterday was determining that some of >the cases you're handling in riscv_expand_conditional_move were things >we were doing inside ifcvt.cc. > >The difference is likely because the initial work on zicond here was >primarily driven by changes to ifcvt. It was only after evaluating that >initial implementation that we started to the effort to use zicond at >RTL expansion time. > >I could make a case for either approach, but the more I ponder them the >more I'm inclined to go with something like yours. >We want to capture >the cases implementable as a conditional move as early as possible in >the RTL pipeline rather than relying on ifcvt to catch it later. It >also avoids polluting ifcvt with transformations that are only likely >needed on risc-v. That's why I did this optimization in riscv.cc riscv_expand_conditional_move. > > >>> >> >> If it's just for the Zicond instruction set, is it necessary to make judgments >> outside of eq/ne? After all, it does not support comparison actions other >> than eq/ne. Of course, it is also possible to use a special technique to use >> Zicond in non eq/ne comparisons. >It's not necessary, but it's definitely helpful to cover the other >conditions. In fact, we can even cover a variety of fp conditions by >utilizing the sCC type insns. It would be great if we could do this. > > >So what I'm looking at for patch #3 is to split out the costing bits >into its own patch which can go forward immediately. As you expected, V2-patch[3/5] has arrived, and its address is: https://gcc.gnu.org/pipermail/gcc-patches/2023-July/625781.html >THen continue >evaluating the best way to handle unifying the expander/canonicalization >code. That's nice. >Your testcases in patch #3 are particularly helpful to make sure >we're not missing cases. Yes, I have always believed that test cases can be redundant, but they cannot be omitted. As we all know, the compiler will always make some magical changes without our knowledge, which may not be what we expect. And test cases can help us stay away from this risk. > >Jeff Thanks Xiao Zeng
On 7/28/23 00:34, Xiao Zeng wrote: > >> >> What I like about yours is it keeps all the logic in riscv.cc rather >> than scattering it across riscv.cc and riscv.md. > > Yes, when I use enough test cases, I cannot find a concise way to optimize > all test cases. When I enumerated all possible cases in the mov<mode>cc > function of the RISC-V backend, I found a method that satisfied me, which > is the method in patch [3/5]. I continue to work with the riscv_expand_conditional_move improvements. Given the deeper problems we have with costing, I'm considering starting to push some of the riscv_expand_conditional_move work you've done without the testcases since those testcases depend on fixing the costing problems. The expansion changes still have value without the costing changes. When we expand a COND_EXPR from gimple, we will attempt to use the conditional move pattern first, without regard for costing. > > If it's just for the Zicond instruction set, is it necessary to make judgments > outside of eq/ne? After all, it does not support comparison actions other > than eq/ne. Of course, it is also possible to use a special technique to use > Zicond in non eq/ne comparisons. It's not necessary, but it's certainly helpful to utilize sCC insns in conjuction with zicond to if-convert other conditional branches. It's conceptually pretty simple. If the incoming code is not EQ/NE or we're not comparing a register against 0, then we can emit an scc insn to get the comparison result into a temporary, then use the standard zicond expansions. Jeff