From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x632.google.com (mail-pl1-x632.google.com [IPv6:2607:f8b0:4864:20::632]) by sourceware.org (Postfix) with ESMTPS id 5CCED3858D28 for ; Sun, 5 Mar 2023 19:03:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5CCED3858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-pl1-x632.google.com with SMTP id a2so7980454plm.4 for ; Sun, 05 Mar 2023 11:03:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=HdtMF5cqikZPppZF2L8BCiKiBR+5M6XeGduZ7hQMWko=; b=SioYYbWgx0adqujLtvPT30juNlW7ZXremsiejPBY73/ip7vbgoS3dse1oMhhfPkH0s s1MtmvCSADx5/dcTTi5ByGWSEC7mEbgeGijDsY0uPZfkjcoiPdJuw2A4HKuwrqPKCFua oxYiyrApol738lD9Y3Hu3igvRGUUe2a6J9RkKfiti/LPONNHg2YKt5UuZ4Ew22mwT7xZ J/oD3WtUuxoLpiBnsGjLR2c2md7+XXdbZ0SwFoNqaT9g1xA/IOQqw4hwnCkfMx7MFm4O BZYa9d/rxxKLZkMqOGc9QdYhJQOb3Drvry5KHceBTEomMfqHuQUsATPad++BlwkUoUao tNnA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=HdtMF5cqikZPppZF2L8BCiKiBR+5M6XeGduZ7hQMWko=; b=F9LyN9NQTmnGgercqmd/SXMrL4PKEqNpr6dZaIpaEfgI+CsSFdie3oY+iDAeBGF60q 5FS7wa4nhD5S8VdUqVQ7LnjGl1kWejFsPMOruz0eqDZPeU9yaLm4mb3/A5kgA0DtqlyG PWbWhlGgJDXnafQ2ABHT92lTAkuPph5JabmhSV0NiugHK/ina8E8rum0Fm6EqPyBuhX2 Yanv7e2Hma+T4i2IW1jQwBnOfUbXKvmL2lP8ECsAiDXnO4gCgmu7ihPAFeb8LzXalRN4 yd2D6c4A9IzBnST12Go3oRyRtCjtEqpqqkxpfkvvVSqIAkbVHfpIj0QaiKF8jhX3sh5j p8fg== X-Gm-Message-State: AO0yUKVVl0WVfFwWJ/qwxteHrLdki8BKDoHUDoAtwVuV4Ja90iV6We90 tlQC64WpQpRRLE3R/qLXHHkDXygGvwDZ31BuQNQ= X-Google-Smtp-Source: AK7set/tVLYswoTlB5YHLVSpVXxcFSKCiiY1rpnluEgT7bCI3Bx82GbWtryPR1afWsU/msQ7PIqk6+aR63Yq1CqZv6w= X-Received: by 2002:a17:90a:c257:b0:233:bada:17b5 with SMTP id d23-20020a17090ac25700b00233bada17b5mr3123922pjx.4.1678043026070; Sun, 05 Mar 2023 11:03:46 -0800 (PST) MIME-Version: 1.0 References: <20221209182510.43515-1-rzinsly@ventanamicro.com> <41ba8a45-b4a6-aaf4-1b7a-6e7a7d2dec4f@gmail.com> In-Reply-To: <41ba8a45-b4a6-aaf4-1b7a-6e7a7d2dec4f@gmail.com> From: Andrew Pinski Date: Sun, 5 Mar 2023 11:03:34 -0800 Message-ID: Subject: Re: [PATCH v2] RISC-V: Produce better code with complex constants [PR95632] [PR106602] To: Jeff Law Cc: gcc-patches@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-7.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 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, Mar 5, 2023 at 10:14=E2=80=AFAM Jeff Law via Gcc-patches wrote: > > > > On 2/23/23 14:23, Andrew Pinski via Gcc-patches wrote: > > On Fri, Dec 9, 2022 at 10:25 AM Raphael Moreira Zinsly > > wrote: > >> > >> Changes since v1: > >> - Fixed formatting issues. > >> - Added a name to the define_insn_and_split pattern. > >> - Set the target on the 'dg-do compile' in pr106602.c. > >> - Removed the rv32 restriction in pr95632.c. > >> > >> -- >8 -- > >> > >> Due to RISC-V limitations on operations with big constants combine > >> is failing to match such operations and is not being able to > >> produce optimal code as it keeps splitting them. By pretending we > >> can do those operations we can get more opportunities for > >> simplification of surrounding instructions. > >> > >> 2022-12-06 Raphael Moreira Zinsly > >> Jeff Law > >> > >> gcc/Changelog: > >> PR target/95632 > >> PR target/106602 > >> * config/riscv/riscv.md: New pattern to simulate complex > >> const_int loads. > >> > >> gcc/testsuite/ChangeLog: > >> * gcc.target/riscv/pr95632.c: New test. > >> * gcc.target/riscv/pr106602.c: New test. > >> --- > >> gcc/config/riscv/riscv.md | 15 +++++++++++++++ > >> gcc/testsuite/gcc.target/riscv/pr106602.c | 14 ++++++++++++++ > >> gcc/testsuite/gcc.target/riscv/pr95632.c | 15 +++++++++++++++ > >> 3 files changed, 44 insertions(+) > >> create mode 100644 gcc/testsuite/gcc.target/riscv/pr106602.c > >> create mode 100644 gcc/testsuite/gcc.target/riscv/pr95632.c > >> > >> diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md > >> index df57e2b0b4a..b0daa4b19eb 100644 > >> --- a/gcc/config/riscv/riscv.md > >> +++ b/gcc/config/riscv/riscv.md > >> @@ -1667,6 +1667,21 @@ > >> MAX_MACHINE_MODE, &operands[3], TRUE); > >> }) > >> > >> +;; Pretend to have the ability to load complex const_int in order to = get > >> +;; better code generation around them. > >> +(define_insn_and_split "*mvconst_internal" > >> + [(set (match_operand:GPR 0 "register_operand" "=3Dr") > >> + (match_operand:GPR 1 "splittable_const_int_operand" "i"))] > >> + "cse_not_expected" > > > > This is just way broken. This should be combined with the normal move > > instructions and just be a define_split. > > See PR 108892 for a testcase which shows this breaking how the > > register allocator thinks it should work. > I'm pretty sure that won't work. You need them exposed as a define_insn > so that they can act as a bridge pattern for combine. You don't want to > expose before combine as that'll regress things in a variety of other > ways. You don't want the bridge form to survive after splitting. Hence > define_insn_and_split. > > I haven't looked at that bug in detail, but Raphael and I certainly will. So the register allocator does not know how to handle if there are two different patterns which are to be used but differ by constraints/predicats. This is especially true for mov instructions which this is. What I am saying is the "*movdi_64bit" and "*movsi_internal" patterns should handle the same instruction as the above and still have a define_split. Take a look at how aarch64 handles this here. It has one pattern for the move but it is a define_insn_and_split still. This is explicitly to handle the case you are doing really. "*movsi_aarch64" and "*movdi_aarch64" . Thanks, Andrew Pinski > > jeff