From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13802 invoked by alias); 22 Oct 2004 19:07:59 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 13689 invoked from network); 22 Oct 2004 19:07:57 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org with SMTP; 22 Oct 2004 19:07:57 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.11/8.12.11) with ESMTP id i9MJ7lNN018352; Fri, 22 Oct 2004 15:07:52 -0400 Received: from [172.16.83.146] (vpn83-146.boston.redhat.com [172.16.83.146]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id i9MJ7dr30611; Fri, 22 Oct 2004 15:07:39 -0400 Subject: Re: Fix a tcb crash and a potential bug on mainline From: Jeffrey A Law Reply-To: law@redhat.com To: Andrew MacLeod Cc: Steven Bosscher , gcc-patches , Diego Novillo In-Reply-To: <1098278085.5695.3886.camel@pain> References: <200410182303.52997.stevenb@suse.de> <200410192328.28252.stevenb@suse.de> <1098223558.2915.131.camel@localhost.localdomain> <200410200036.32335.stevenb@suse.de> <1098227633.2915.166.camel@localhost.localdomain> <1098278085.5695.3886.camel@pain> Content-Type: text/plain Organization: Red Hat, Inc Message-Id: <1098472058.2915.209.camel@localhost.localdomain> Mime-Version: 1.0 Date: Fri, 22 Oct 2004 19:10:00 -0000 Content-Transfer-Encoding: 7bit X-SW-Source: 2004-10/txt/msg01946.txt.bz2 On Wed, 2004-10-20 at 07:14, Andrew MacLeod wrote: > On Tue, 2004-10-19 at 19:13, Jeffrey A Law wrote: > > On Tue, 2004-10-19 at 16:36, Steven Bosscher wrote: > > > > > > Err, if we remove the last argument to a PHI node then by definition > > > > the block containing the PHI is unreachable as it has no incoming > > > > edges. > > > > > > > > There is no such thing as an "undefined" PHI node. > > > > > > I didn't say we do. I said we may have an undefined PHI result, > > > i.e. and SSA_NAME without a definition. > > OK. And that is something that is, unfortunately, normal in our > > implementation. If I look at the SSA_NAME manager that's by far > > its largest wart. > > > > If I understand the problem properly, the fundamental problem here is > that a block becomes unreachable by removing all the incoming edges. PHI > arguments for those edges are removed as the edge is removed. When the > last edge is removed, the PHI node has no more arguments, and thus gets > deleted. None of the code in the dead block is removed until later when > delete_unreachable_blocks is called. Meanwhile, we end up pawing through > one or more stmts in the dead block which contains uses of the PHI > result. > > I have a couple of questions. > > 1 - Why does cleanup_control_flow() ever look at a stmts inside an > unreachable block? Its seems rather, umm, unnecessary, for starters. Why > not just call delete_unreachable_blocks() as the *first* thing in > cfg_cleanup(). It doesn't look like its an expensive call... That doesn't help solve this problem. The unreachable blocks are created by cleanup_control_flow itself. So calling delete_unreachable_blocks before or after cleanup_control_flow doesn't solve the problem. > 2 - The issue may also exist outside cleanup_control_flow, so why does > remove_phi_arg_num have to remove the PHI node when is removes the last > argument. Why not allow the PHI node to exist with no arguments. It'll > get removed and free'd if the block remains unreachable. An excellent question and suggestion. It may actually be the best proposed solution. I think we would all agree that a PHI with no arguments can only occur in an unreachable block. Can someone verify that we release PHI nodes when we remove unreachable blocks? I suspect we don't right now because we rely upon generic code to identify and remove unreachable blocks. IIRC that does doesn't know about PHIs at all. > Maybe someone > will add another edge back in at some point during the optimization, > then there are PHI nodes ready to take the arguments. I presume right > now we make sure we add new edges before deleting old ones when > redirecting things (to avoid deleting the PHI). I'm not aware of any code that does this. But your solution certainly avoids this kind of problem in the future. It's certainly the case that we've had code elsewhere in the compiler which would temporarily bump up a counter to avoid having objects disappear. This was pretty common with labels in the RTL optimizers. Jeff