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 B55E438582B1 for ; Fri, 12 Aug 2022 15:29:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B55E438582B1 Received: from mail-ot1-f69.google.com (mail-ot1-f69.google.com [209.85.210.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-220-RsSl771JPd6Svsb7C_Ni9Q-1; Fri, 12 Aug 2022 11:29:33 -0400 X-MC-Unique: RsSl771JPd6Svsb7C_Ni9Q-1 Received: by mail-ot1-f69.google.com with SMTP id cg12-20020a056830630c00b00636971fbe06so329963otb.0 for ; Fri, 12 Aug 2022 08:29:33 -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=O911ULz6IIjO7hiFmR2lI+o2TpP474FitAnie7qOIZ8=; b=YnY4PPEuI0cX0W3Up2DiQtLn1kzzKg62N1Gb0QD3oqZ1Au+bVsmLrPEK3JzUTwBFJ+ 74McRbqlDK/PdtXIgaFXIB14/eOglPcj+tE35jfrUmTTnP7zy36lQrO1eqUx/XvSEq+J lFcO01B/zLiA6dkYxwOH1bCfoYIOc+NrIJo9gTgnCPqMsAubVDOfMx7eIkNliIrWxX1H wX8e7vLhLWs/zpXqCXtgZ99evcIJZK3HuPScjiQSLeX5scBcU/RO1XWKxkfWMNsn5f24 IJyebvFWOzILSr7CvX99NyxbssafZIu9tvnlqXOHkin1ZInUDErxjWbir2kXUSthG7zt oOwA== X-Gm-Message-State: ACgBeo0hySuDJzNYf1lXrKnNJyjg0WLazXOWA2/Vbfd1Tieaf0apdnv5 ysp3Iw7sZ0Sqt8M1KD5+P7I33wGXSbdQF3c+JW7lf3+oMdxTIAlJz/QVN2w5iZbHOtfxZrPqAqs LI6UUaDDbDVok9GuejH/UUkpzWUIeE0IsDQ== X-Received: by 2002:a05:6870:210b:b0:10b:ed11:4e2d with SMTP id f11-20020a056870210b00b0010bed114e2dmr1982920oae.265.1660318172686; Fri, 12 Aug 2022 08:29:32 -0700 (PDT) X-Google-Smtp-Source: AA6agR6jQINYNHKsd6u+t4bqcz5ZIrEDWaTRFd9YmVZw4DdrJlYXgUWgnHFQ3RK/Ts20A0+07RVw7NZESZnv122Cryw= X-Received: by 2002:a05:6870:210b:b0:10b:ed11:4e2d with SMTP id f11-20020a056870210b00b0010bed114e2dmr1982901oae.265.1660318172373; Fri, 12 Aug 2022 08:29:32 -0700 (PDT) MIME-Version: 1.0 References: <20220812105937.227C413305@imap2.suse-dmz.suse.de> In-Reply-To: From: Aldy Hernandez Date: Fri, 12 Aug 2022 17:29:21 +0200 Message-ID: Subject: Re: [PATCH] tree-optimization/106593 - fix ICE with backward threading To: Richard Biener Cc: "MacLeod, Andrew" , gcc-patches X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-5.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, 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 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 15:29:36 -0000 On Fri, Aug 12, 2022 at 1:36 PM Richard Biener wrote: > > On Fri, 12 Aug 2022, 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 :). > > > > For now I would return VARYING in range_on_path_entry if range_of_expr > > returns false. We shouldn't be ICEing when we can gracefully handle > > things. This gcc_unreachable was there to catch implementation issues > > during development. > > > > I would keep your gimple_range_ssa_p check regardless. No sense doing > > extra work if we're absolutely sure we won't handle it. > > OK, I'll push just the gimple_range_ssa_p then since that resolves > the PR on its own. I was first misled about the gcc_unreachable > and my brain hurt understanding this function ... (also as to > why using range_of_expr on a _random_ stmt would be OK). Calling range_of_expr on a random stmt, is not OK, and is bound to lead to subtle issues. As I mentioned earlier, and both in the comments for class path_range_query and path_range_query::internal_range_of_expr, all we really support is querying range_of_stmt and range_of_expr as it would appear at the end of the path. Internally to the path solver, if it uses range_of_expr and the SSA is defined out side the path, we'll ignore the statement altogether and return the range on entry to the path. So yeah... feeding random statements is not good. It's meant to be used to query ranges of SSA names at the end of the path. Hmmm, perhaps I should rewrite path_range_query::internal_range_of_expr() to explicitly ignore the STMT, or even put some asserts if it's being used nonsensibly. Aldy