From patchwork Fri Aug 9 18:31:09 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Law X-Patchwork-Id: 1971059 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=ventanamicro.com header.i=@ventanamicro.com header.a=rsa-sha256 header.s=google header.b=eKHw5IDp; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces~incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WgXYl10wTz1yYl for ; Sat, 10 Aug 2024 04:31:39 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 31D08385C6CB for ; Fri, 9 Aug 2024 18:31:37 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-pj1-x102c.google.com (mail-pj1-x102c.google.com [IPv6:2607:f8b0:4864:20::102c]) by sourceware.org (Postfix) with ESMTPS id 3D8D73858CD9 for ; Fri, 9 Aug 2024 18:31:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3D8D73858CD9 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=ventanamicro.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=ventanamicro.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 3D8D73858CD9 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::102c ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1723228278; cv=none; b=LUyc4ANdhRIph5zWh1MC+R+A5GF9cYEpDsyJVI5FzRWCGvPfXRPD+hVqgcQZWVkwgXtO0nCWAusRz4Bda76t9Gx3v8/o0dOEkUyA17SK0XHSqWCk4ccif88A1nMidDazdF2XotP8jMXvp2y2WHUHYIHkFb/1f2XYuYeGiRQSQrE= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1723228278; c=relaxed/simple; bh=LzRfBhEN+QHX36ikeQusWqDAhusRe/9Lb4e49L8M6eU=; h=DKIM-Signature:Message-ID:Date:MIME-Version:From:Subject:To; b=J2yN1/lWnoJ6qVQceCEvYEjlecuDzTCE90LiuTQohlVxS7/uNZeOzKnUGT2oB15JPmRq5L/5cx8H0gbqIZkqERFDQbzdEKQCHHaOHdes2XD1oa6FQW10+u5ajC+WzSZM+jSSnmtOafwuM9o12msFsWlpv/Fndu/yb/uiGbt1Oys= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pj1-x102c.google.com with SMTP id 98e67ed59e1d1-2cb5787b4a5so1802098a91.2 for ; Fri, 09 Aug 2024 11:31:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ventanamicro.com; s=google; t=1723228275; x=1723833075; darn=gcc.gnu.org; h=to:subject:from:content-language:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=L9GC1r90J5Pbhpe7wd0X/7uN4cnftzvfBWiygWWt0GY=; b=eKHw5IDpfeQGwTnuW2CFuikQwimTRQ70ngMrjTjDKCUawrXQM4o2Ulqqb3fXDYmvix CaZuTxWaeg5d4n7ViJ0WTy/5hQrYkW8VwFId292cECOFpfe0K5QR0YhDSX6rFxONf0K6 bMTG0KWFJyGRokzMdRhmDS1YkCE3f6uDAHuzMyYPXfOdoHvgxzsnxEvFoDzVJbypHbQM dn1syrGJxxm7oRSBwR/QBrwmMTHioDp0fyJwyexYNABytpgBZdvLf2PlcjDNM+B47VVX n1RjnCbwmiJvfopHmG6318S5Z5HLbgGSzBzatvAr6ArlU5u1lzQl72W6R2Hi/dTnGab9 Aa+w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723228275; x=1723833075; h=to:subject:from:content-language:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=L9GC1r90J5Pbhpe7wd0X/7uN4cnftzvfBWiygWWt0GY=; b=NGZagXxqnN5OSl9GNAi7n/prXtV/1szhVYeY6d7SJ0VYRrIt6vYzOgRD204TyMY5gF go9q1JB3dbiIMfd9x3hZBuERjrYMYi7JnVHd1T6FdL1D08WUO0P+Um6PO5+Hr2jQRbm0 qXYRvyn+4eEYp0yaA2Gbd1iKudQ3RPWpSYVGfchCzLNFLw+Xi8hlF+VhYnALDjuELnXw ayr/Qk2YSUI2T14NmDVJPL6G3mVp+j0VELMOC/EjZRECOoS7Zw3fGFZKVvVJ3jNlpjBK hEJdRuSSQoZ3EMeVKC96RfHRo82RFj5jMeYNDDdUMaE/BlWpPkXzYKrd0HveFo5PXjPe RpKA== X-Gm-Message-State: AOJu0YzSDUgfajrciA/sn7xbzTxJ6LikTV92tFn/tjpxQbXYvag0Gl7i YEF4viXqXtZQiJTraZkntaT+PyTwrP8UrFSIq9ulpREMB0S69XqWc0zWm/chGdRr0jorAAfAf+k Peb4= X-Google-Smtp-Source: AGHT+IGqGbJnrIFI8uqcJ8HabERI3OfhXRxhi3R1MCXe2WgtXAZ0KizIw5A64DZ+MPW8zyEhjXx5iQ== X-Received: by 2002:a17:90b:3a82:b0:2c9:887e:46de with SMTP id 98e67ed59e1d1-2d1e7fce4eemr2646215a91.12.1723228274712; Fri, 09 Aug 2024 11:31:14 -0700 (PDT) Received: from [172.31.0.109] ([136.36.72.243]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-2d1c9db8047sm3347315a91.41.2024.08.09.11.31.12 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 09 Aug 2024 11:31:14 -0700 (PDT) Message-ID: Date: Fri, 9 Aug 2024 12:31:09 -0600 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Content-Language: en-US From: Jeff Law Subject: [to-be-committed][RISC-V][PR target/116283] Fix split code for recent Zbs improvements with masked bit positions To: "gcc-patches@gcc.gnu.org" X-Spam-Status: No, score=-11.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces~incoming=patchwork.ozlabs.org@gcc.gnu.org So Patrick's fuzzer found an interesting little buglet in the Zbs improvements I added a couple months back. Specifically when we have masked bit position for a Zbs instruction. If the mask has extraneous bits set we'll generate an unrecognizable insn due to an invalid constant. More concretely, let's take this pattern: > (define_insn_and_split "" > [(set (match_operand:DI 0 "register_operand" "=r") > (any_extend:DI > (ashift:SI (const_int 1) > (subreg:QI > (and:DI (match_operand:DI 1 "register_operand" "r") > (match_operand 2 "const_int_operand")) 0))))] What we need to know to transform this into bset for rv64. After masking the shift count we want to know the low 5 bits aren't 0x1f. If they were 0x1f, then the constant generated would be 0x80000000 which would then need sign extension out to 64bits, which the bset instruction will not do for us. We can ignore anything outside the low 5 bits. The mode of the shift is SI, so shifting by 32+ bits is undefined behavior. It's also worth explicitly mentioning that the hardware is going to mask the count against 0x3f. The net is if (operands[2] & 0x1f) != 0x1f, then this transformation is safe. So onto the generated split code... > [(set (match_dup 0) (and:DI (match_dup 1) (match_dup 2))) > (set (match_dup 0) (zero_extend:DI (ashift:SI > (const_int 1) > (subreg:QI (match_dup 0) 0))))] Which would seemingly do exactly what we want. The problem is the first split insn. If the constant does not fit into a simm12, that insn won't be recognized resulting in the ICE. The fix is simple, we just need to mask the constant before generating RTL. We can just mask it against 0x1f since we only care about the low 5 bits. This affects multiple patterns. I've added the appropriate fix to all of them. Tested in my tester. Waiting for the pre-commit bits to run before pushing. Jeff PR target/116283 gcc/ * config/riscv/bitmanip.md (Zbs combiner patterns/splitters): Mask the bit position in the split code appropriately. gcc/testsuite/ * gcc.target/riscv/pr116283.c: New test diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md index b19295cd942..6872ee56022 100644 --- a/gcc/config/riscv/bitmanip.md +++ b/gcc/config/riscv/bitmanip.md @@ -643,7 +643,10 @@ (define_insn_and_split "" (set (match_dup 3) (and:DI (not:DI (match_dup 1)) (match_dup 3))) (set (match_dup 0) (zero_extend:DI (ashift:SI (const_int 1) (match_dup 4))))] - { operands[4] = gen_lowpart (QImode, operands[3]); } +{ + operands[4] = gen_lowpart (QImode, operands[3]); + operands[2] = GEN_INT (INTVAL (operands[2]) & 0x1f); +} [(set_attr "type" "bitmanip")]) (define_insn_and_split "" @@ -662,7 +665,7 @@ (define_insn_and_split "" (set (match_dup 0) (zero_extend:DI (ashift:SI (const_int 1) (subreg:QI (match_dup 0) 0))))] - { } + { operands[2] = GEN_INT (INTVAL (operands[2]) & 0x1f); } [(set_attr "type" "bitmanip")]) ;; Similarly two patterns for IOR/XOR generating bset/binv to @@ -687,7 +690,10 @@ (define_insn_and_split "" [(set (match_dup 4) (match_dup 2)) (set (match_dup 4) (and:DI (not:DI (match_dup 1)) (match_dup 4))) (set (match_dup 0) (any_or:DI (ashift:DI (const_int 1) (match_dup 5)) (match_dup 3)))] - { operands[5] = gen_lowpart (QImode, operands[4]); } +{ + operands[5] = gen_lowpart (QImode, operands[4]); + operands[2] = GEN_INT (INTVAL (operands[2]) & 0x1f); +} [(set_attr "type" "bitmanip")]) (define_insn_and_split "" @@ -708,7 +714,7 @@ (define_insn_and_split "" "&& reload_completed" [(set (match_dup 4) (and:DI (match_dup 1) (match_dup 2))) (set (match_dup 0) (any_or:DI (ashift:DI (const_int 1) (subreg:QI (match_dup 4) 0)) (match_dup 3)))] - { } + { operands[2] = GEN_INT (INTVAL (operands[2]) & 0x1f); } [(set_attr "type" "bitmanip")]) ;; Similarly two patterns for AND generating bclr to @@ -734,7 +740,10 @@ (define_insn_and_split "" [(set (match_dup 4) (match_dup 2)) (set (match_dup 4) (and:DI (not:DI (match_dup 1)) (match_dup 4))) (set (match_dup 0) (and:DI (rotate:DI (const_int -2) (match_dup 5)) (match_dup 3)))] - { operands[5] = gen_lowpart (QImode, operands[4]); } +{ + operands[5] = gen_lowpart (QImode, operands[4]); + operands[2] = GEN_INT (INTVAL (operands[2]) & 0x1f); +} [(set_attr "type" "bitmanip")]) @@ -757,7 +766,10 @@ (define_insn_and_split "" "&& reload_completed" [(set (match_dup 4) (and:DI (match_dup 1) (match_dup 2))) (set (match_dup 0) (and:DI (rotate:DI (const_int -2) (match_dup 5)) (match_dup 3)))] - { operands[5] = gen_lowpart (QImode, operands[4]); } +{ + operands[5] = gen_lowpart (QImode, operands[4]); + operands[2] = GEN_INT (INTVAL (operands[2]) & 0x1f); +} [(set_attr "type" "bitmanip")]) (define_insn "*bset_1_mask" diff --git a/gcc/testsuite/gcc.target/riscv/pr116283.c b/gcc/testsuite/gcc.target/riscv/pr116283.c new file mode 100644 index 00000000000..21861557edc --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/pr116283.c @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-additional-options "-march=rv64id_zbs -mabi=lp64d" } */ + +char c; +#define d(a, b) \ + { \ + __typeof__(a) e = a; \ + e; \ + } +long f; +void g(signed h[][9][9][9][9]) { + for (unsigned i = f; i; i += 3) + c = (d(1 << (3629 & h[i][i][1][5][i]), )); +} +