From patchwork Fri Jul 26 14:17:27 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Law X-Patchwork-Id: 1965330 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=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20230601 header.b=SyDyLVkC; 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 4WVqbQ6nCGz1ybY for ; Sat, 27 Jul 2024 00:17:54 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 176E73860C2C for ; Fri, 26 Jul 2024 14:17:53 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-ot1-x336.google.com (mail-ot1-x336.google.com [IPv6:2607:f8b0:4864:20::336]) by sourceware.org (Postfix) with ESMTPS id 44DAA3858D26 for ; Fri, 26 Jul 2024 14:17:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 44DAA3858D26 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 44DAA3858D26 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::336 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1722003453; cv=none; b=QMzxsgeIN783v0/qaiowsQUySeGWCWe/TEvGp5e1DEMa+Zy5krkEIKcHgSXUQlX2eDY2X3boQe4aETI5W9bfTDbOLUkbOYGB0e6fOP8RD/dBnalNoTDwIMjtRyyU/VuhyNEGnIY4DcO0DuJFgTsqIWIjteRFDjAPL6KeL+yVmgM= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1722003453; c=relaxed/simple; bh=teO1IltTpnW0xwVHO/b3q0xgooEx8APLvHOMmWMQl54=; h=DKIM-Signature:Message-ID:Date:MIME-Version:From:Subject:To; b=WpkGJ+KUagRQ3qpECyXTZKqie4dLDDOYth4mvNlrIuzE9XUJ0rAMCXzHvHlm8mwdSuJwqMnrluqrzQXP5v6NIM5Bd4D3YYy4szYZhNnzz6Pq6/HssTeMuk/7W81wQQcwI2rLowi8jBx8eEDYEdblmDkx4miV92f0UARNpP6Owdo= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-ot1-x336.google.com with SMTP id 46e09a7af769-7093705c708so976662a34.1 for ; Fri, 26 Jul 2024 07:17:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1722003450; x=1722608250; 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=VBPoJGRK9mlLOjG0zJWUaBmONDbieXNm5VnkemOp2+A=; b=SyDyLVkCRVfxLOmsfJyh0bXzWq1obBhRcWaygPaseqlQS5YLdD7Qp7qJda64Tz+AQT CqYN9bc/ciipmtml8vjBbAKIwSIwOjDtI9uMDDn8eOrEneR/uggpaUp47nraohlf04uT brgZCk8UNsDf7YN8lnGwQMKrlHxxtpLJVnklqcl4BavkJ6ZDPBAw6F24C8ioU9kzNk0/ gbA2jQphn/lLsZiFa06pMds7Pdu80drTL6litWbZsO31poE3cy1UP/qrOVWTxcPS75as fT5TMoybujUrA1DGmAj1jkKGN3RgDLN7shaaqxGbyvL3SB/Q+lQl9U2+c2YP+cP6Cf8z YX7g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722003450; x=1722608250; 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=VBPoJGRK9mlLOjG0zJWUaBmONDbieXNm5VnkemOp2+A=; b=m1SyBB639kSPgB9TopqMcjQ3bceEiEV+sKwCJq8BPNEBN7jRue38UQ6Tqty2+aG+UC mcb2oga/U/v3MhQt2NBz4e9IhBfHm2sQKZJrJaV6hlu5rcWAGoXl83xniAoghRy1+YPW fEpjWVcQWyEkqzn8J23HY+Olb3zSCb/EQT3bUCZxnLBKvDqb4p1p5SDQ8RWcPqDnTMcX aPf64Nm1ddP4fzD7WuT6Jg4aC9aDQ1e4y+BoxnSM4LjQl+q7vJjbLfeiSD4zckhPBnlG VVyINrAFc+Idls54ZyfOJoMK6BEaKgk55mffZ1gXsr/L8sY0jDsyByWMZvYHJQfNwCvD RcEA== X-Gm-Message-State: AOJu0Yz10/rKg2b0g5Cg92ybHqsvbMwi+McOWOo7kHxO4FfHCkPaykR8 qFvI+shuL1SYoz7xNNYJmrBo6FZxhiylKmmqsddY3ZfVwMdSLFljruA6Bg== X-Google-Smtp-Source: AGHT+IFHKoslN/9z8QzVoHOz7S5NcdTctDG0slIVMMTnY+hJ0DLsVdy6UUE//5S/8a6R90GJ50RA9Q== X-Received: by 2002:a05:6870:e40d:b0:25c:b3c9:ecda with SMTP id 586e51a60fabf-264a0eaf215mr7158798fac.38.1722003449863; Fri, 26 Jul 2024 07:17:29 -0700 (PDT) Received: from [172.31.0.109] ([136.36.72.243]) by smtp.gmail.com with ESMTPSA id 586e51a60fabf-2653e7d4b35sm691938fac.33.2024.07.26.07.17.28 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 26 Jul 2024 07:17:28 -0700 (PDT) Message-ID: <424b5d69-1f79-4e7e-950f-bef19a08e343@gmail.com> Date: Fri, 26 Jul 2024 08:17:27 -0600 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Content-Language: en-US From: Jeff Law Subject: [to-be-committed] [RISC-V][target/116085] Fix rv64 minmax extension avoidance splitter To: "gcc-patches@gcc.gnu.org" X-Spam-Status: No, score=-8.5 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 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 pr116085 is a long standing (since late 2022) regression on the riscv port. A patch introduced a pattern to avoid unnecessary extensions when doing a min/max operation where one of the values is a 32 bit positive constant. > (define_insn_and_split "*minmax" > [(set (match_operand:DI 0 "register_operand" "=r") > (sign_extend:DI > (subreg:SI > (bitmanip_minmax:DI (zero_extend:DI (match_operand:SI 1 "register_operand" "r")) > (match_operand:DI 2 "immediate_operand" "i")) > 0))) > (clobber (match_scratch:DI 3 "=&r")) > (clobber (match_scratch:DI 4 "=&r"))] > "TARGET_64BIT && TARGET_ZBB && sext_hwi (INTVAL (operands[2]), 32) >= 0" > "#" > "&& reload_completed" > [(set (match_dup 3) (sign_extend:DI (match_dup 1))) > (set (match_dup 4) (match_dup 2)) > (set (match_dup 0) (:DI (match_dup 3) (match_dup 4)))] Lots going on in here. The key is the nonconstant value is zero extended from SI to DI in the original RTL and we know the constant value is unchanged if we were to sign extend it from 32 to 64 bits. We change the extension of the nonconstant operand from zero to sign extension. I'm pretty confident the goal there is take advantage of the fact that SI values are kept sign extended and will often be optimized away. The problem occurs when the nonconstant operand has the SI sign bit set. As an example: smax (0x8000000, 0x7) resulting in 0x80000000 The split RTL will generate smax (sign_extend (0x80000000), 0x7)) smax (0xffffffff80000000, 0x7) resulting in 0x7 Opps. We really needed to change the opcode to umax for this transformation to work. That's easy enough. But there's further improvements we can make. First the pattern is a define_and_split with a post-reload split condition. It would be better implemented as a 4->3 define_split so that the costing model just works. Second, if operands[1] is a suitably promoted subreg, then we can elide the sign extension when we generate the split code, so often it'll be a 4->2 split, again with the cost model working with no adjustments needed. Tested on rv32 and rv64 in my tester. I'll wait for the pre-commit tester to spin it as well. Jeff PR target/116085 gcc/ * config/riscv/bitmanip.md (minmax extension avoidance splitter): Rewrite as a simpler define_split. Adjust the opcode appropriately. Avoid emitting sign extension if it's clearly not needed. * config/riscv/iterators.md (minmax_optab): Rename to uminmax_optab and map everything to unsigned variants. gcc/testsuite/ * gcc.target/riscv/pr116085.c: New test. diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md index 9fc5215d6e3..e8876bf3c94 100644 --- a/gcc/config/riscv/bitmanip.md +++ b/gcc/config/riscv/bitmanip.md @@ -549,23 +549,34 @@ (define_insn "*3" ;; Optimize the common case of a SImode min/max against a constant ;; that is safe both for sign- and zero-extension. -(define_insn_and_split "*minmax" - [(set (match_operand:DI 0 "register_operand" "=r") +(define_split + [(set (match_operand:DI 0 "register_operand") (sign_extend:DI (subreg:SI - (bitmanip_minmax:DI (zero_extend:DI (match_operand:SI 1 "register_operand" "r")) - (match_operand:DI 2 "immediate_operand" "i")) + (bitmanip_minmax:DI (zero_extend:DI (match_operand:SI 1 "register_operand")) + (match_operand:DI 2 "immediate_operand")) 0))) - (clobber (match_scratch:DI 3 "=&r")) - (clobber (match_scratch:DI 4 "=&r"))] + (clobber (match_operand:DI 3 "register_operand")) + (clobber (match_operand:DI 4 "register_operand"))] "TARGET_64BIT && TARGET_ZBB && sext_hwi (INTVAL (operands[2]), 32) >= 0" - "#" - "&& reload_completed" - [(set (match_dup 3) (sign_extend:DI (match_dup 1))) - (set (match_dup 4) (match_dup 2)) - (set (match_dup 0) (:DI (match_dup 3) (match_dup 4)))] - "" - [(set_attr "type" "bitmanip")]) + [(set (match_dup 0) (:DI (match_dup 4) (match_dup 3)))] + " +{ + /* Load the constant into a regsiter. */ + emit_move_insn (operands[3], operands[2]); + + /* Sign extend operands[1] if it isn't extended already. */ + /* If operands[1] is a sign extended SUBREG, then we can use it + directly. Otherwise extend it into another temporary. */ + if (SUBREG_P (operands[1]) + && SUBREG_PROMOTED_VAR_P (operands[1]) + && SUBREG_PROMOTED_SIGNED_P (operands[1])) + operands[4] = SUBREG_REG (operands[1]); + else + emit_move_insn (operands[4], gen_rtx_SIGN_EXTEND (DImode, operands[1])); + + /* The minmax is actually emitted from the split pattern. */ +}") ;; ZBS extension. diff --git a/gcc/config/riscv/iterators.md b/gcc/config/riscv/iterators.md index 734da041f0c..0a669f560e3 100644 --- a/gcc/config/riscv/iterators.md +++ b/gcc/config/riscv/iterators.md @@ -327,10 +327,11 @@ (define_code_attr atomic_optab [(plus "add") (ior "or") (xor "xor") (and "and")]) ; bitmanip code attributes -(define_code_attr minmax_optab [(smin "smin") - (smax "smax") - (umin "umin") - (umax "umax")]) +;; Unsigned variant of a min/max optab. +(define_code_attr uminmax_optab [(smin "umin") + (smax "umax") + (umin "umin") + (umax "umax")]) (define_code_attr bitmanip_optab [(smin "smin") (smax "smax") (umin "umin") diff --git a/gcc/testsuite/gcc.target/riscv/pr116085.c b/gcc/testsuite/gcc.target/riscv/pr116085.c new file mode 100644 index 00000000000..998d82bd235 --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/pr116085.c @@ -0,0 +1,29 @@ +/* { dg-do run } */ +/* { dg-require-effective-target rv64 } */ +/* { dg-options "-march=rv64gc_zbb -mabi=lp64d -fno-ext-dce" } */ + +extern void abort (void); + +int a = 2; +unsigned b = 0x80000000; +int arr_5[2][23]; +void test(int, unsigned, int); +int main() { + test(a, b, 1); + if (arr_5[1][0] != -2147483648) + abort (); + return 0; +} + +#define c(a, b) \ + ({ \ + long d = a; \ + long e = b; \ + d > e ? d : e; \ + }) +__attribute__((noipa)) +void test(int f, unsigned g, int h) { + for (int i = 0; i < h; i = f) + arr_5[1][i] = h ? c(g, 7) : 0; +} +