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 D5D8D3858D28 for ; Fri, 12 Aug 2022 14:55:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D5D8D3858D28 Received: from mail-oi1-f198.google.com (mail-oi1-f198.google.com [209.85.167.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-652-Y6MmyxfaOZGtAh4hVBwy4g-1; Fri, 12 Aug 2022 10:55:52 -0400 X-MC-Unique: Y6MmyxfaOZGtAh4hVBwy4g-1 Received: by mail-oi1-f198.google.com with SMTP id m81-20020aca5854000000b00342c68cf859so523352oib.13 for ; Fri, 12 Aug 2022 07:55:52 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc; bh=m10Rlk00lkXGQFTRbLctZSTqhGLSgUOYKRk78kQAbIY=; b=NX6ta5HJWbURdI8q9unstiCTvM0siH5CrwJ47KMerr9x2XTVz8CGjci+hyW6ck7HiW UEH45ef9RvTdGHy4JFWgCKbxygLK2xmak+eKVcJ+Y/uSVbQW13/TRYvu15DqvDk8VNbl 3DN+J25ASDrfzdAZsnWbAeMZ2u8svraqWJizs42SxMEjyI47pr0nSBC7eXm4+F8C/Vog 3zcrgIOnUKrPF0t46/Jbjy0BD6eAazuxHBmEQVBknmzryuWIR3CCZuGLGNv1cwMrKviz wj4BubdAwLe5svGhy+yIF72eOc3Pq8TdiyC41oQK4ml8FHWPS/fRiLSuK+y7XpPVLhYO Bw4w== X-Gm-Message-State: ACgBeo2893LXdPI6VkLwz4OOQiiuSIbN21iMgQUbyW9N+tcYruWbdH4J lNavb1xMD5xsczGHSHq1/kaYx2z65rJ/P1TTH6uwvzhYIQSbGJOBfehhefvWMdFVnDv3vdOwdKR IXx1P7CFTOjEqkO0wuaj2MREUH+b+kCmcYQ== X-Received: by 2002:a05:6870:b617:b0:10d:f7ce:50df with SMTP id cm23-20020a056870b61700b0010df7ce50dfmr5958477oab.36.1660316151704; Fri, 12 Aug 2022 07:55:51 -0700 (PDT) X-Google-Smtp-Source: AA6agR6q/zYEblnaPPtUvypTDowfjsI9Pxe2rSWHIsbR8oxo1UFqeo9ntzulEfuczf+TvVqxYnaDTLSfnA+doDQMoK4= X-Received: by 2002:a05:6870:b617:b0:10d:f7ce:50df with SMTP id cm23-20020a056870b61700b0010df7ce50dfmr5958470oab.36.1660316151455; Fri, 12 Aug 2022 07:55:51 -0700 (PDT) MIME-Version: 1.0 References: <20220812105937.227C413305@imap2.suse-dmz.suse.de> <1d535caf-2451-b648-08ad-51776bc35ab8@redhat.com> <8bf88bc2-9bd5-f4d3-7207-066a973d13fd@redhat.com> In-Reply-To: <8bf88bc2-9bd5-f4d3-7207-066a973d13fd@redhat.com> From: Aldy Hernandez Date: Fri, 12 Aug 2022 16:55:39 +0200 Message-ID: Subject: Re: [PATCH] tree-optimization/106593 - fix ICE with backward threading To: Andrew MacLeod Cc: Richard Biener , gcc-patches X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-5.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, HTML_MESSAGE, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 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: Fri, 12 Aug 2022 14:55:55 -0000 In that case Richi, go right ahead with your original patch. I for one am happy we can use range_on_entry, which always seemed cleaner. Aldy On Fri, Aug 12, 2022, 16:07 Andrew MacLeod wrote: > > On 8/12/22 09:38, Andrew MacLeod wrote: > > > > On 8/12/22 07:31, Aldy Hernandez wrote: > >> On Fri, Aug 12, 2022 at 12:59 PM Richard Biener > >> wrote: > >>> With the last re-org I failed to make sure to not add SSA names > >>> nor supported by ranger into m_imports which then triggers an > >>> ICE in range_on_path_entry because range_of_expr returns false. I've > >>> noticed that range_on_path_entry does mightly complicated things > >>> that don't make sense to me and the commentary might just be > >>> out of date. For the sake of it I replaced it with range_on_entry > >>> and statistics show we thread _more_ jumps with that, so better > >>> not do magic there. > >> Hang on, hang on. range_on_path_entry was written that way for a > >> reason. Andrew and I had numerous discussions about this. For that > >> matter, my first implementation did exactly what you're proposing, but > >> he had reservations about using range_on_entry, which IIRC he thought > >> should be removed from the (public) API because it had a tendency to > >> blow up lookups. > >> > >> Let's wait for Andrew to chime in on this. If indeed the commentary > >> is out of date, I would much rather use range_on_entry like you > >> propose, but he and I have fought many times about this... over > >> various versions of the path solver :). > > > > The original issue with range-on-entry is one needed to be very > > careful with it. If you ask for range-on-entry of something which is > > not dominated by the definition, then the cache filling walk was > > getting filled all the way back to the top of the IL, and that was > > both a waste of time and memory., and in some pathological cases was > > outrageous. And it was happening more frequently than one imagines... > > even if accidentally. I think the most frequent accidental misuse we > > saw was calling range on entry for a def within the block, or a PHI > > for the block. > > > > Its a legitimate issue for used before defined cases, but there isnt > > much we can do about those anyway, > > > > range_of_expr on any stmt within a block, when the definition comes > > from outside he block causes ranger to trigger its internal > > range-on-entry "more safely", which is why it didn't need to be part > > of the API... but i admit it does cause some conniptions when for > > instance there is no stmt in the block. > > > > That said, the improvements since then to the cache to be able to > > always use dominators, and selectively update the cache at strategic > > locations probably removes most issues with it. That plus we're more > > careful about timing things these days to make sure something horrid > > isn't introduced. I also notice all my internal range_on_entry and > > _exit routines have evolved and are much cleaner than they once were. > > > > So. now that we are sufficiently mature in this space... I think we > > can promote range_on_entry and range_on_exit to full public API.. It > > does seem that there is some use practical use for them. > > > > Andrew > > > > PS. It might even be worthwhile to add an assert to make sure it isnt > > being called on the def block.. just to avoid that particular stupidty > > :-) I'll take care of doing this. > > > > > Actually, as I look at it, perhaps better to leave things as they are.. > ie, not promote it to a part of the range_query API.. that appears > fraught with derived issues in other places. > > Continue to leave it in rangers public API and anyone using a ranger can > use it. I will add the assert to make sure its not abused in the common > way of the past. > > And yes, this will dramatically simplify the path_entry routine :-) > > Andrew > >