From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x52c.google.com (mail-ed1-x52c.google.com [IPv6:2a00:1450:4864:20::52c]) by sourceware.org (Postfix) with ESMTPS id 313A13858C60 for ; Thu, 21 Oct 2021 10:20:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 313A13858C60 Received: by mail-ed1-x52c.google.com with SMTP id ec8so298422edb.6 for ; Thu, 21 Oct 2021 03:20:16 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=AR1MgFfJdsqkmD/LU/ib5WTtO+r5VY+ujy2LZRSuwyY=; b=n4hTeOuUBPeNJbDXbqzviDjAPh4qc1av75KraHom/1R17Pj6/mT8QxvMiHPL9I2qJ6 8BRpC0JqQ3cbh/rNLGKg3fxWE+xl71qyeK69waEXrBFXAz67CO0wgwYalXbChpraiJP2 E6IgGNlg5Y+xoFVAsAq00Mw2RSV+0/k3K7R1ErbzRsiAKPCSvmzjVPkJrD2kT6AGPW6T EwONYdYa71EKdHzsytVp4ME4ubNyj/GAJTGbVY3WEJUaLyuitjt8IB09+/TXzPYg8nss aukcjRp31N9z1kyW+drMXfLwxuCM9P/39zQoPU1EXAyJcz6vq6AK/wlUfm3Nyh5+QIyd u7pA== X-Gm-Message-State: AOAM531aSSeQm/6UCEmYpa0uiVl6CxJatI56ZLlcxiIV7Xp18E4B9Hha u4kC/O7NYc4A7+l+Rg1sKZws935PLsCfMJTm5RM= X-Google-Smtp-Source: ABdhPJwyPITLlNnR/UU358D68ksKWu4PVXfulytAFE7MIhuJMNBpwiQD0jJ3s9UCon8G0Qd2EIY9+AlPrKIxKHGZSzk= X-Received: by 2002:a17:907:3eaa:: with SMTP id hs42mr6061661ejc.429.1634811615032; Thu, 21 Oct 2021 03:20:15 -0700 (PDT) MIME-Version: 1.0 References: <20211008151222.37790-1-aldyh@redhat.com> <49371bdc-edd7-a5fa-4d75-9b2ed51478ad@gmail.com> <88f397a8-78d1-fe74-c221-e846072edff1@redhat.com> <49b035a6-7160-07d9-0c34-4accdebc1af4@gmail.com> In-Reply-To: <49b035a6-7160-07d9-0c34-4accdebc1af4@gmail.com> From: Richard Biener Date: Thu, 21 Oct 2021 12:20:03 +0200 Message-ID: Subject: Re: [PATCH] Convert strlen pass from evrp to ranger. To: Jeff Law Cc: Aldy Hernandez , Jakub Jelinek , GCC patches , Andrew MacLeod Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, 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: Thu, 21 Oct 2021 10:20:18 -0000 On Wed, Oct 20, 2021 at 10:58 PM Jeff Law wrote: > > > > On 10/18/2021 2:17 AM, Aldy Hernandez wrote: > > > > > > On 10/18/21 12:52 AM, Jeff Law wrote: > >> > >> > >> On 10/8/2021 9:12 AM, Aldy Hernandez via Gcc-patches wrote: > >>> The following patch converts the strlen pass from evrp to ranger, > >>> leaving DOM as the last remaining user. > >> So is there any reason why we can't convert DOM as well? DOM's use > >> of EVRP is pretty limited. You've mentioned FP bits before, but my > >> recollection is those are not part of the EVRP analysis DOM uses. > >> Hell, give me a little guidance and I'll do the work... > > > > Not only will I take you up on that offer, but I can provide 90% of > > the work. Here be dragons, though (well, for me, maybe not for you ;-)). > > > > DOM is actually an evrp pass at -O1 in disguise. The reason it really > > is a covert evrp pass is because: > > > > a) It calls extract_range_from_stmt on each statement. > > > > b) It folds conditionals with simplify_using_ranges. > > > > c) But most importantly, it exports discovered ranges when it's done > > (evrp_range_analyzer(true)). > > > > If you look at the evrp pass, you'll notice that that's basically what > > it does, albeit with the substitute and fold engine, which also calls > > gimple fold plus other goodies. > > > > But I could argue that we've made DOM into an evrp pass without > > noticing. The last item (c) is particularly invasive because these > > exported ranges show up in other passes unexpectedly. For instance, I > > saw an RTL pass at -O1 miss an optimization because it was dependent > > on some global range being set. IMO, DOM should not export global > > ranges it discovered during its walk (do one thing and do it well), > > but I leave it to you experts to pontificate. > All true. But I don't think we've got many, if any, hard dependencies > on those behaviors. > > > > > The attached patch is rather trivial. It's mostly deleting state. It > > seems DOM spends a lot of time massaging the IL so that it can fold > > conditionals or thread paths. None of this is needed, because the > > ranger can do all of this. Well, except floats, but... > Massaging the IL should only take two forms IIRC. > > First, if we have a simplification we can do. That could be const/copy > propagation, replacing an expression with an SSA_NAME or constant and > the like. It doesn't massage the IL just to massage the IL. > > Second, we do temporarily copy propagate the current known values of an > SSA name into use points and then see if that allows us to determine if > a statement is already in the hash tables. But we undo that so that > nobody should see that temporary change in state. Are you sure we still do that? I can't find it at least. > > Finally, it does create some expressions & statements on the fly to > enter them into the tables. For example, if it sees a store, it'll > create a load with the source & dest interchanged and enter that into > the expression table. But none of this stuff ever shows up in the IL. > It's just to create entries in the expression tables. > > So ITSM the only real concern would be if those temporary const/copy > propagations were still in the IL and we called back into Ranger and it > poked at that data somehow. > > > > That's the good news. The bad news is that DOM changes the IL as it > > goes and the patch doesn't bootstrap. Andrew insists that we should > > work even with DOM's changing IL, but last time we played this dance > > with the substitute_and_fold engine, there were some tweaks needed to > > the ranger. Could be this, but I haven't investigated. It could also > > be that the failures I was seeing were just DOM things that were no > > longer needed (shuffling the IL to simplify things for evrp). > So if we're referring to those temporary const/copy propagations > "escaping" into Ranger, then I would fully expect that to cause > problems. Essentially they're path sensitive const/copy propagations > and may not be valid on all the paths through the CFG to the statement > where the propagation occurs > > > > > > > This just needs a little shepherding from a DOM expert ;-). If you > > get it to bootstrap, I could take care of the tests, performance, and > > making sure we're getting the same number of threads etc. > > > >> > >>> > >>> No additional cleanups have been done. For example, the strlen pass > >>> still has uses of VR_ANTI_RANGE, and the sprintf still passes around > >>> pairs of integers instead of using a proper range. Fixing this > >>> could further improve these passes. > >>> > >>> As a further enhancement, if the relevant maintainers deem useful, > >>> the domwalk could be removed from strlen. That is, unless the pass > >>> needs it for something else. > >> The dom walk was strictly for the benefit of EVRP when it was added. > >> So I think it can get zapped once the pass is converted. > > > > Jakub mentioned a while ago, that the strlen pass itself needs DOM, so > > perhaps this needs to stay. > Yes, strlen wants DOM. But the printf bits don't really need it. Well, > maybe they do now that the printf warnings are more tightly integrated > with the strlen bits. > > > jeff >