From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-x529.google.com (mail-pg1-x529.google.com [IPv6:2607:f8b0:4864:20::529]) by sourceware.org (Postfix) with ESMTPS id 742983858D28 for ; Mon, 7 Aug 2023 14:45:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 742983858D28 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-pg1-x529.google.com with SMTP id 41be03b00d2f7-55c993e26ffso2659975a12.0 for ; Mon, 07 Aug 2023 07:45:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=vrull.eu; s=google; t=1691419522; x=1692024322; 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=xRdKdXQzBjTMZA26ViNdboLk392+tbBTbkOHY5myCd8=; b=eFCuU+zDTSIFFCg4uA0oDZoalU6S7EZYm/mQk3X1LrDjQtD5OlYgd8Bfhgl2ob9lBW 7FowvGGdZY1FIhJKLjuHpoAVchBbH8vl/ELADs6tG5TnjbPL9AfNMRM9GCLHmGxb7Eqz wgUzwWiTZjIgz16SN0+t7JbhsyPLKlj7lXnkSgriHfxHopMTVvLPlWXCcmNvesm11lmd 3F/rU5qjgYctEk6w6aufGBKQ2BpQJbkKQNkvsJN04ldgTbO+qlEsc2Xs23NVaqTVxHUz qAH0ghGrcBLhwP53TYG0cGcMPcoQTsGDBtU91YdPgC8iUKyBDqvKXqWXzrjVvGlDh0K8 zDJw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691419522; x=1692024322; 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=xRdKdXQzBjTMZA26ViNdboLk392+tbBTbkOHY5myCd8=; b=gfKAJ1NYbtPF3jb8La3p+yzITvM3UE2L24CC90l3TZyU4bWMS7s1IcVgRQrtVyeldm J1WNozmrh++DeI4FeNKo/l8QKaROC30fudPDAgLS7W4fjeTG/2EfJ/9C/31HOkyZiYQS G9pwyFb1HWdIZ9Tt6oLwZFUKvhOTHfx6vj6NoIRULJN8OGPPe1N/rirIayWdjQ+VB8JL Jvc11vhjaSYWJv05KlWEnZCsM35UElh12E8KoMJyOgMwlM1T1rI3Qo2qyJSNs+w83Y+2 iU9G+957uPrvCurf/jPcpElWNfgSzspU5atoJkR+VBdrCnzYqOmn2Fk0FTkTTHL6+urS Ru+w== X-Gm-Message-State: AOJu0Yz3TJqLgq4/tEVG7k+1RhLCzaxIkgb4d6egVOA121zJVIy3DA14 dc8xmXzbslUr7/cPHFjaFeS01mOH1Rd96aOLTin6MQ== X-Google-Smtp-Source: AGHT+IFTVaF6BCjhG8LPO1SIhpahmjYCLjZCb7K6bil+sotVwDBC/m0jXzm5unTmBkN2acxt3bTJWyYP3NrqWJa3oCg= X-Received: by 2002:a17:90b:100e:b0:268:7be6:29a5 with SMTP id gm14-20020a17090b100e00b002687be629a5mr8229728pjb.9.1691419522118; Mon, 07 Aug 2023 07:45:22 -0700 (PDT) MIME-Version: 1.0 References: <20230713141336.3950751-1-manolis.tsamis@vrull.eu> <1420ed26-9c30-b09e-f45c-54c89516814e@gmail.com> <59f02830-eae9-6a52-dd63-48a7217905ef@rivosinc.com> <93fd5732-145a-94ab-5a9e-5a2eb8bac886@gmail.com> <33394d74-60c6-dd0e-55a0-0e0901f05e2e@rivosinc.com> <43afd736-4980-2d14-67a2-4425acf6d523@gmail.com> In-Reply-To: <43afd736-4980-2d14-67a2-4425acf6d523@gmail.com> From: Manolis Tsamis Date: Mon, 7 Aug 2023 17:44:46 +0300 Message-ID: Subject: Re: [PATCH v3] Implement new RTL optimizations pass: fold-mem-offsets. To: Jeff Law Cc: Vineet Gupta , gcc-patches@gcc.gnu.org, Philipp Tomsich , Richard Biener , Jakub Jelinek , gnu-toolchain Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-2.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,JMQ_SPF_NEUTRAL,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: I have sent out a new v4 of this (https://gcc.gnu.org/pipermail/gcc-patches/2023-August/626502.html). In the new version I both restore the INSN_CODE as mentioned here and I also call recog when I commit the offset change (because there may be a change from no offset to some offsets). I have also removed the driver function as per Vineet's suggestion. Last thing I have fixed a nasty bug with REG_EQUIV's that resulted in failing bootstrap on AArch64. The offending sequence looked like this (in 'simplified RTX'): (set (reg/f:DI 3 x3) (plus:DI (reg/f:DI 2 x2) (const_int 8 [0x8]))) (set (reg/f:DI 19 x19) (plus:DI (reg/f:DI 3 x3) (reg:DI 19 x19))) ... (set (reg:TI 2 x2) (mem:TI (reg/f:DI 0 x0))) (expr_list:REG_DEAD (reg/f:DI 0 x0) (expr_list:REG_EQUIV (mem:TI (reg/f:DI 19 x19)) (nil))) (set (mem:TI (reg/f:DI 19 x19)) (reg:TI 2 x2)) (expr_list:REG_DEAD (reg/f:DI 19 x19) (expr_list:REG_DEAD (reg:TI 2 x2) (nil))) Were the first instruction (x3 =3D x2 + 8) was folded in the address calculation of the last one, resulting in (mem:TI (plus:DI (reg/f:DI 19 x19) (const_int 8 [0x8])), but then the previous REG_EQUIV was incorrect due to the modified runtime value of x19. For now I opted to treat REG_EQ/EQUIV notes as uses that we cannot handle, so if any of them are involved we don't fold offsets. Although it would may be possible to improve this in the future, I think it's fine for now and the reduction of folded insns is a small %. I have tested v4 with a number of benchmarks and large projects on x64, aarch64 and riscv64 without observing any issues. The x86 testsuite issue still exists as I don't have a satisfactory solution to it yet. Any feedback for the changes is appreciated! Thanks, Manolis On Fri, Jul 21, 2023 at 12:59=E2=80=AFAM Jeff Law w= rote: > > > > On 7/20/23 00:18, Vineet Gupta wrote: > > > > > > On 7/18/23 21:31, Jeff Law via Gcc-patches wrote: > >>> > >>> In a run with -fno-fold-mem-offsets, the same insn 93 is successfully > >>> grok'ed by cprop_hardreg, > >>> > >>> | (insn 93 337 522 11 (set (mem/c:DF (plus:DI (reg/f:DI 2 sp) > >>> | (const_int 8 [0x8])) [4 %sfp+-8 S8 A64]) > >>> | (const_double:DF 0.0 [0x0.0p+0])) "sff.i":23:11 190 > >>> {*movdf_hardfloat_rv64} > >>> ^^^^^^^^^^^^^^^ > >>> | (expr_list:REG_EQUAL (const_double:DF 0.0 [0x0.0p+0]) > >>> | (nil))) > >>> > >>> P.S. I wonder if it is a good idea in general to call recog() post > >>> reload since the insn could be changed sufficiently to no longer > >>> match the md patterns. Of course I don't know the answer. > >> If this ever causes a problem, it's a backend bug. It's that simple. > >> > >> Conceptually it should always be safe to set INSN_CODE to -1 for any > >> insn. > > > > Sure the -1 should be handled, but are you implying that f-mo- will > > always generate a valid combination and recog() failing is simply a bug > > in backend and/or f-m-o. If not, the -1 setting can potentially trigger > > an ICE in future. > A recog failure after setting INSN_CODE to -1 would always be an > indicator of a target bug at the point where f-m-o runs. > > In that would be generally true as well. There are some very obscure > exceptions and those exceptions are for narrow periods of time. > > > > > > >> > >> Odds are for this specific case in the RV backend, we just need a > >> constraint to store 0.0 into a memory location. That can actually be > >> implemented as a store from x0 since 0.0 has the bit pattern 0x0. This > >> is probably a good thing to expose anyway as an optimization and can > >> move forward independently of the f-m-o patch. > > > > I call dibs on this :-) Seems like an interesting little side project. > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D110748 > It's yours :-) > > jeff