From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-il1-x130.google.com (mail-il1-x130.google.com [IPv6:2607:f8b0:4864:20::130]) by sourceware.org (Postfix) with ESMTPS id C9C733858D3C for ; Sun, 21 Jan 2024 23:21:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C9C733858D3C Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org C9C733858D3C Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::130 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705879289; cv=none; b=k2rRIBArN6iXANvCDA06Q/I2y2JB5g7ZQNJELFyHm6sKjGcVj0tr+OVvVGH4ijdoQu/+eL9HcOHZjm5BmQwIdOxkk5r4mBZBig26/1odTanimOWHJJZmyYdod6DjRqZ/ZzG363Xhik/3ZVdzWgTwQRtvofdNtAQ09Ry2X4yQO84= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705879289; c=relaxed/simple; bh=vJB+43vgfHcBismMppAbJMQFaXfwbsyrsNlwIWK5Mtk=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=CI3SEaP8ar+NVDNfVV5cjBjpsWsLR29AcO6B/VQ4/0X1+dLl6IMGusezxh6k/JpSuLU+s0RCn0VX2z467uryT49QG7HVqR5vI/ZEboquIsTYxeKQrhOs0Ft+fVEhsazghdeSq8OFfKpLXuGXBmCkAjkAqKh7+2t6Qqaeqn52W+o= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-il1-x130.google.com with SMTP id e9e14a558f8ab-3627e9f1b80so882975ab.1 for ; Sun, 21 Jan 2024 15:21:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1705879283; x=1706484083; darn=gcc.gnu.org; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=rkRkaQa0lgMX8vI302vfKubYOav+y+lwHhT70aQPiso=; b=Abp+dDzVa61ewXbc/ZImxV8/LqbvF4FfpKZC7/Z21gExyKDqhVUsx1U7v/QHJEZwGJ 8S0sOrR67XN/1RD8iZfRVld7nyyCPDHAwwKdnSmH8JiGgz8LmDUikQP3uJ1g8UMQjPds smemfi1BbIqBmUzlu3s3+HjyKFuIhKrXVJIQNhUdI+h7v0L9TYFfeliGTL1++0iimXAW T9xE+tNNopxkKJBqxPcHgWJFuOnzqKpBN4sKDLeBHA8W6Xm4UizpkKwz52+y2O/DLet8 Icxt46Lpov/hXyjJpvWzG8PGryQ/Mksnwq2Gt9eMWakRK+VADbTIQW9VuLFGym62aDrb BgBw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705879283; x=1706484083; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=rkRkaQa0lgMX8vI302vfKubYOav+y+lwHhT70aQPiso=; b=SCkgvM4hx7JBhuuvYmGiuiRJ6sn27S54MsRDv4FtokrjZwrkAm3qRUOQXaWFU6pLOu 20JTDVmKNZJnSja0/C5VDV6UusMXCUAsdpwAL0YZ0IsKoMHzRDbP7+O72YdhdPBEQWAg 6rIOdt7cas+hQCG+6LP1/B18GKvSEOa0xKvlHf6Zpc37dOBxjh/XaSESzgPTvaHqKRmG qVejd32r5cURnhgFNZhUDu1q42cwmvay568YZeI/v/ORJ8sWuPifdkXQMLiUKv2L/zr+ xvekMuc15964xJ1KsqbfRvnqXlAcJoHXcCIOOG/5mzb7dM6Gneu/Tkeh98erIigTgpqU f/ew== X-Gm-Message-State: AOJu0YyuXLqfbfPoHCXOsUa1FrpQd6qagkPNESZc5PccGkxxtVLtAJt4 o4X2tE6NMreLzuW11JBxR7RwuKbAjjaNyjuwvIi+XIsnxFjCWs+X X-Google-Smtp-Source: AGHT+IGNE7zhfcPqafyqjBg7uBt58y4KAYz6CrKNrMZjHH+2N/22WzGH6nltZFNt9HBRGB3ThrugVw== X-Received: by 2002:a05:6e02:df1:b0:361:ad21:8c78 with SMTP id m17-20020a056e020df100b00361ad218c78mr4930868ilj.20.1705879282982; Sun, 21 Jan 2024 15:21:22 -0800 (PST) Received: from [172.31.0.109] ([136.36.72.243]) by smtp.gmail.com with ESMTPSA id w12-20020a056e021c8c00b003627a96fb5dsm470217ill.38.2024.01.21.15.21.22 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 21 Jan 2024 15:21:22 -0800 (PST) Message-ID: Date: Sun, 21 Jan 2024 16:21:21 -0700 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 1/1] RISC-V: Add support for XCVbitmanip extension in CV32E40P Content-Language: en-US To: Mary Bennett , gcc-patches@gcc.gnu.org References: <20231109105542.4013483-1-mary.bennett@embecosm.com^Cthread> <20240116162552.461635-1-mary.bennett@embecosm.com> <20240116162552.461635-2-mary.bennett@embecosm.com> From: Jeff Law In-Reply-To: <20240116162552.461635-2-mary.bennett@embecosm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-8.4 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: On 1/16/24 09:25, Mary Bennett wrote: > Spec: github.com/openhwgroup/core-v-sw/blob/master/specifications/corev-builtin-spec.md > > Contributors: > Mary Bennett > Nandni Jamnadas > Pietra Ferreira > Charlie Keaney > Jessica Mills > Craig Blackmore > Simon Cook > Jeremy Bennett > Helene Chelin > > gcc/ChangeLog: > * common/config/riscv/riscv-common.cc: Add XCVbitmanip. > * config/riscv/constraints.md: Likewise. > * config/riscv/corev.def: Likewise. > * config/riscv/corev.md: Likewise. > * config/riscv/predicates.md: Likewise. > * config/riscv/riscv-builtins.cc (AVAIL): Likewise. > * config/riscv/riscv-ftypes.def: Likewise. > * config/riscv/riscv.opt: Likewise. > * doc/extend.texi: Add XCVbitmanip builtin documentation. > * doc/sourcebuild.texi: Likewise. > > gcc/testsuite/ChangeLog: > * gcc.target/riscv/cv-bitmanip-compile-bclr.c: New test. > * gcc.target/riscv/cv-bitmanip-compile-bclrr.c: New test. > * gcc.target/riscv/cv-bitmanip-compile-bitrev.c: New test. > * gcc.target/riscv/cv-bitmanip-compile-bset.c: New test. > * gcc.target/riscv/cv-bitmanip-compile-bsetr.c: New test. > * gcc.target/riscv/cv-bitmanip-compile-clb.c: New test. > * gcc.target/riscv/cv-bitmanip-compile-cnt.c: New test. > * gcc.target/riscv/cv-bitmanip-compile-extract.c: New test. > * gcc.target/riscv/cv-bitmanip-compile-extractr.c: New test. > * gcc.target/riscv/cv-bitmanip-compile-extractu.c: New test. > * gcc.target/riscv/cv-bitmanip-compile-extractur.c: New test. > * gcc.target/riscv/cv-bitmanip-compile-ff1.c: New test. > * gcc.target/riscv/cv-bitmanip-compile-fl1.c: New test. > * gcc.target/riscv/cv-bitmanip-compile-insert.c: New test. > * gcc.target/riscv/cv-bitmanip-compile-insertr.c: New test. > * gcc.target/riscv/cv-bitmanip-compile-ror.c: New test. > * gcc.target/riscv/cv-bitmanip-fail-compile-bclr.c: New test. > * gcc.target/riscv/cv-bitmanip-fail-compile-bitrev.c: New test. > * gcc.target/riscv/cv-bitmanip-fail-compile-bset.c: New test. > * gcc.target/riscv/cv-bitmanip-fail-compile-extract.c: New test. > * gcc.target/riscv/cv-bitmanip-fail-compile-extractu.c: New test. > * gcc.target/riscv/cv-bitmanip-fail-compile-insert.c: New test. > * lib/target-supports.exp: Add proc for the XCVbitmanip extension. > --- > diff --git a/gcc/config/riscv/corev.md b/gcc/config/riscv/corev.md > index adad2409fb6..9afd69ec080 100644 > --- a/gcc/config/riscv/corev.md > +++ b/gcc/config/riscv/corev.md > @@ -706,3 +710,163 @@ > > [(set_attr "type" "load") > (set_attr "mode" "SI")]) > + > +;; XCVBITMANIP builtins > + > +(define_insn "riscv_cv_bitmanip_extract" > + [(set (match_operand:SI 0 "register_operand" "=r,r") > + (sign_extend:SI (and:SI (rotate:SI (ashift:SI (const_int 1) > + (ashiftrt:HI > + (match_operand:HI 2 "bit_extract_operand" "CV_bit_si10,r") > + (const_int 5))) > + (and:HI (match_dup 2) > + (const_int 31))) > + (match_operand:SI 1 "register_operand" "r,r"))))] So it looks like the you've got an ashift:SI where the shift amount of the result of an ashiftrt:HI -- note the difference in the modes. That doesn't seem right. It also looks like the indentation on the RTL pattern is wrong -- that ashiftrt is an argument of the ashift:SI, so it should line up with the (const_int 1) argument. It looks like the same issue is repeated on the extractu pattern. Which leads to a second comment. I think the two patterns can be combined. There'a an "any_extend" iterator that you could use in place of the sign_extend/zero_extend. And there's a "u" iterator that you can use in the output template to conditionally emit a "u". The insertion, bclr, and bset patterns seem to be goofy on their modes in a similar way. > + > +(define_insn "riscv_cv_bitmanip_ff1" > + [(set (match_operand:QI 0 "register_operand" "=r") > + (if_then_else (eq:SI (match_operand:SI 1 "register_operand" "r") (const_int 0)) > + (const_int 32) > + (minus:SI (ffs:SI (match_dup 1)) > + (const_int 1))))] Presumably this is an if-then-else to deal with the case when the input value is zero? Is there any way to use ctz/clz in combination with C[LT]Z_DEFINED_VALUE_AT_ZERO instead of ffs with a conditional? More generally, can we avoid the UNSPECs in here by defining these operations in terms of ffs, clz, ctz? > + > +(define_insn "riscv_cv_bitmanip_cnt" > + [(set (match_operand:QI 0 "register_operand" "=r") > + (truncate:QI (popcount:SI (match_operand:SI 1 "register_operand" "r"))))] > + > + "TARGET_XCVBITMANIP && !TARGET_64BIT" > + "cv.cnt\t%0,%1" > + [(set_attr "type" "bitmanip") > + (set_attr "mode" "SI")]) Interesting truncation here? Does the cv.cnt instruction really just write the result in QImode? Or are you trying to express that the number of bits must be in the range 0..32 inclusive? The former would make your code correct, the latter is better left to the generic VRP optimizer to discover. > + > +(define_insn "riscv_cv_bitmanip_bitrev" > + [(set (match_operand:SI 0 "register_operand" "=r") > + (unspec:SI [(match_operand:SI 1 "register_operand" "r") > + (match_operand:QI 2 "const_csr_operand" "K") > + (match_operand:QI 3 "const_int2_operand" "D03")] > + UNSPEC_CV_BITMANIP_BITREV))] Note I think there's a bitreverse opcode now. Jeff