From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out30-43.freemail.mail.aliyun.com (out30-43.freemail.mail.aliyun.com [115.124.30.43]) by sourceware.org (Postfix) with ESMTPS id B2FE1384F6C6 for ; Fri, 18 Nov 2022 04:22:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B2FE1384F6C6 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=linux.alibaba.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linux.alibaba.com X-Alimail-AntiSpam:AC=PASS;BC=-1|-1;BR=01201311R381e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=ay29a033018046056;MF=cooper.qu@linux.alibaba.com;NM=1;PH=DS;RN=2;SR=0;TI=SMTPD_---0VV3RfgC_1668745316; Received: from localhost(mailfrom:cooper.qu@linux.alibaba.com fp:SMTPD_---0VV3RfgC_1668745316) by smtp.aliyun-inc.com; Fri, 18 Nov 2022 12:21:57 +0800 Date: Fri, 18 Nov 2022 12:21:55 +0800 From: "cooper.qu@linux.alibaba.com" To: Christoph Muellner Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH 5/7] riscv: thead: Add support for XTheadBb ISA extension Message-ID: <20221118033148.GA12113@L-PF1ZESZG-1136.hz.ali.com> References: <20221113214636.2747737-1-christoph.muellner@vrull.eu> <20221113214636.2747737-6-christoph.muellner@vrull.eu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20221113214636.2747737-6-christoph.muellner@vrull.eu> X-Spam-Status: No, score=-20.5 required=5.0 tests=BAYES_00,ENV_AND_HDR_SPF_MATCH,GIT_PATCH_0,KAM_DMARC_STATUS,KAM_SHORT,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP,UNPARSEABLE_RELAY,USER_IN_DEF_SPF_WL 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 Sun, Nov 13, 2022 at 10:46:34PM +0100, Christoph Muellner wrote: > +(define_expand "extv" > + [(set (match_operand:GPR 0 "register_operand" "=r") > + (sign_extract:GPR (match_operand:GPR 1 "register_operand" "r") > + (match_operand 2 "const_int_operand") > + (match_operand 3 "const_int_operand")))] > + "TARGET_XTHEADBB" > +{ > + if (TARGET_XTHEADBB > + && ((INTVAL (operands[2]) + INTVAL (operands[3])) > + >= GET_MODE_BITSIZE (GET_MODE (operands[1])).to_constant ())) > + FAIL; > +}) > + > +(define_expand "extzv" > + [(set (match_operand:GPR 0 "register_operand" "=r") > + (zero_extract:GPR (match_operand:GPR 1 "register_operand" "r") > + (match_operand 2 "const_int_operand") > + (match_operand 3 "const_int_operand")))] > + "TARGET_XTHEADBB" > +{ > + if (TARGET_XTHEADBB > + && ((INTVAL (operands[2]) + INTVAL (operands[3])) > + >= GET_MODE_BITSIZE (GET_MODE (operands[1])).to_constant ())) > + FAIL; I doubt whether it is necessary to add this judgment here, and other architectures seem to have not added it. But there's nothing wrong with adding > + > +(define_insn "*th_ext" > + [(set (match_operand:X 0 "register_operand" "=r") > + (sign_extract:X (match_operand:X 1 "register_operand" "r") > + (match_operand 2 "const_int_operand") > + (match_operand 3 "const_int_operand")))] > + "TARGET_XTHEADBB" > +{ > + operands[3] = GEN_INT (INTVAL (operands[2]) + INTVAL (operands[3])); > + return "th.ext\t%0,%1,%2,%3"; > +} > + [(set_attr "type" "bitmanip")]) > + > +(define_insn "*th_extu" > + [(set (match_operand:X 0 "register_operand" "=r") > + (zero_extract:X (match_operand:X 1 "register_operand" "r") > + (match_operand 2 "const_int_operand") > + (match_operand 3 "const_int_operand")))] > + "TARGET_XTHEADBB" > +{ > + operands[3] = GEN_INT (INTVAL (operands[2]) + INTVAL (operands[3])); > + return "th.extu\t%0,%1,%2,%3"; > +} > + [(set_attr "type" "bitmanip")]) > + I think the operands[3] should be: operands[3] = GEN_INT (INTVAL (operands[2]) + INTVAL (operands[3])) - 1 Because the ext and extu extract the bits %2..%3, when size is 1, the 2% equals to 3%. And a small optimization can be done here, the extzv can generate c.andi when the start bit is 0 and the size is less than 7. > +/* { dg-final { scan-assembler-times "th.revw\t" 2 } } */ > +/* { dg-final { scan-assembler-times "th.rev\t" 2 } } */ > diff --git a/gcc/testsuite/gcc.target/riscv/xtheadbb-srri.c b/gcc/testsuite/gcc.target/riscv/xtheadbb-srri.c > new file mode 100644 > index 00000000000..cd992ae3f0a > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/xtheadbb-srri.c > @@ -0,0 +1,18 @@ > +/* { dg-do compile } */ > +/* { dg-options "-march=rv64gc_xtheadbb -mabi=lp64" } */ > +/* { dg-skip-if "" { *-*-* } { "-O0" "-g" } } */ > + > +unsigned long foo1(unsigned long rs1) > +{ > + long shamt = __riscv_xlen - 11; > + return (rs1 << shamt) | > + (rs1 >> ((__riscv_xlen - shamt) & (__riscv_xlen - 1))); > +} > +unsigned long foo2(unsigned long rs1) > +{ > + unsigned long shamt = __riscv_xlen - 11; > + return (rs1 >> shamt) | > + (rs1 << ((__riscv_xlen - shamt) & (__riscv_xlen - 1))); > +} > + > +/* { dg-final { scan-assembler-times "th.srri" 2 } } */ Why is there no testcase for ff1 here? It can be generated by the builtin function '__builtin_clzl'. Thanks, Cooper