From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 14AF73857C65 for ; Thu, 21 Oct 2021 13:30:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 14AF73857C65 Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-596-w6--jgK8MneFxwKG_w4Lhw-1; Thu, 21 Oct 2021 09:30:13 -0400 X-MC-Unique: w6--jgK8MneFxwKG_w4Lhw-1 Received: by mail-wr1-f69.google.com with SMTP id l8-20020a5d6d88000000b001611b5de796so187571wrs.10 for ; Thu, 21 Oct 2021 06:30:13 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=biPxc0VUJM6JkCiiZzQVGDJWlICNxoMaCKdVqMYfDhw=; b=uCwccYWv4cMw/OLjNkmtUFF58mayFY3Sm5bU61uhVL7fFQwb/Bd5FjC0o+oXdZRs7/ uDaEEq+nzVreL4hdVf/fYmXoUGWL4VhJUeIL3zrEf/kIN+d0nEJhFKgcpK2NnI/r6RZy O6bBhSAz6rTK/bcgUJ4SozbQQJfxduPVEvFKlLErGwKJLkDTtSEFwMa/cPeLTRoskjMZ YzokyrLiJaikfXVzpA97P47FTYswor9ScvleLtX092bqVX3TSmY9inQTDsfHTCNdsso+ ADQET1pCVBTk3NStyAW8w6UT+mP+KjlzkE8JimamjbdNXRWGI4SEbg2bR3BKjEYebIFP Xssw== X-Gm-Message-State: AOAM531bTEsdxPcmldLYTELVvYuaA6IbEdON1VPpGtPGSt1icAauHF56 X91vCc9dwY8RcHdG7nD0AdGrON6GNNbqu9iqi7017faQ3XeUsIUBJPy9x2coS2yvJWixj0bwj/o EPiZ9vcXjnZV4ctWQKA== X-Received: by 2002:a7b:cc8c:: with SMTP id p12mr6738650wma.105.1634823012258; Thu, 21 Oct 2021 06:30:12 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxCcn2BioIw5CVR3wLRNv+Rao00K3ABGmXe40YZiJTAUKdNvou+3zL7RzU3KD/c3i0YZHLVkA== X-Received: by 2002:a7b:cc8c:: with SMTP id p12mr6738600wma.105.1634823011865; Thu, 21 Oct 2021 06:30:11 -0700 (PDT) Received: from abulafia.quesejoda.com ([139.47.33.227]) by smtp.gmail.com with ESMTPSA id r205sm5203287wma.3.2021.10.21.06.30.11 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 21 Oct 2021 06:30:11 -0700 (PDT) Subject: Re: [PATCH] Convert strlen pass from evrp to ranger. To: Richard Biener Cc: Jeff Law , Jakub Jelinek , GCC patches , Andrew MacLeod 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> From: Aldy Hernandez Message-ID: <73764d81-f90c-f110-b879-cefd7c13258d@redhat.com> Date: Thu, 21 Oct 2021 15:30:10 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-7.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, 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 13:30:16 -0000 On 10/21/21 3:14 PM, Richard Biener wrote: > On Thu, Oct 21, 2021 at 2:56 PM Aldy Hernandez wrote: >> >> On Thu, Oct 21, 2021 at 12:20 PM Richard Biener >> wrote: >>> >>> 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. >> >> I couldn't either, but perhaps what Jeff is referring to is the ad-hoc >> copy propagation that happens in the DOM's use of the threader: >> >> /* Make a copy of the uses & vuses into USES_COPY, then cprop into >> the operands. */ >> FOR_EACH_SSA_USE_OPERAND (use_p, stmt, iter, SSA_OP_ALL_USES) >> { >> tree tmp = NULL; >> tree use = USE_FROM_PTR (use_p); >> >> copy[i++] = use; >> if (TREE_CODE (use) == SSA_NAME) >> tmp = SSA_NAME_VALUE (use); >> if (tmp) >> SET_USE (use_p, tmp); >> } >> >> cached_lhs = simplifier->simplify (stmt, stmt, bb, this); >> >> /* Restore the statement's original uses/defs. */ >> i = 0; >> FOR_EACH_SSA_USE_OPERAND (use_p, stmt, iter, SSA_OP_ALL_USES) >> SET_USE (use_p, copy[i++]); > > Ah, likely. These days we'd likely use a gimple_match_op but then > this seems heavily abstracted, no idea where simplifier->simplify > might lead to ;) > I'm also not sure why the threader would do the valueization here and > not the simplify() function (and lookup_avail_expr misses an 'exploded' operand > lookup as well). Lot's of legacy code ;) Yes, there's a lot of legacy code I've left mostly alone. There are two copies of simplify() now, the DOM version (dom_jt_simplifier::simplify) and the VRP version (hybrid_jt_simplifier::simplify). Each do slightly different things, as has always been the case. It's just that the separation is now explicit. No idea what's up with the valueization either. The VRP version doesn't need any of this valuezation or on the side structures. You just ask the range of a stmt on a path and it gives you an answer. > > But I think the above is not going to be an issue unless ranger runs in > circles around backedges, arriving at this very same stmt again? A way > out might be to copy the stmt to the stack, adjust operands and use that > for the simplify entry. If you look at the patch I sent Jeff, you'll see that I've removed most of what's in that function. There's no need to propagate anything. The ranger can simplify the final conditional without having to set up anything. Aldy