From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from server.nextmovesoftware.com (server.nextmovesoftware.com [162.254.253.69]) by sourceware.org (Postfix) with ESMTPS id 3D2703858C3A for ; Tue, 21 Sep 2021 12:54:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3D2703858C3A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=nextmovesoftware.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=nextmovesoftware.com DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=nextmovesoftware.com; s=default; h=Content-Transfer-Encoding:Content-Type: MIME-Version:Message-ID:Date:Subject:In-Reply-To:References:Cc:To:From:Sender :Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help: List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=g7o3DJm6YQ0Xa6oM3vO2Xrg2Lc0GH6WM9PiR5HN5qbA=; b=A3lar0cr2h9Cvc8zJUyWpsjGoY FWSLzp7CQf9P+FNi4oqJEqEChneb8a4twO6ETfFXIpVY5v7PbL0p7Ts1jGdGxSmOPfc16If/CCT24 7VABG876YVdb6Cr9j2uQhi7VZApMCS9i5nx0KNCktSjGCofC8FoMBzyYwIPxpQ1heG1B+bD/90ycl ooL7jy0ME/n59LjaBUI1251WV+kjB5L/Ozy+3gs1OUyjESlGXKMLDZg/JXCtpfSyB68HnH7LWeI8d TK9Qvhdn7waGILlif3ck4FIRIQrfBiw2gfB1/KC1tEp2M5YDJWcgRHHvCJkoT01/t5FroZ6iiMOr8 muLDd2uA==; Received: from [185.62.158.67] (port=49815 helo=Dell) by server.nextmovesoftware.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1mSfHx-0001zM-M3; Tue, 21 Sep 2021 08:54:21 -0400 From: "Roger Sayle" To: "'Richard Sandiford'" , "'GCC Patches'" Cc: "'Segher Boessenkool'" , "'Richard Biener'" References: <001401d7a2a5$5bf07db0$13d17910$@nextmovesoftware.com> <20210906101414.GT1583@gate.crashing.org> In-Reply-To: Subject: RE: [PATCH] Simplify paradoxical subreg extensions of TRUNCATE Date: Tue, 21 Sep 2021 13:54:18 +0100 Message-ID: <008201d7aee7$cc6357e0$652a07a0$@nextmovesoftware.com> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQKP/FEQ3FZzz8P1ySolfR2YJ5xdUgDf4mpQAk055+QBSUFqTKoaHKig Content-Language: en-gb X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - server.nextmovesoftware.com X-AntiAbuse: Original Domain - gcc.gnu.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - nextmovesoftware.com X-Get-Message-Sender-Via: server.nextmovesoftware.com: authenticated_id: roger@nextmovesoftware.com X-Authenticated-Sender: server.nextmovesoftware.com: roger@nextmovesoftware.com X-Source: X-Source-Args: X-Source-Dir: X-Spam-Status: No, score=-6.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Sep 2021 12:54:23 -0000 That define_insn is making my eyes bleed! I think that's the most = convincing argument I've ever read on gcc-patches, and I can see now what Segher is = so opposed to. My new goal is to add sh_mul and uh_mul as RTL expression codes for highpart multiplication, so that that pattern will go away = [and never ever been seen again]. Fortunately, the proposed patch is part of the solution, rather than = part of the problem. simplify_subreg is used to find a sensible alternate form = for a hypothetical SUBREG before it's created (and normally it'll only be = created if it's valid c.f. validate_subreg in simplify_gen_subreg). This = function is called over 30 million times during stage2/stage3 of a bootstrap on x86_64, = with the most common "SUBREG" operand RTX codes being: 16160294 reg 5627157 const_int 3278692 mem 960839 lshiftrt 902685 plus 690246 and 430131 subreg 319407 zero_extract 319145 minus 255043 value 232959 ashift 225468 mult 217819 xor 207645 clobber 207454 ashiftrt 100213 ior Adding a simplify_subreg for mult, replacing them with a highpart rtx = should avoid (reduce) combine's Frankenstein patterns appearing in machine = descriptions. Presumably, the reason that this define_insn starts with a "*" even for = a recognized named pattern, is that if expand ever attempted emitting this = instruction, the internal sanity checks for a sane/valid SUBREG would fail/ICE. p.s. You're right that there's a temptation to add ugly patterns to a = backend, instead of fixing their simplification in the middle-end; but only when = backend patches get reviewed faster/easier than middle-end patches =F0=9F=98=8A Awesome (counter-)example! Roger -- -----Original Message----- From: Richard Sandiford =20 Sent: 21 September 2021 13:02 To: Richard Biener via Gcc-patches Cc: Segher Boessenkool ; Richard Biener = ; Roger Sayle Subject: Re: [PATCH] Simplify paradoxical subreg extensions of TRUNCATE [Using this is a convenient place to reply to the thread as a whole] Richard Biener via Gcc-patches writes: > On Mon, Sep 6, 2021 at 12:15 PM Segher Boessenkool=20 > wrote: >> >> On Sun, Sep 05, 2021 at 11:28:30PM +0100, Roger Sayle wrote: >> > This patch simplifies the RTX (subreg:HI (truncate:QI (reg:SI))) as = >> > (truncate:HI (reg:SI)), and closely related variants. >> >> Subregs of other than regs are undefined in RTL. You will first have = >> to define this (in documentation as well as in other code that=20 >> handles subregs). I doubt this is possible to do, subreg have so=20 >> many overloaded meanings already. > > I suppose (subreg:MODE1 (xyz:MODE2 ..)) where xyz is not REG or MEM is = > equal to > > (set (reg:MODE2) (xyz:MODE2 ..)) > (subreg:MODE1 (reg:MODE2) ...) > > with 'reg' being a pseudo reg is the (only?) sensible way of defining = it. Agreed. And I think that's already the de facto definition (and has = been for a long time). Subreg as an operation has to have defined = semantics for all the cases that simplify_subreg handles, otherwise we = have GIGO and a lot of the function could be deleted. We can (and do) = choose to prevent some of those operations becoming actual rtxes, but = even there, the de facto rules are quite broad. E.g.: - Like you said later, simplify_gen_subreg is opt-out rather than opt-in in terms of the subreg rtxes that it's prepared to create. - Even rs6000.md has: (define_insn "*mul3_highpart" [(set (match_operand:GPR 0 "gpc_reg_operand" "=3Dr") (subreg:GPR (mult: (any_extend: (match_operand:GPR 1 "gpc_reg_operand" = "r")) (any_extend: (match_operand:GPR 2 "gpc_reg_operand" = "r"))) 0))] "WORDS_BIG_ENDIAN && !(mode =3D=3D SImode && = TARGET_POWERPC64)" "mulh %0,%1,%2" [(set_attr "type" "mul") (set_attr "size" "")]) Many other ports have similar patterns. The problem with =E2=80=9Ccombine can generate invalid rtl but backends = should reject it=E2=80=9D is that, generally, people write combine = patterns by looking at what combine _wants_ to generate and then writing = .md patterns to match that. In other words, combine in practice defines = the (de facto) correct rtl representation of a combined sequence. Given: Trying 10 -> 15: 10: r29:QI=3Dtrunc(r32:SI) REG_DEAD r32:SI 15: r38:HI=3Dr29:QI#0 REG_DEAD r29:QI Failed to match this instruction: (set (reg:HI 38) (subreg:HI (truncate:QI (reg:SI 32)) 0)) I'm sure there's a temptation to add an .md pattern that matches the = subreg. :-) Thanks, Richard > That would make the simplification apply when substituting the pseudos = > definition inside the subreg which maybe is what this targets? > > Richard. > >> >> >> Segher