Message ID | 20221113204858.4062163-1-philipp.tomsich@vrull.eu |
---|---|
State | Deferred, archived |
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 3797D392AC1A for <patchwork@sourceware.org>; Sun, 13 Nov 2022 20:49:22 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-lf1-x133.google.com (mail-lf1-x133.google.com [IPv6:2a00:1450:4864:20::133]) by sourceware.org (Postfix) with ESMTPS id 7847A389839B for <gcc-patches@gcc.gnu.org>; Sun, 13 Nov 2022 20:49:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7847A389839B Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=vrull.eu Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=vrull.eu Received: by mail-lf1-x133.google.com with SMTP id b3so16281866lfv.2 for <gcc-patches@gcc.gnu.org>; Sun, 13 Nov 2022 12:49:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=vrull.eu; s=google; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=q+kJzHS/6RrJp6hdRWT1ODD4KwbOtWAuCjFBl2cYyV4=; b=SO+EecdS/6ML+rynVclvp/YW+AmSEloLd8bpN0HaZPCAGrIeRzzhSRfqoMejJz/IgR c270QSeMvI0PG4ETrkAkvBEZQ0r8Uwamuxf2P9KvuEegkPahts7pFq5iS4fwRJNXO4Vc C+m5glSrF0/TENoq9aBLNJPmY0Z0SyEzRCICL/L4n+wRah0QRs56roakcuR0Na4PYQDU 3qx5UfFbM9LICDm6W04qEvPGR4vUWHycr1xxPTbBbUkJAvBOrN1Lrukh4bGa9stti2UK fabR1d5URaVf0J52z92TfHKSK3ay+a+rDwdS1YP9wWkNQmL2AEui+yH8BGYgOPqNXHgz ym1A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=q+kJzHS/6RrJp6hdRWT1ODD4KwbOtWAuCjFBl2cYyV4=; b=386QV7PwF8TbyLbi62u21varTQiLzAzoaof7vaR1CA4qBYopF6tk1w5DyL+9jOJehn It1XO+uL5m5J1GZnjqxI3HXJ1Hr4mL8U2IXZ0DpmpnycYEh/v4yzJrSbUsJrD/rJfDMH OEFlQ4gF51eM9zRj1LnxRxMkq0dMv3EUsEE03lU8nZ6DdhI9WjVA7lfwcibkY98h90PA Pd088bmv89AueFmJ1xDL6842xK+lGNDUqBZFZUbNBRV3tgymV7Q+zpLXvBxCdo2Tlg5w OF2UADebl6xV8AQMH86lLdlPYuhfJwaoEM54Ku6VeaMWUmTpng3igbZvnM/2wGr/Bjh0 Vcpw== X-Gm-Message-State: ANoB5pnz2GFkG3aEXYRYyK6NJXcniz52LjKGgTWy+eEPX+JMsRwjkRKb Co11nS6wxMiVF+uGP/ZK1kFkLvbvh/obMOxn X-Google-Smtp-Source: AA0mqf7y3PdVZZ0D7Jv8qSFx1COOr+sjUKIjU0wfluvAeNlZFzFVYnfZr+PYecAhlzKlI/CxWT73fw== X-Received: by 2002:ac2:4294:0:b0:4a2:46f6:6cee with SMTP id m20-20020ac24294000000b004a246f66ceemr3670739lfh.642.1668372541743; Sun, 13 Nov 2022 12:49:01 -0800 (PST) Received: from ubuntu-focal.. ([2a01:4f9:3a:1e26::2]) by smtp.gmail.com with ESMTPSA id p5-20020a2ea405000000b00278e9c0d3a2sm1434207ljn.33.2022.11.13.12.49.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 13 Nov 2022 12:49:01 -0800 (PST) From: Philipp Tomsich <philipp.tomsich@vrull.eu> To: gcc-patches@gcc.gnu.org Cc: Christoph Muellner <christoph.muellner@vrull.eu>, Kito Cheng <kito.cheng@gmail.com>, Vineet Gupta <vineetg@rivosinc.com>, Jeff Law <jlaw@ventanamicro.com>, Palmer Dabbelt <palmer@rivosinc.com>, Philipp Tomsich <philipp.tomsich@vrull.eu> Subject: [PATCH] RISC-V: Handle "(a & twobits) == singlebit" in branches using Zbs Date: Sun, 13 Nov 2022 21:48:58 +0100 Message-Id: <20221113204858.4062163-1-philipp.tomsich@vrull.eu> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.3 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 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 |
RISC-V: Handle "(a & twobits) == singlebit" in branches using Zbs
|
|
Commit Message
Philipp Tomsich
Nov. 13, 2022, 8:48 p.m. UTC
Use Zbs when generating a sequence for "if ((a & twobits) == singlebit) ..."
that can be expressed as bexti + bexti + andn.
gcc/ChangeLog:
* config/riscv/bitmanip.md (*branch<X:mode>_mask_twobits_equals_singlebit):
Handle "if ((a & T) == C)" using Zbs, when T has 2 bits set and C has one
of these tow bits set.
* config/riscv/predicates.md (const_twobits_operand): New predicate.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/zbs-if_then_else-01.c: New test.
Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
---
gcc/config/riscv/bitmanip.md | 42 +++++++++++++++++++
gcc/config/riscv/predicates.md | 5 +++
.../gcc.target/riscv/zbs-if_then_else-01.c | 20 +++++++++
3 files changed, 67 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c
Comments
On 11/13/22 13:48, Philipp Tomsich wrote: > Use Zbs when generating a sequence for "if ((a & twobits) == singlebit) ..." > that can be expressed as bexti + bexti + andn. > > gcc/ChangeLog: > > * config/riscv/bitmanip.md (*branch<X:mode>_mask_twobits_equals_singlebit): > Handle "if ((a & T) == C)" using Zbs, when T has 2 bits set and C has one > of these tow bits set. > * config/riscv/predicates.md (const_twobits_operand): New predicate. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/zbs-if_then_else-01.c: New test. s/tow/two/ in the ChangeLog. > > Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu> > --- > > gcc/config/riscv/bitmanip.md | 42 +++++++++++++++++++ > gcc/config/riscv/predicates.md | 5 +++ > .../gcc.target/riscv/zbs-if_then_else-01.c | 20 +++++++++ > 3 files changed, 67 insertions(+) > create mode 100644 gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c > > diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md > index 7a8f4e35880..2cea394671f 100644 > --- a/gcc/config/riscv/bitmanip.md > +++ b/gcc/config/riscv/bitmanip.md > @@ -690,3 +690,45 @@ > "TARGET_ZBS" > [(set (match_dup 0) (zero_extract:X (match_dup 1) (const_int 1) (match_dup 2))) > (set (match_dup 0) (xor:X (match_dup 0) (const_int 1)))]) > + > +;; IF_THEN_ELSE: test for 2 bits of opposite polarity > +(define_insn_and_split "*branch<X:mode>_mask_twobits_equals_singlebit" > + [(set (pc) > + (if_then_else (match_operator 1 "equality_operator" > + [(and:X (match_operand:X 2 "register_operand" "r") > + (match_operand:X 3 "const_twobits_operand" "i")) > + (match_operand:X 4 "single_bit_mask_operand" "i")]) > + (label_ref (match_operand 0 "" "")) > + (pc))) > + (clobber (match_scratch:X 5 "=&r")) > + (clobber (match_scratch:X 6 "=&r"))] > + "TARGET_ZBS && TARGET_ZBB && !SMALL_OPERAND (INTVAL (operands[3]))" > + "#" > + "&& reload_completed" > + [(set (match_dup 5) (zero_extract:X (match_dup 2) > + (const_int 1) > + (match_dup 8))) > + (set (match_dup 6) (zero_extract:X (match_dup 2) > + (const_int 1) > + (match_dup 9))) > + (set (match_dup 6) (and:X (not:X (match_dup 6)) (match_dup 5))) > + (set (pc) (if_then_else (match_op_dup 1 [(match_dup 6) (const_int 0)]) > + (label_ref (match_dup 0)) > + (pc)))] > +{ > + unsigned HOST_WIDE_INT twobits_mask = UINTVAL (operands[3]); > + unsigned HOST_WIDE_INT singlebit_mask = UINTVAL (operands[4]); > + > + /* Make sure that the reference value has one of the bits of the mask set */ > + if ((twobits_mask & singlebit_mask) == 0) > + FAIL; This fails the split, but in the event this scenario occurs we still would up with an ICE as the output template requires splitting. Don't we need to have this be part of the pattern's condition instead so that it never matches in that case? ISTM we should probably have a test to cover this scenario. jeff
On Thu, 17 Nov 2022 at 15:58, Jeff Law <jeffreyalaw@gmail.com> wrote: > > > On 11/13/22 13:48, Philipp Tomsich wrote: > > Use Zbs when generating a sequence for "if ((a & twobits) == singlebit) ..." > > that can be expressed as bexti + bexti + andn. > > > > gcc/ChangeLog: > > > > * config/riscv/bitmanip.md (*branch<X:mode>_mask_twobits_equals_singlebit): > > Handle "if ((a & T) == C)" using Zbs, when T has 2 bits set and C has one > > of these tow bits set. > > * config/riscv/predicates.md (const_twobits_operand): New predicate. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.target/riscv/zbs-if_then_else-01.c: New test. > > s/tow/two/ in the ChangeLog. > > > > > > > > > Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu> > > --- > > > > gcc/config/riscv/bitmanip.md | 42 +++++++++++++++++++ > > gcc/config/riscv/predicates.md | 5 +++ > > .../gcc.target/riscv/zbs-if_then_else-01.c | 20 +++++++++ > > 3 files changed, 67 insertions(+) > > create mode 100644 gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c > > > > diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md > > index 7a8f4e35880..2cea394671f 100644 > > --- a/gcc/config/riscv/bitmanip.md > > +++ b/gcc/config/riscv/bitmanip.md > > @@ -690,3 +690,45 @@ > > "TARGET_ZBS" > > [(set (match_dup 0) (zero_extract:X (match_dup 1) (const_int 1) (match_dup 2))) > > (set (match_dup 0) (xor:X (match_dup 0) (const_int 1)))]) > > + > > +;; IF_THEN_ELSE: test for 2 bits of opposite polarity > > +(define_insn_and_split "*branch<X:mode>_mask_twobits_equals_singlebit" > > + [(set (pc) > > + (if_then_else (match_operator 1 "equality_operator" > > + [(and:X (match_operand:X 2 "register_operand" "r") > > + (match_operand:X 3 "const_twobits_operand" "i")) > > + (match_operand:X 4 "single_bit_mask_operand" "i")]) > > + (label_ref (match_operand 0 "" "")) > > + (pc))) > > + (clobber (match_scratch:X 5 "=&r")) > > + (clobber (match_scratch:X 6 "=&r"))] > > + "TARGET_ZBS && TARGET_ZBB && !SMALL_OPERAND (INTVAL (operands[3]))" > > + "#" > > + "&& reload_completed" > > + [(set (match_dup 5) (zero_extract:X (match_dup 2) > > + (const_int 1) > > + (match_dup 8))) > > + (set (match_dup 6) (zero_extract:X (match_dup 2) > > + (const_int 1) > > + (match_dup 9))) > > + (set (match_dup 6) (and:X (not:X (match_dup 6)) (match_dup 5))) > > + (set (pc) (if_then_else (match_op_dup 1 [(match_dup 6) (const_int 0)]) > > + (label_ref (match_dup 0)) > > + (pc)))] > > +{ > > + unsigned HOST_WIDE_INT twobits_mask = UINTVAL (operands[3]); > > + unsigned HOST_WIDE_INT singlebit_mask = UINTVAL (operands[4]); > > + > > + /* Make sure that the reference value has one of the bits of the mask set */ > > + if ((twobits_mask & singlebit_mask) == 0) > > + FAIL; > > This fails the split, but in the event this scenario occurs we still > would up with an ICE as the output template requires splitting. Don't > we need to have this be part of the pattern's condition instead so that > it never matches in that case? This serves as an assertion only, as that case is non-sensical and will be optimized away by earlier passes (as "a & C == T" with C and T sharing no bits will always be false). IFAIK the preceding transforms should always clean such a check up, but we can't exclude the possibility that with enough command line overrides and params we might see such a non-sensical test making it all the way to the backend. What would you recommend? Adding this to the pattern's condition feels a bit redundant. In fact, I am leaning towards hiding the !SMALL_OPERAND check in yet another predicate that combines const_twobits_operand with a match_test for !SMALL_OPERAND. > ISTM we should probably have a test to cover this scenario. > > > > jeff > >
On 11/17/22 08:12, Philipp Tomsich wrote: > > This serves as an assertion only, as that case is non-sensical and > will be optimized away by earlier passes (as "a & C == T" with C and T > sharing no bits will always be false). > IFAIK the preceding transforms should always clean such a check up, > but we can't exclude the possibility that with enough command line > overrides and params we might see such a non-sensical test making it > all the way to the backend. Good! I was thinking in the back of my mind that the no-sharing-bits case should have been handled in the generic optimizers. Thanks for clarifying. > > What would you recommend? Adding this to the pattern's condition feels > a bit redundant. We can leave it in the splitter. > In fact, I am leaning towards hiding the !SMALL_OPERAND check in yet > another predicate that combines const_twobits_operand with a > match_test for !SMALL_OPERAND. Sure. jeff
On Thu, 17 Nov 2022 at 17:39, Jeff Law <jeffreyalaw@gmail.com> wrote: > > > On 11/17/22 08:12, Philipp Tomsich wrote: > > > > This serves as an assertion only, as that case is non-sensical and > > will be optimized away by earlier passes (as "a & C == T" with C and T > > sharing no bits will always be false). > > IFAIK the preceding transforms should always clean such a check up, > > but we can't exclude the possibility that with enough command line > > overrides and params we might see such a non-sensical test making it > > all the way to the backend. > > Good! I was thinking in the back of my mind that the no-sharing-bits > case should have been handled in the generic optimizers. Thanks for > clarifying. > > > > > > What would you recommend? Adding this to the pattern's condition feels > > a bit redundant. > > We can leave it in the splitter. > > > > In fact, I am leaning towards hiding the !SMALL_OPERAND check in yet > > another predicate that combines const_twobits_operand with a > > match_test for !SMALL_OPERAND. I'll send a v2 with this cleaned up (and look into clarifying things around the FAIL). Philipp.
On Sun, Nov 13, 2022 at 12:51 PM Philipp Tomsich <philipp.tomsich@vrull.eu> wrote: > > Use Zbs when generating a sequence for "if ((a & twobits) == singlebit) ..." > that can be expressed as bexti + bexti + andn. Can't you also handle if ((a & twobits) == 0) case doing a similar thing. That is: two bexti + and and then compare against zero which is exactly the same # of instructions as the above case. > > gcc/ChangeLog: > > * config/riscv/bitmanip.md (*branch<X:mode>_mask_twobits_equals_singlebit): > Handle "if ((a & T) == C)" using Zbs, when T has 2 bits set and C has one > of these tow bits set. > * config/riscv/predicates.md (const_twobits_operand): New predicate. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/zbs-if_then_else-01.c: New test. > > Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu> > --- > > gcc/config/riscv/bitmanip.md | 42 +++++++++++++++++++ > gcc/config/riscv/predicates.md | 5 +++ > .../gcc.target/riscv/zbs-if_then_else-01.c | 20 +++++++++ > 3 files changed, 67 insertions(+) > create mode 100644 gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c > > diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md > index 7a8f4e35880..2cea394671f 100644 > --- a/gcc/config/riscv/bitmanip.md > +++ b/gcc/config/riscv/bitmanip.md > @@ -690,3 +690,45 @@ > "TARGET_ZBS" > [(set (match_dup 0) (zero_extract:X (match_dup 1) (const_int 1) (match_dup 2))) > (set (match_dup 0) (xor:X (match_dup 0) (const_int 1)))]) > + > +;; IF_THEN_ELSE: test for 2 bits of opposite polarity > +(define_insn_and_split "*branch<X:mode>_mask_twobits_equals_singlebit" > + [(set (pc) > + (if_then_else (match_operator 1 "equality_operator" > + [(and:X (match_operand:X 2 "register_operand" "r") > + (match_operand:X 3 "const_twobits_operand" "i")) > + (match_operand:X 4 "single_bit_mask_operand" "i")]) > + (label_ref (match_operand 0 "" "")) > + (pc))) > + (clobber (match_scratch:X 5 "=&r")) > + (clobber (match_scratch:X 6 "=&r"))] > + "TARGET_ZBS && TARGET_ZBB && !SMALL_OPERAND (INTVAL (operands[3]))" > + "#" > + "&& reload_completed" > + [(set (match_dup 5) (zero_extract:X (match_dup 2) > + (const_int 1) > + (match_dup 8))) > + (set (match_dup 6) (zero_extract:X (match_dup 2) > + (const_int 1) > + (match_dup 9))) > + (set (match_dup 6) (and:X (not:X (match_dup 6)) (match_dup 5))) > + (set (pc) (if_then_else (match_op_dup 1 [(match_dup 6) (const_int 0)]) > + (label_ref (match_dup 0)) > + (pc)))] > +{ > + unsigned HOST_WIDE_INT twobits_mask = UINTVAL (operands[3]); > + unsigned HOST_WIDE_INT singlebit_mask = UINTVAL (operands[4]); > + > + /* Make sure that the reference value has one of the bits of the mask set */ > + if ((twobits_mask & singlebit_mask) == 0) > + FAIL; > + > + int setbit = ctz_hwi (singlebit_mask); > + int clearbit = ctz_hwi (twobits_mask & ~singlebit_mask); > + > + operands[1] = gen_rtx_fmt_ee (GET_CODE (operands[1]) == NE ? EQ : NE, > + <X:MODE>mode, operands[6], GEN_INT(0)); > + > + operands[8] = GEN_INT (setbit); > + operands[9] = GEN_INT (clearbit); > +}) > diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md > index 490bff688a7..6e34829a59b 100644 > --- a/gcc/config/riscv/predicates.md > +++ b/gcc/config/riscv/predicates.md > @@ -321,6 +321,11 @@ > (and (match_code "const_int") > (match_test "popcount_hwi (~UINTVAL (op)) == 2"))) > > +;; A CONST_INT operand that has exactly two bits set. > +(define_predicate "const_twobits_operand" > + (and (match_code "const_int") > + (match_test "popcount_hwi (UINTVAL (op)) == 2"))) > + > ;; A CONST_INT operand that fits into the unsigned half of a > ;; signed-immediate after the top bit has been cleared. > (define_predicate "uimm_extra_bit_operand" > diff --git a/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c b/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c > new file mode 100644 > index 00000000000..d249a841ff9 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c > @@ -0,0 +1,20 @@ > +/* { dg-do compile } */ > +/* { dg-options "-march=rv64gc_zbb_zbs -mabi=lp64" } */ > +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-O1" } } */ It would be useful to add a rv32 testcase too. Thanks, Andrew Pinski > + > +void g(); > + > +void f1 (long a) > +{ > + if ((a & ((1ul << 33) | (1 << 4))) == (1ul << 33)) > + g(); > +} > + > +void f2 (long a) > +{ > + if ((a & 0x12) == 0x10) > + g(); > +} > + > +/* { dg-final { scan-assembler-times "bexti\t" 2 } } */ > +/* { dg-final { scan-assembler-times "andn\t" 1 } } */ > -- > 2.34.1 >
On Thu, Nov 17, 2022 at 10:25 AM Andrew Pinski <pinskia@gmail.com> wrote: > > On Sun, Nov 13, 2022 at 12:51 PM Philipp Tomsich > <philipp.tomsich@vrull.eu> wrote: > > > > Use Zbs when generating a sequence for "if ((a & twobits) == singlebit) ..." > > that can be expressed as bexti + bexti + andn. > > Can't you also handle if ((a & twobits) == 0) case doing a similar thing. > That is: > two bexti + and and then compare against zero which is exactly the > same # of instructions as the above case. > > > > > > gcc/ChangeLog: > > > > * config/riscv/bitmanip.md (*branch<X:mode>_mask_twobits_equals_singlebit): > > Handle "if ((a & T) == C)" using Zbs, when T has 2 bits set and C has one > > of these tow bits set. > > * config/riscv/predicates.md (const_twobits_operand): New predicate. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.target/riscv/zbs-if_then_else-01.c: New test. > > > > Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu> > > --- > > > > gcc/config/riscv/bitmanip.md | 42 +++++++++++++++++++ > > gcc/config/riscv/predicates.md | 5 +++ > > .../gcc.target/riscv/zbs-if_then_else-01.c | 20 +++++++++ > > 3 files changed, 67 insertions(+) > > create mode 100644 gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c > > > > diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md > > index 7a8f4e35880..2cea394671f 100644 > > --- a/gcc/config/riscv/bitmanip.md > > +++ b/gcc/config/riscv/bitmanip.md > > @@ -690,3 +690,45 @@ > > "TARGET_ZBS" > > [(set (match_dup 0) (zero_extract:X (match_dup 1) (const_int 1) (match_dup 2))) > > (set (match_dup 0) (xor:X (match_dup 0) (const_int 1)))]) > > + > > +;; IF_THEN_ELSE: test for 2 bits of opposite polarity > > +(define_insn_and_split "*branch<X:mode>_mask_twobits_equals_singlebit" > > + [(set (pc) > > + (if_then_else (match_operator 1 "equality_operator" > > + [(and:X (match_operand:X 2 "register_operand" "r") > > + (match_operand:X 3 "const_twobits_operand" "i")) > > + (match_operand:X 4 "single_bit_mask_operand" "i")]) > > + (label_ref (match_operand 0 "" "")) > > + (pc))) > > + (clobber (match_scratch:X 5 "=&r")) > > + (clobber (match_scratch:X 6 "=&r"))] > > + "TARGET_ZBS && TARGET_ZBB && !SMALL_OPERAND (INTVAL (operands[3]))" Is there a reason why you can't do this at expand time? I think there are recent patches floating around which is supposed to help with that case and the RISCV backend just needs to plug into that infrastructure too. Thanks, Andrew Pinski > > + "#" > > + "&& reload_completed" > > + [(set (match_dup 5) (zero_extract:X (match_dup 2) > > + (const_int 1) > > + (match_dup 8))) > > + (set (match_dup 6) (zero_extract:X (match_dup 2) > > + (const_int 1) > > + (match_dup 9))) > > + (set (match_dup 6) (and:X (not:X (match_dup 6)) (match_dup 5))) > > + (set (pc) (if_then_else (match_op_dup 1 [(match_dup 6) (const_int 0)]) > > + (label_ref (match_dup 0)) > > + (pc)))] > > +{ > > + unsigned HOST_WIDE_INT twobits_mask = UINTVAL (operands[3]); > > + unsigned HOST_WIDE_INT singlebit_mask = UINTVAL (operands[4]); > > + > > + /* Make sure that the reference value has one of the bits of the mask set */ > > + if ((twobits_mask & singlebit_mask) == 0) > > + FAIL; > > + > > + int setbit = ctz_hwi (singlebit_mask); > > + int clearbit = ctz_hwi (twobits_mask & ~singlebit_mask); > > + > > + operands[1] = gen_rtx_fmt_ee (GET_CODE (operands[1]) == NE ? EQ : NE, > > + <X:MODE>mode, operands[6], GEN_INT(0)); > > + > > + operands[8] = GEN_INT (setbit); > > + operands[9] = GEN_INT (clearbit); > > +}) > > diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md > > index 490bff688a7..6e34829a59b 100644 > > --- a/gcc/config/riscv/predicates.md > > +++ b/gcc/config/riscv/predicates.md > > @@ -321,6 +321,11 @@ > > (and (match_code "const_int") > > (match_test "popcount_hwi (~UINTVAL (op)) == 2"))) > > > > +;; A CONST_INT operand that has exactly two bits set. > > +(define_predicate "const_twobits_operand" > > + (and (match_code "const_int") > > + (match_test "popcount_hwi (UINTVAL (op)) == 2"))) > > + > > ;; A CONST_INT operand that fits into the unsigned half of a > > ;; signed-immediate after the top bit has been cleared. > > (define_predicate "uimm_extra_bit_operand" > > diff --git a/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c b/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c > > new file mode 100644 > > index 00000000000..d249a841ff9 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c > > @@ -0,0 +1,20 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-march=rv64gc_zbb_zbs -mabi=lp64" } */ > > +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-O1" } } */ > > It would be useful to add a rv32 testcase too. > > Thanks, > Andrew Pinski > > > + > > +void g(); > > + > > +void f1 (long a) > > +{ > > + if ((a & ((1ul << 33) | (1 << 4))) == (1ul << 33)) > > + g(); > > +} > > + > > +void f2 (long a) > > +{ > > + if ((a & 0x12) == 0x10) > > + g(); > > +} > > + > > +/* { dg-final { scan-assembler-times "bexti\t" 2 } } */ > > +/* { dg-final { scan-assembler-times "andn\t" 1 } } */ > > -- > > 2.34.1 > >
Am I wrong to worry that this will increase dynamic instruction count when used in a loop? The obvious code is more efficient when the constant loads can be hoisted out of a loop. Or does the cost model account for this somehow? On Sun, Nov 13, 2022 at 12:50 PM Philipp Tomsich <philipp.tomsich@vrull.eu> wrote: > > Use Zbs when generating a sequence for "if ((a & twobits) == singlebit) ..." > that can be expressed as bexti + bexti + andn. > > gcc/ChangeLog: > > * config/riscv/bitmanip.md (*branch<X:mode>_mask_twobits_equals_singlebit): > Handle "if ((a & T) == C)" using Zbs, when T has 2 bits set and C has one > of these tow bits set. > * config/riscv/predicates.md (const_twobits_operand): New predicate. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/zbs-if_then_else-01.c: New test. > > Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu> > --- > > gcc/config/riscv/bitmanip.md | 42 +++++++++++++++++++ > gcc/config/riscv/predicates.md | 5 +++ > .../gcc.target/riscv/zbs-if_then_else-01.c | 20 +++++++++ > 3 files changed, 67 insertions(+) > create mode 100644 gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c > > diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md > index 7a8f4e35880..2cea394671f 100644 > --- a/gcc/config/riscv/bitmanip.md > +++ b/gcc/config/riscv/bitmanip.md > @@ -690,3 +690,45 @@ > "TARGET_ZBS" > [(set (match_dup 0) (zero_extract:X (match_dup 1) (const_int 1) (match_dup 2))) > (set (match_dup 0) (xor:X (match_dup 0) (const_int 1)))]) > + > +;; IF_THEN_ELSE: test for 2 bits of opposite polarity > +(define_insn_and_split "*branch<X:mode>_mask_twobits_equals_singlebit" > + [(set (pc) > + (if_then_else (match_operator 1 "equality_operator" > + [(and:X (match_operand:X 2 "register_operand" "r") > + (match_operand:X 3 "const_twobits_operand" "i")) > + (match_operand:X 4 "single_bit_mask_operand" "i")]) > + (label_ref (match_operand 0 "" "")) > + (pc))) > + (clobber (match_scratch:X 5 "=&r")) > + (clobber (match_scratch:X 6 "=&r"))] > + "TARGET_ZBS && TARGET_ZBB && !SMALL_OPERAND (INTVAL (operands[3]))" > + "#" > + "&& reload_completed" > + [(set (match_dup 5) (zero_extract:X (match_dup 2) > + (const_int 1) > + (match_dup 8))) > + (set (match_dup 6) (zero_extract:X (match_dup 2) > + (const_int 1) > + (match_dup 9))) > + (set (match_dup 6) (and:X (not:X (match_dup 6)) (match_dup 5))) > + (set (pc) (if_then_else (match_op_dup 1 [(match_dup 6) (const_int 0)]) > + (label_ref (match_dup 0)) > + (pc)))] > +{ > + unsigned HOST_WIDE_INT twobits_mask = UINTVAL (operands[3]); > + unsigned HOST_WIDE_INT singlebit_mask = UINTVAL (operands[4]); > + > + /* Make sure that the reference value has one of the bits of the mask set */ > + if ((twobits_mask & singlebit_mask) == 0) > + FAIL; > + > + int setbit = ctz_hwi (singlebit_mask); > + int clearbit = ctz_hwi (twobits_mask & ~singlebit_mask); > + > + operands[1] = gen_rtx_fmt_ee (GET_CODE (operands[1]) == NE ? EQ : NE, > + <X:MODE>mode, operands[6], GEN_INT(0)); > + > + operands[8] = GEN_INT (setbit); > + operands[9] = GEN_INT (clearbit); > +}) > diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md > index 490bff688a7..6e34829a59b 100644 > --- a/gcc/config/riscv/predicates.md > +++ b/gcc/config/riscv/predicates.md > @@ -321,6 +321,11 @@ > (and (match_code "const_int") > (match_test "popcount_hwi (~UINTVAL (op)) == 2"))) > > +;; A CONST_INT operand that has exactly two bits set. > +(define_predicate "const_twobits_operand" > + (and (match_code "const_int") > + (match_test "popcount_hwi (UINTVAL (op)) == 2"))) > + > ;; A CONST_INT operand that fits into the unsigned half of a > ;; signed-immediate after the top bit has been cleared. > (define_predicate "uimm_extra_bit_operand" > diff --git a/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c b/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c > new file mode 100644 > index 00000000000..d249a841ff9 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c > @@ -0,0 +1,20 @@ > +/* { dg-do compile } */ > +/* { dg-options "-march=rv64gc_zbb_zbs -mabi=lp64" } */ > +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-O1" } } */ > + > +void g(); > + > +void f1 (long a) > +{ > + if ((a & ((1ul << 33) | (1 << 4))) == (1ul << 33)) > + g(); > +} > + > +void f2 (long a) > +{ > + if ((a & 0x12) == 0x10) > + g(); > +} > + > +/* { dg-final { scan-assembler-times "bexti\t" 2 } } */ > +/* { dg-final { scan-assembler-times "andn\t" 1 } } */ > -- > 2.34.1 >
On Thu, 17 Nov 2022 at 19:33, Andrew Waterman <andrew@sifive.com> wrote: > > Am I wrong to worry that this will increase dynamic instruction count > when used in a loop? The obvious code is more efficient when the > constant loads can be hoisted out of a loop. Or does the cost model > account for this somehow? With this change merged, GCC still hoists the constants out of the loop (just checked with a quick test case). So the cost model seems correct (whether intentionally or accidentally). Thanks, Philipp. > > > On Sun, Nov 13, 2022 at 12:50 PM Philipp Tomsich > <philipp.tomsich@vrull.eu> wrote: > > > > Use Zbs when generating a sequence for "if ((a & twobits) == singlebit) ..." > > that can be expressed as bexti + bexti + andn. > > > > gcc/ChangeLog: > > > > * config/riscv/bitmanip.md (*branch<X:mode>_mask_twobits_equals_singlebit): > > Handle "if ((a & T) == C)" using Zbs, when T has 2 bits set and C has one > > of these tow bits set. > > * config/riscv/predicates.md (const_twobits_operand): New predicate. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.target/riscv/zbs-if_then_else-01.c: New test. > > > > Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu> > > --- > > > > gcc/config/riscv/bitmanip.md | 42 +++++++++++++++++++ > > gcc/config/riscv/predicates.md | 5 +++ > > .../gcc.target/riscv/zbs-if_then_else-01.c | 20 +++++++++ > > 3 files changed, 67 insertions(+) > > create mode 100644 gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c > > > > diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md > > index 7a8f4e35880..2cea394671f 100644 > > --- a/gcc/config/riscv/bitmanip.md > > +++ b/gcc/config/riscv/bitmanip.md > > @@ -690,3 +690,45 @@ > > "TARGET_ZBS" > > [(set (match_dup 0) (zero_extract:X (match_dup 1) (const_int 1) (match_dup 2))) > > (set (match_dup 0) (xor:X (match_dup 0) (const_int 1)))]) > > + > > +;; IF_THEN_ELSE: test for 2 bits of opposite polarity > > +(define_insn_and_split "*branch<X:mode>_mask_twobits_equals_singlebit" > > + [(set (pc) > > + (if_then_else (match_operator 1 "equality_operator" > > + [(and:X (match_operand:X 2 "register_operand" "r") > > + (match_operand:X 3 "const_twobits_operand" "i")) > > + (match_operand:X 4 "single_bit_mask_operand" "i")]) > > + (label_ref (match_operand 0 "" "")) > > + (pc))) > > + (clobber (match_scratch:X 5 "=&r")) > > + (clobber (match_scratch:X 6 "=&r"))] > > + "TARGET_ZBS && TARGET_ZBB && !SMALL_OPERAND (INTVAL (operands[3]))" > > + "#" > > + "&& reload_completed" > > + [(set (match_dup 5) (zero_extract:X (match_dup 2) > > + (const_int 1) > > + (match_dup 8))) > > + (set (match_dup 6) (zero_extract:X (match_dup 2) > > + (const_int 1) > > + (match_dup 9))) > > + (set (match_dup 6) (and:X (not:X (match_dup 6)) (match_dup 5))) > > + (set (pc) (if_then_else (match_op_dup 1 [(match_dup 6) (const_int 0)]) > > + (label_ref (match_dup 0)) > > + (pc)))] > > +{ > > + unsigned HOST_WIDE_INT twobits_mask = UINTVAL (operands[3]); > > + unsigned HOST_WIDE_INT singlebit_mask = UINTVAL (operands[4]); > > + > > + /* Make sure that the reference value has one of the bits of the mask set */ > > + if ((twobits_mask & singlebit_mask) == 0) > > + FAIL; > > + > > + int setbit = ctz_hwi (singlebit_mask); > > + int clearbit = ctz_hwi (twobits_mask & ~singlebit_mask); > > + > > + operands[1] = gen_rtx_fmt_ee (GET_CODE (operands[1]) == NE ? EQ : NE, > > + <X:MODE>mode, operands[6], GEN_INT(0)); > > + > > + operands[8] = GEN_INT (setbit); > > + operands[9] = GEN_INT (clearbit); > > +}) > > diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md > > index 490bff688a7..6e34829a59b 100644 > > --- a/gcc/config/riscv/predicates.md > > +++ b/gcc/config/riscv/predicates.md > > @@ -321,6 +321,11 @@ > > (and (match_code "const_int") > > (match_test "popcount_hwi (~UINTVAL (op)) == 2"))) > > > > +;; A CONST_INT operand that has exactly two bits set. > > +(define_predicate "const_twobits_operand" > > + (and (match_code "const_int") > > + (match_test "popcount_hwi (UINTVAL (op)) == 2"))) > > + > > ;; A CONST_INT operand that fits into the unsigned half of a > > ;; signed-immediate after the top bit has been cleared. > > (define_predicate "uimm_extra_bit_operand" > > diff --git a/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c b/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c > > new file mode 100644 > > index 00000000000..d249a841ff9 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c > > @@ -0,0 +1,20 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-march=rv64gc_zbb_zbs -mabi=lp64" } */ > > +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-O1" } } */ > > + > > +void g(); > > + > > +void f1 (long a) > > +{ > > + if ((a & ((1ul << 33) | (1 << 4))) == (1ul << 33)) > > + g(); > > +} > > + > > +void f2 (long a) > > +{ > > + if ((a & 0x12) == 0x10) > > + g(); > > +} > > + > > +/* { dg-final { scan-assembler-times "bexti\t" 2 } } */ > > +/* { dg-final { scan-assembler-times "andn\t" 1 } } */ > > -- > > 2.34.1 > >
On Thu, Nov 17, 2022 at 10:52 AM Philipp Tomsich <philipp.tomsich@vrull.eu> wrote: > > On Thu, 17 Nov 2022 at 19:33, Andrew Waterman <andrew@sifive.com> wrote: > > > > Am I wrong to worry that this will increase dynamic instruction count > > when used in a loop? The obvious code is more efficient when the > > constant loads can be hoisted out of a loop. Or does the cost model > > account for this somehow? > > With this change merged, GCC still hoists the constants out of the > loop (just checked with a quick test case). > So the cost model seems correct (whether intentionally or accidentally). Cool, thanks for checking. > > Thanks, > Philipp. > > > > > > > On Sun, Nov 13, 2022 at 12:50 PM Philipp Tomsich > > <philipp.tomsich@vrull.eu> wrote: > > > > > > Use Zbs when generating a sequence for "if ((a & twobits) == singlebit) ..." > > > that can be expressed as bexti + bexti + andn. > > > > > > gcc/ChangeLog: > > > > > > * config/riscv/bitmanip.md (*branch<X:mode>_mask_twobits_equals_singlebit): > > > Handle "if ((a & T) == C)" using Zbs, when T has 2 bits set and C has one > > > of these tow bits set. > > > * config/riscv/predicates.md (const_twobits_operand): New predicate. > > > > > > gcc/testsuite/ChangeLog: > > > > > > * gcc.target/riscv/zbs-if_then_else-01.c: New test. > > > > > > Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu> > > > --- > > > > > > gcc/config/riscv/bitmanip.md | 42 +++++++++++++++++++ > > > gcc/config/riscv/predicates.md | 5 +++ > > > .../gcc.target/riscv/zbs-if_then_else-01.c | 20 +++++++++ > > > 3 files changed, 67 insertions(+) > > > create mode 100644 gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c > > > > > > diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md > > > index 7a8f4e35880..2cea394671f 100644 > > > --- a/gcc/config/riscv/bitmanip.md > > > +++ b/gcc/config/riscv/bitmanip.md > > > @@ -690,3 +690,45 @@ > > > "TARGET_ZBS" > > > [(set (match_dup 0) (zero_extract:X (match_dup 1) (const_int 1) (match_dup 2))) > > > (set (match_dup 0) (xor:X (match_dup 0) (const_int 1)))]) > > > + > > > +;; IF_THEN_ELSE: test for 2 bits of opposite polarity > > > +(define_insn_and_split "*branch<X:mode>_mask_twobits_equals_singlebit" > > > + [(set (pc) > > > + (if_then_else (match_operator 1 "equality_operator" > > > + [(and:X (match_operand:X 2 "register_operand" "r") > > > + (match_operand:X 3 "const_twobits_operand" "i")) > > > + (match_operand:X 4 "single_bit_mask_operand" "i")]) > > > + (label_ref (match_operand 0 "" "")) > > > + (pc))) > > > + (clobber (match_scratch:X 5 "=&r")) > > > + (clobber (match_scratch:X 6 "=&r"))] > > > + "TARGET_ZBS && TARGET_ZBB && !SMALL_OPERAND (INTVAL (operands[3]))" > > > + "#" > > > + "&& reload_completed" > > > + [(set (match_dup 5) (zero_extract:X (match_dup 2) > > > + (const_int 1) > > > + (match_dup 8))) > > > + (set (match_dup 6) (zero_extract:X (match_dup 2) > > > + (const_int 1) > > > + (match_dup 9))) > > > + (set (match_dup 6) (and:X (not:X (match_dup 6)) (match_dup 5))) > > > + (set (pc) (if_then_else (match_op_dup 1 [(match_dup 6) (const_int 0)]) > > > + (label_ref (match_dup 0)) > > > + (pc)))] > > > +{ > > > + unsigned HOST_WIDE_INT twobits_mask = UINTVAL (operands[3]); > > > + unsigned HOST_WIDE_INT singlebit_mask = UINTVAL (operands[4]); > > > + > > > + /* Make sure that the reference value has one of the bits of the mask set */ > > > + if ((twobits_mask & singlebit_mask) == 0) > > > + FAIL; > > > + > > > + int setbit = ctz_hwi (singlebit_mask); > > > + int clearbit = ctz_hwi (twobits_mask & ~singlebit_mask); > > > + > > > + operands[1] = gen_rtx_fmt_ee (GET_CODE (operands[1]) == NE ? EQ : NE, > > > + <X:MODE>mode, operands[6], GEN_INT(0)); > > > + > > > + operands[8] = GEN_INT (setbit); > > > + operands[9] = GEN_INT (clearbit); > > > +}) > > > diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md > > > index 490bff688a7..6e34829a59b 100644 > > > --- a/gcc/config/riscv/predicates.md > > > +++ b/gcc/config/riscv/predicates.md > > > @@ -321,6 +321,11 @@ > > > (and (match_code "const_int") > > > (match_test "popcount_hwi (~UINTVAL (op)) == 2"))) > > > > > > +;; A CONST_INT operand that has exactly two bits set. > > > +(define_predicate "const_twobits_operand" > > > + (and (match_code "const_int") > > > + (match_test "popcount_hwi (UINTVAL (op)) == 2"))) > > > + > > > ;; A CONST_INT operand that fits into the unsigned half of a > > > ;; signed-immediate after the top bit has been cleared. > > > (define_predicate "uimm_extra_bit_operand" > > > diff --git a/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c b/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c > > > new file mode 100644 > > > index 00000000000..d249a841ff9 > > > --- /dev/null > > > +++ b/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c > > > @@ -0,0 +1,20 @@ > > > +/* { dg-do compile } */ > > > +/* { dg-options "-march=rv64gc_zbb_zbs -mabi=lp64" } */ > > > +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-O1" } } */ > > > + > > > +void g(); > > > + > > > +void f1 (long a) > > > +{ > > > + if ((a & ((1ul << 33) | (1 << 4))) == (1ul << 33)) > > > + g(); > > > +} > > > + > > > +void f2 (long a) > > > +{ > > > + if ((a & 0x12) == 0x10) > > > + g(); > > > +} > > > + > > > +/* { dg-final { scan-assembler-times "bexti\t" 2 } } */ > > > +/* { dg-final { scan-assembler-times "andn\t" 1 } } */ > > > -- > > > 2.34.1 > > >
On Thu, 17 Nov 2022 at 19:28, Andrew Pinski <pinskia@gmail.com> wrote: > > On Thu, Nov 17, 2022 at 10:25 AM Andrew Pinski <pinskia@gmail.com> wrote: > > > > On Sun, Nov 13, 2022 at 12:51 PM Philipp Tomsich > > <philipp.tomsich@vrull.eu> wrote: > > > > > > Use Zbs when generating a sequence for "if ((a & twobits) == singlebit) ..." > > > that can be expressed as bexti + bexti + andn. > > > > Can't you also handle if ((a & twobits) == 0) case doing a similar thing. > > That is: > > two bexti + and and then compare against zero which is exactly the > > same # of instructions as the above case. We can form any 2-bit constant with BSETI + BSETI (no OR required). So no explicit support for that case will be required (as a AND + BEQ will be formed anyway). > > > > > > > > > > gcc/ChangeLog: > > > > > > * config/riscv/bitmanip.md (*branch<X:mode>_mask_twobits_equals_singlebit): > > > Handle "if ((a & T) == C)" using Zbs, when T has 2 bits set and C has one > > > of these tow bits set. > > > * config/riscv/predicates.md (const_twobits_operand): New predicate. > > > > > > gcc/testsuite/ChangeLog: > > > > > > * gcc.target/riscv/zbs-if_then_else-01.c: New test. > > > > > > Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu> > > > --- > > > > > > gcc/config/riscv/bitmanip.md | 42 +++++++++++++++++++ > > > gcc/config/riscv/predicates.md | 5 +++ > > > .../gcc.target/riscv/zbs-if_then_else-01.c | 20 +++++++++ > > > 3 files changed, 67 insertions(+) > > > create mode 100644 gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c > > > > > > diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md > > > index 7a8f4e35880..2cea394671f 100644 > > > --- a/gcc/config/riscv/bitmanip.md > > > +++ b/gcc/config/riscv/bitmanip.md > > > @@ -690,3 +690,45 @@ > > > "TARGET_ZBS" > > > [(set (match_dup 0) (zero_extract:X (match_dup 1) (const_int 1) (match_dup 2))) > > > (set (match_dup 0) (xor:X (match_dup 0) (const_int 1)))]) > > > + > > > +;; IF_THEN_ELSE: test for 2 bits of opposite polarity > > > +(define_insn_and_split "*branch<X:mode>_mask_twobits_equals_singlebit" > > > + [(set (pc) > > > + (if_then_else (match_operator 1 "equality_operator" > > > + [(and:X (match_operand:X 2 "register_operand" "r") > > > + (match_operand:X 3 "const_twobits_operand" "i")) > > > + (match_operand:X 4 "single_bit_mask_operand" "i")]) > > > + (label_ref (match_operand 0 "" "")) > > > + (pc))) > > > + (clobber (match_scratch:X 5 "=&r")) > > > + (clobber (match_scratch:X 6 "=&r"))] > > > + "TARGET_ZBS && TARGET_ZBB && !SMALL_OPERAND (INTVAL (operands[3]))" > > Is there a reason why you can't do this at expand time? I think there > are recent patches floating around which is supposed to help with that > case and the RISCV backend just needs to plug into that infrastructure > too. I may have missed the specific patches you refer to (pointer to the relevant series appreciated). However, if we move this to expand-time, then ifcvt.cc will run after (and may form this case once our support for polarity-reversed bit tests is merged). So there is good reason to have this pattern. > Thanks, > Andrew Pinski > > > > + "#" > > > + "&& reload_completed" > > > + [(set (match_dup 5) (zero_extract:X (match_dup 2) > > > + (const_int 1) > > > + (match_dup 8))) > > > + (set (match_dup 6) (zero_extract:X (match_dup 2) > > > + (const_int 1) > > > + (match_dup 9))) > > > + (set (match_dup 6) (and:X (not:X (match_dup 6)) (match_dup 5))) > > > + (set (pc) (if_then_else (match_op_dup 1 [(match_dup 6) (const_int 0)]) > > > + (label_ref (match_dup 0)) > > > + (pc)))] > > > +{ > > > + unsigned HOST_WIDE_INT twobits_mask = UINTVAL (operands[3]); > > > + unsigned HOST_WIDE_INT singlebit_mask = UINTVAL (operands[4]); > > > + > > > + /* Make sure that the reference value has one of the bits of the mask set */ > > > + if ((twobits_mask & singlebit_mask) == 0) > > > + FAIL; > > > + > > > + int setbit = ctz_hwi (singlebit_mask); > > > + int clearbit = ctz_hwi (twobits_mask & ~singlebit_mask); > > > + > > > + operands[1] = gen_rtx_fmt_ee (GET_CODE (operands[1]) == NE ? EQ : NE, > > > + <X:MODE>mode, operands[6], GEN_INT(0)); > > > + > > > + operands[8] = GEN_INT (setbit); > > > + operands[9] = GEN_INT (clearbit); > > > +}) > > > diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md > > > index 490bff688a7..6e34829a59b 100644 > > > --- a/gcc/config/riscv/predicates.md > > > +++ b/gcc/config/riscv/predicates.md > > > @@ -321,6 +321,11 @@ > > > (and (match_code "const_int") > > > (match_test "popcount_hwi (~UINTVAL (op)) == 2"))) > > > > > > +;; A CONST_INT operand that has exactly two bits set. > > > +(define_predicate "const_twobits_operand" > > > + (and (match_code "const_int") > > > + (match_test "popcount_hwi (UINTVAL (op)) == 2"))) > > > + > > > ;; A CONST_INT operand that fits into the unsigned half of a > > > ;; signed-immediate after the top bit has been cleared. > > > (define_predicate "uimm_extra_bit_operand" > > > diff --git a/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c b/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c > > > new file mode 100644 > > > index 00000000000..d249a841ff9 > > > --- /dev/null > > > +++ b/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c > > > @@ -0,0 +1,20 @@ > > > +/* { dg-do compile } */ > > > +/* { dg-options "-march=rv64gc_zbb_zbs -mabi=lp64" } */ > > > +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-O1" } } */ > > > > It would be useful to add a rv32 testcase too. > > > > Thanks, > > Andrew Pinski > > > > > + > > > +void g(); > > > + > > > +void f1 (long a) > > > +{ > > > + if ((a & ((1ul << 33) | (1 << 4))) == (1ul << 33)) > > > + g(); > > > +} > > > + > > > +void f2 (long a) > > > +{ > > > + if ((a & 0x12) == 0x10) > > > + g(); > > > +} > > > + > > > +/* { dg-final { scan-assembler-times "bexti\t" 2 } } */ > > > +/* { dg-final { scan-assembler-times "andn\t" 1 } } */ > > > -- > > > 2.34.1 > > >
On Thu, 17 Nov 2022 at 19:56, Andrew Waterman <andrew@sifive.com> wrote: > > On Thu, Nov 17, 2022 at 10:52 AM Philipp Tomsich > <philipp.tomsich@vrull.eu> wrote: > > > > On Thu, 17 Nov 2022 at 19:33, Andrew Waterman <andrew@sifive.com> wrote: > > > > > > Am I wrong to worry that this will increase dynamic instruction count > > > when used in a loop? The obvious code is more efficient when the > > > constant loads can be hoisted out of a loop. Or does the cost model > > > account for this somehow? > > > > With this change merged, GCC still hoists the constants out of the > > loop (just checked with a quick test case). > > So the cost model seems correct (whether intentionally or accidentally). > > Cool, thanks for checking. We have an updated cost-model for IF_THEN_ELSE brewing, but it didn't make the cut (and will need some more adjustments and a lot more testing). It seems to make a difference on some SPEC workloads. I don't have a timeline on finalizing that cost-model improvement yet. > > > > > Thanks, > > Philipp. > > > > > > > > > > > On Sun, Nov 13, 2022 at 12:50 PM Philipp Tomsich > > > <philipp.tomsich@vrull.eu> wrote: > > > > > > > > Use Zbs when generating a sequence for "if ((a & twobits) == singlebit) ..." > > > > that can be expressed as bexti + bexti + andn. > > > > > > > > gcc/ChangeLog: > > > > > > > > * config/riscv/bitmanip.md (*branch<X:mode>_mask_twobits_equals_singlebit): > > > > Handle "if ((a & T) == C)" using Zbs, when T has 2 bits set and C has one > > > > of these tow bits set. > > > > * config/riscv/predicates.md (const_twobits_operand): New predicate. > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > * gcc.target/riscv/zbs-if_then_else-01.c: New test. > > > > > > > > Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu> > > > > --- > > > > > > > > gcc/config/riscv/bitmanip.md | 42 +++++++++++++++++++ > > > > gcc/config/riscv/predicates.md | 5 +++ > > > > .../gcc.target/riscv/zbs-if_then_else-01.c | 20 +++++++++ > > > > 3 files changed, 67 insertions(+) > > > > create mode 100644 gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c > > > > > > > > diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md > > > > index 7a8f4e35880..2cea394671f 100644 > > > > --- a/gcc/config/riscv/bitmanip.md > > > > +++ b/gcc/config/riscv/bitmanip.md > > > > @@ -690,3 +690,45 @@ > > > > "TARGET_ZBS" > > > > [(set (match_dup 0) (zero_extract:X (match_dup 1) (const_int 1) (match_dup 2))) > > > > (set (match_dup 0) (xor:X (match_dup 0) (const_int 1)))]) > > > > + > > > > +;; IF_THEN_ELSE: test for 2 bits of opposite polarity > > > > +(define_insn_and_split "*branch<X:mode>_mask_twobits_equals_singlebit" > > > > + [(set (pc) > > > > + (if_then_else (match_operator 1 "equality_operator" > > > > + [(and:X (match_operand:X 2 "register_operand" "r") > > > > + (match_operand:X 3 "const_twobits_operand" "i")) > > > > + (match_operand:X 4 "single_bit_mask_operand" "i")]) > > > > + (label_ref (match_operand 0 "" "")) > > > > + (pc))) > > > > + (clobber (match_scratch:X 5 "=&r")) > > > > + (clobber (match_scratch:X 6 "=&r"))] > > > > + "TARGET_ZBS && TARGET_ZBB && !SMALL_OPERAND (INTVAL (operands[3]))" > > > > + "#" > > > > + "&& reload_completed" > > > > + [(set (match_dup 5) (zero_extract:X (match_dup 2) > > > > + (const_int 1) > > > > + (match_dup 8))) > > > > + (set (match_dup 6) (zero_extract:X (match_dup 2) > > > > + (const_int 1) > > > > + (match_dup 9))) > > > > + (set (match_dup 6) (and:X (not:X (match_dup 6)) (match_dup 5))) > > > > + (set (pc) (if_then_else (match_op_dup 1 [(match_dup 6) (const_int 0)]) > > > > + (label_ref (match_dup 0)) > > > > + (pc)))] > > > > +{ > > > > + unsigned HOST_WIDE_INT twobits_mask = UINTVAL (operands[3]); > > > > + unsigned HOST_WIDE_INT singlebit_mask = UINTVAL (operands[4]); > > > > + > > > > + /* Make sure that the reference value has one of the bits of the mask set */ > > > > + if ((twobits_mask & singlebit_mask) == 0) > > > > + FAIL; > > > > + > > > > + int setbit = ctz_hwi (singlebit_mask); > > > > + int clearbit = ctz_hwi (twobits_mask & ~singlebit_mask); > > > > + > > > > + operands[1] = gen_rtx_fmt_ee (GET_CODE (operands[1]) == NE ? EQ : NE, > > > > + <X:MODE>mode, operands[6], GEN_INT(0)); > > > > + > > > > + operands[8] = GEN_INT (setbit); > > > > + operands[9] = GEN_INT (clearbit); > > > > +}) > > > > diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md > > > > index 490bff688a7..6e34829a59b 100644 > > > > --- a/gcc/config/riscv/predicates.md > > > > +++ b/gcc/config/riscv/predicates.md > > > > @@ -321,6 +321,11 @@ > > > > (and (match_code "const_int") > > > > (match_test "popcount_hwi (~UINTVAL (op)) == 2"))) > > > > > > > > +;; A CONST_INT operand that has exactly two bits set. > > > > +(define_predicate "const_twobits_operand" > > > > + (and (match_code "const_int") > > > > + (match_test "popcount_hwi (UINTVAL (op)) == 2"))) > > > > + > > > > ;; A CONST_INT operand that fits into the unsigned half of a > > > > ;; signed-immediate after the top bit has been cleared. > > > > (define_predicate "uimm_extra_bit_operand" > > > > diff --git a/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c b/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c > > > > new file mode 100644 > > > > index 00000000000..d249a841ff9 > > > > --- /dev/null > > > > +++ b/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c > > > > @@ -0,0 +1,20 @@ > > > > +/* { dg-do compile } */ > > > > +/* { dg-options "-march=rv64gc_zbb_zbs -mabi=lp64" } */ > > > > +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-O1" } } */ > > > > + > > > > +void g(); > > > > + > > > > +void f1 (long a) > > > > +{ > > > > + if ((a & ((1ul << 33) | (1 << 4))) == (1ul << 33)) > > > > + g(); > > > > +} > > > > + > > > > +void f2 (long a) > > > > +{ > > > > + if ((a & 0x12) == 0x10) > > > > + g(); > > > > +} > > > > + > > > > +/* { dg-final { scan-assembler-times "bexti\t" 2 } } */ > > > > +/* { dg-final { scan-assembler-times "andn\t" 1 } } */ > > > > -- > > > > 2.34.1 > > > >
diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md index 7a8f4e35880..2cea394671f 100644 --- a/gcc/config/riscv/bitmanip.md +++ b/gcc/config/riscv/bitmanip.md @@ -690,3 +690,45 @@ "TARGET_ZBS" [(set (match_dup 0) (zero_extract:X (match_dup 1) (const_int 1) (match_dup 2))) (set (match_dup 0) (xor:X (match_dup 0) (const_int 1)))]) + +;; IF_THEN_ELSE: test for 2 bits of opposite polarity +(define_insn_and_split "*branch<X:mode>_mask_twobits_equals_singlebit" + [(set (pc) + (if_then_else (match_operator 1 "equality_operator" + [(and:X (match_operand:X 2 "register_operand" "r") + (match_operand:X 3 "const_twobits_operand" "i")) + (match_operand:X 4 "single_bit_mask_operand" "i")]) + (label_ref (match_operand 0 "" "")) + (pc))) + (clobber (match_scratch:X 5 "=&r")) + (clobber (match_scratch:X 6 "=&r"))] + "TARGET_ZBS && TARGET_ZBB && !SMALL_OPERAND (INTVAL (operands[3]))" + "#" + "&& reload_completed" + [(set (match_dup 5) (zero_extract:X (match_dup 2) + (const_int 1) + (match_dup 8))) + (set (match_dup 6) (zero_extract:X (match_dup 2) + (const_int 1) + (match_dup 9))) + (set (match_dup 6) (and:X (not:X (match_dup 6)) (match_dup 5))) + (set (pc) (if_then_else (match_op_dup 1 [(match_dup 6) (const_int 0)]) + (label_ref (match_dup 0)) + (pc)))] +{ + unsigned HOST_WIDE_INT twobits_mask = UINTVAL (operands[3]); + unsigned HOST_WIDE_INT singlebit_mask = UINTVAL (operands[4]); + + /* Make sure that the reference value has one of the bits of the mask set */ + if ((twobits_mask & singlebit_mask) == 0) + FAIL; + + int setbit = ctz_hwi (singlebit_mask); + int clearbit = ctz_hwi (twobits_mask & ~singlebit_mask); + + operands[1] = gen_rtx_fmt_ee (GET_CODE (operands[1]) == NE ? EQ : NE, + <X:MODE>mode, operands[6], GEN_INT(0)); + + operands[8] = GEN_INT (setbit); + operands[9] = GEN_INT (clearbit); +}) diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md index 490bff688a7..6e34829a59b 100644 --- a/gcc/config/riscv/predicates.md +++ b/gcc/config/riscv/predicates.md @@ -321,6 +321,11 @@ (and (match_code "const_int") (match_test "popcount_hwi (~UINTVAL (op)) == 2"))) +;; A CONST_INT operand that has exactly two bits set. +(define_predicate "const_twobits_operand" + (and (match_code "const_int") + (match_test "popcount_hwi (UINTVAL (op)) == 2"))) + ;; A CONST_INT operand that fits into the unsigned half of a ;; signed-immediate after the top bit has been cleared. (define_predicate "uimm_extra_bit_operand" diff --git a/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c b/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c new file mode 100644 index 00000000000..d249a841ff9 --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-01.c @@ -0,0 +1,20 @@ +/* { dg-do compile } */ +/* { dg-options "-march=rv64gc_zbb_zbs -mabi=lp64" } */ +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-O1" } } */ + +void g(); + +void f1 (long a) +{ + if ((a & ((1ul << 33) | (1 << 4))) == (1ul << 33)) + g(); +} + +void f2 (long a) +{ + if ((a & 0x12) == 0x10) + g(); +} + +/* { dg-final { scan-assembler-times "bexti\t" 2 } } */ +/* { dg-final { scan-assembler-times "andn\t" 1 } } */