From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21040 invoked by alias); 19 Oct 2004 20:48:31 -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 21014 invoked from network); 19 Oct 2004 20:48:29 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org with SMTP; 19 Oct 2004 20:48:29 -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.10) with ESMTP id i9JKmTem012849; Tue, 19 Oct 2004 16:48:29 -0400 Received: from [172.16.83.149] (vpn83-149.boston.redhat.com [172.16.83.149]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id i9JKmRr22480; Tue, 19 Oct 2004 16:48:27 -0400 Subject: Re: Fix a tcb crash and a potential bug on mainline From: Jeffrey A Law Reply-To: law@redhat.com To: Steven Bosscher Cc: gcc-patches@gcc.gnu.org, dnovillo@redhat.com, amacleod@redhat.com In-Reply-To: <200410182303.52997.stevenb@suse.de> References: <200410182303.52997.stevenb@suse.de> Content-Type: text/plain Organization: Red Hat, Inc Message-Id: <1098218905.2915.123.camel@localhost.localdomain> Mime-Version: 1.0 Date: Tue, 19 Oct 2004 20:58:00 -0000 Content-Transfer-Encoding: 7bit X-SW-Source: 2004-10/txt/msg01624.txt.bz2 On Mon, 2004-10-18 at 15:03, Steven Bosscher wrote: > Later on cleanup_control_flow() tries to do its thing on basic block 4. > It tries to fold "(c_13 != 0B)", but c_13 is on the free list, so it > does not have a TREE_TYPE, so fold crashes. Hmm. I wonder if this is the same bug Andrew Pinski was trying to fix by not clearing TREE_TYPE. Your description matches a lot of the tidbits Andrew mentioned. > > I think the root cause of this bug is that c_13 is put on the free list, > and I think its SSA_NAME_DEF_STMT should be set to the empty statement, > because remove_phi_arg_num() cannot know if there are any immediate > uses of it still left. ??? Putting c_13 on the free list is the right thing to do. I'm not sure why you think setting SSA_NAME_DEF_STMT to NULL is particularly interesting though. Whether or not the name has any uses left in the IL isn't something we use to determine whether or not to set SSA_NAME_DEF_STMT to any particular value IIRC. > I don't know if making that change could have > some unforseen effects, so I decided to re-hide the the bug by just not > looking at unreachable blocks in cleanup_control_flow(). I'm not sure this is sufficient to hide the bug well enough though. > I've tried to > convince myself that this can not happen in other situations, but I'm > not sure. Well, any code that deletes edges from the CFG, then later looks at statements without first calling delete_unreachable_blocks is suspect. But I'm not sure if we've even solved this problem within the context of cleanup_tree_cfg. Let's say we have a PHI in block A. Assume the result is used in block B. Assume block A will become unreachable. While block B must also be unreachable (block B is dominated by block A), we won't necessarily have removed all its incoming edges. Thus when we process block B we can still end up touching a statement which references a released SSA_NAME. Ugh... > This may be something Andrew may want to look into as part > of his immediate uses work?? It's something we've pondered and talked about a little. Basically with the immediate use work we would know where all the uses are and we could probably arrange to delete those uses. The trick is whether or not we can do that without introducing the kinds of problems we had with delete_insn in the past which tried to do similar stuff. > > This fix is completely untested at the moment, but it's quite obvious > and it fixes the ICE I was seeing. Still, I am a bit surprised that we > have not been in trouble over this bug before - perhaps we should still > fix the remove_phi_arg_num() issue instead of papering it over this way? I'm not sure what you mean by fix remove_phi_arg_num except to compute the immediate uses of the result and delete those statements, which in turn can cause other statements to get deleted. Basically it turns into immediate use based DCE. Roughly the same thing we'd end up doing if we tried to do integrated dead code removal using immediate uses. Alternately we could split up cleanup_control_expr_graph into two phases. The first identifies all the COND_EXPRs and SWITCH_EXPRs which need updating, but does not do any edge removal. Once we have done our full IL scan, we go back to the nodes where we know which outgoing edge will be taken and proceed to remove the COND_EXPR/SWITCH_EXPR and the untaken edges. jeff