From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21459 invoked by alias); 19 Oct 2004 22:35: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 21450 invoked from network); 19 Oct 2004 22:35:57 -0000 Received: from unknown (HELO x93.infopact.nl) (212.29.160.93) by sourceware.org with SMTP; 19 Oct 2004 22:35:57 -0000 Received: from steven.lr-s.tudelft.nl (70-90.ipact.nl [82.210.90.70]) by x93.infopact.nl (8.12.10/8.12.10) with ESMTP id i9JMZt10010845; Wed, 20 Oct 2004 00:35:56 +0200 From: Steven Bosscher Organization: SUSE Labs To: law@redhat.com Subject: Re: Fix a tcb crash and a potential bug on mainline Date: Tue, 19 Oct 2004 22:36:00 -0000 User-Agent: KMail/1.5.4 Cc: gcc-patches@gcc.gnu.org, dnovillo@redhat.com, amacleod@redhat.com References: <200410182303.52997.stevenb@suse.de> <200410192328.28252.stevenb@suse.de> <1098223558.2915.131.camel@localhost.localdomain> In-Reply-To: <1098223558.2915.131.camel@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200410200036.32335.stevenb@suse.de> X-CanItPRO-Stream: NoScan X-Spam-Score: undef - spam-scanning disabled X-Canit-Stats-ID: 1167886 - 9186e0ecda57 X-Scanned-By: CanIt (www . canit . ca) X-SW-Source: 2004-10/txt/msg01650.txt.bz2 On Wednesday 20 October 2004 00:05, Jeffrey A Law wrote: > > Not usually. But this case is special. Normally we don't just throw > > away a statement, we first update the code or we delete entire blocks > > of code. In this particular case we're only deleting a PHI node and > > we don't know if the block we're deleting the PHI from is unreachable > > (we could, but it's not the business of remove_phi_arg_num() to remove > > unreachable blocks). So conceptually the PHI_RESULT of the deleted > > PHI becomes undefined. But it is not dead until all its uses are also > > removed. We express "undefined-ness" by setting the SSA_NAME_DEF_STMT > > to the empty statement. > > 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. There is such a thing as an "undefined" SSA name. But you're right, when the last PHI argument disappears, the block for that PHI must be dead. Only, what I was arguing for is that it is not up to remove_phi_arg_num() to decide on that. If we need to do that for the SSA names free list, I think that is unfortunate. > If they are using the result of a deleted PHI or some other deleted > statement, then those statements must also be dead. It's one of > the nice SSA properties. But you end up doing far more than just removing a PHI argument when you start deleting the statements using the result of the deleted PHI. > > What I mean with this is that perhaps we should not put the SSA_NAME on > > the free list if there still are immediate uses. On Andrew's branch > > that is very easy to do, if we decide to fix the bug this way. > > No, you would delete all the immediate uses. So you want to turn remove_phi_arg_num() into an ad-hoc DCE. It would work, no doubt. But just removing a PHI argument does not need to be so complicated. > > I mean to fix it by making it set the SSA_NAME_DEF_STMT of PHIs that > > it is deleting to the empty statement... > > No. That is wrong. I believe your approach is not right and mine is simpler, but, whatever. In any case, the original patch is still useful in itself, and it happens to fix the crash at least for now. Updated version that I've bootstrapped and tested on i686-suse-linux-gnu is below. It fixes emptyif.f90. Compile time is unchanged. OK for mainline? Gr. Steven * tree-cfg.c (cleanup_control_flow): Don't try to cleanup statements in unreachable blocks. Remove all outgoing edges of such blocks. Index: tree-cfg.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/tree-cfg.c,v retrieving revision 2.83 diff -c -3 -p -r2.83 tree-cfg.c *** tree-cfg.c 19 Oct 2004 15:38:25 -0000 2.83 --- tree-cfg.c 19 Oct 2004 19:10:28 -0000 *************** cleanup_control_flow (void) *** 1864,1869 **** --- 1864,1882 ---- FOR_EACH_BB (bb) { + /* We can have unreachable blocks here if we run before + delete_unreachable_blocks or if a block visited before + this one had an edge to bb that we removed. + + Remove all outdoing edges to perhaps make BB's successors + unreachable too. Then move on to the next block. */ + if (EDGE_COUNT (bb->preds) == 0) + { + while (EDGE_COUNT (bb->succs) > 0) + ssa_remove_edge (EDGE_SUCC (bb, 0)); + continue; + } + bsi = bsi_last (bb); if (bsi_end_p (bsi))