From patchwork Sat Nov 3 16:11:22 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexandre Oliva X-Patchwork-Id: 196869 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 10FC52C007B for ; Sun, 4 Nov 2012 03:11:50 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1352563912; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:Received:Received:From:To:Cc:Subject:References:Date: In-Reply-To:Message-ID:User-Agent:MIME-Version:Content-Type: Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:Sender:Delivered-To; bh=bZ9WC5v1YdJtGh1N+QVG lhxao48=; b=RCguLvLrR890RvYbQGFwfg9ELs0+hlhTPIBwAFqrpdKCWw4pEMIZ 6hPFJQ2zEpFupzozJ8Gj+7eLVLooP2NoEop+EZwWbTFzoJzFc4mxzY/Owkq6Lj7T e1DIXyzFJv8EbVFVxc8ItykhxQ9k4xkXXHTLPZP4UqQouBjfOJDbGIU= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Received:Received:Received:From:To:Cc:Subject:References:Date:In-Reply-To:Message-ID:User-Agent:MIME-Version:Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=I6xj/1RxphTODNUi7fx84OmCyOtV/Rq6jUgPvWxLE3B/gzRMnqKIlRePZxzCxR uqryqZ7yhch6Iv9F+Zxqae2rYDurxIUpAlsQNOXLcKNS5QXCREMe/m7jrwUaXhcX rSy1rrzxkdjOF2ssgDe69pWSzS9wU9j6nTIKT8peyZg9k=; Received: (qmail 9097 invoked by alias); 3 Nov 2012 16:11:42 -0000 Received: (qmail 9089 invoked by uid 22791); 3 Nov 2012 16:11:41 -0000 X-SWARE-Spam-Status: No, hits=-5.9 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, KHOP_SPAMHAUS_DROP, RCVD_IN_DNSWL_HI, RCVD_IN_HOSTKARMA_W, RP_MATCHES_RCVD, SPF_HELO_PASS, TW_TM, T_TVD_MIME_NO_HEADERS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 03 Nov 2012 16:11:28 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qA3GBRBx030451 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Sat, 3 Nov 2012 12:11:27 -0400 Received: from freie (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id qA3GBP3b030873 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 3 Nov 2012 12:11:26 -0400 Received: from livre.localdomain (livre-to-gw.oliva.athome.lsd.ic.unicamp.br [172.31.160.19]) by freie (8.14.5/8.14.5) with ESMTP id qA3GBO2i021253; Sat, 3 Nov 2012 14:11:24 -0200 Received: from livre.localdomain (aoliva@localhost.localdomain [127.0.0.1]) by livre.localdomain (8.14.3/8.14.3/Debian-5+lenny1) with ESMTP id qA3GBNTN020258; Sat, 3 Nov 2012 14:11:23 -0200 Received: (from aoliva@localhost) by livre.localdomain (8.14.3/8.14.3/Submit) id qA3GBMYZ020256; Sat, 3 Nov 2012 14:11:22 -0200 From: Alexandre Oliva To: Jakub Jelinek Cc: gcc-patches@gcc.gnu.org Subject: Re: [PR54693] loss of debug info in jump threading and loop ivopts References: <20121101092756.GF1891@tucnak.redhat.com> Date: Sat, 03 Nov 2012 14:11:22 -0200 In-Reply-To: (Alexandre Oliva's message of "Fri, 02 Nov 2012 15:56:35 -0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org On Nov 2, 2012, Alexandre Oliva wrote: > On Nov 1, 2012, Jakub Jelinek wrote: >> Even for stmt frontiers it is IMHO undesirable to duplicate >> (perhaps many times) the whole sequence, as one would then reply the var >> changing sequence in the debugger perhaps once in the original bb, then >> once or many times again in the successor bb. > Quite the opposite: when we do jump threading, we're *bypassing* the > entire block What I wrote above was not entirely accurate. We do bypass empty blocks, in thread_around_empty_block, but we propagated debug stmts from their predecessors, that may actually be the block duplicated for threading. So, the debug stmts from the duplicated block are entirely artificial, exclusively for purposes of SSA resolution, while those from bypassed otherwise-empty blocks actually tell the history of the computation. All that said, even the latter are useless right now, when their bindings happen to be immediately modified by subsequent debug stmts. Now, one of the problems with my patch was that it propagated debug stmts over and over, when there was a long chain of empty blocks. I'd assumed they'd all be eliminated after the jump was redirected, but they're only bypassed by the copy of the initial block. Furthermore, when jump threading fails to determine the taken edge out of e->dest, it proceeds to trying to threading through successors of e->dest, and the current code would propagate debug stmts from e->dest to single-pred successors at such attempt, even if no threading would take place. The patch below (based on a tree from before your first patch that's already in) improves the situation: we only propagate debug stmts to the final destination block, and only after we make sure we are performing jump threading to that block. Also, it refrains from propagating debug bind stmts whose bindings are rendered inoperant by later bindings, in the threaded blocks or at the top of the destination block. I didn't try delaying the creation of the pointer set as you did, but I'm now thinking using a linear on-stack vector for searches up to some size might be even faster than creating the pointer set at the first var. Regstrapping on x86_64-linux-gnu and i686-linux-gnu. Ok to install? Propagate debug stmts only after decision to thread. From: Alexandre Oliva for gcc/ChangeLog PR debug/54693 * tree-ssa-threadedge.c (propagate_threaded_block_debug_into): New, rewritten from debug stmt copying code... (thread_around_empty_block): ... removed from here. (thread_across_edge): Call propagate_threaded_block_debug_into. --- gcc/tree-ssa-threadedge.c | 106 +++++++++++++++++++++++++++++++++++++-------- 1 files changed, 87 insertions(+), 19 deletions(-) diff --git a/gcc/tree-ssa-threadedge.c b/gcc/tree-ssa-threadedge.c index f43a564..a9c671e 100644 --- a/gcc/tree-ssa-threadedge.c +++ b/gcc/tree-ssa-threadedge.c @@ -610,6 +610,85 @@ cond_arg_set_in_bb (edge e, basic_block bb) return false; } +/* Copy debug stmts from DEST's chain of single predecessors up to + SRC, so that we don't lose the bindings as PHI nodes are introduced + when DEST gains new predecessors. */ +static void +propagate_threaded_block_debug_into (basic_block dest, basic_block src) +{ + if (!MAY_HAVE_DEBUG_STMTS) + return; + + if (!single_pred_p (dest)) + return; + + gcc_checking_assert (dest != src); + + gimple_stmt_iterator gsi = gsi_after_labels (dest); + pointer_set_t *vars = pointer_set_create (); + + for (gimple_stmt_iterator si = gsi; + !gsi_end_p (si); gsi_next (&si)) + { + gimple stmt = gsi_stmt (si); + if (!is_gimple_debug (stmt)) + break; + + tree var; + + if (gimple_debug_bind_p (stmt)) + var = gimple_debug_bind_get_var (stmt); + else if (gimple_debug_source_bind_p (stmt)) + var = gimple_debug_source_bind_get_var (stmt); + else + gcc_unreachable (); + + pointer_set_insert (vars, var); + } + + basic_block bb = dest; + + do + { + bb = single_pred (bb); + for (gimple_stmt_iterator si = gsi_last_bb (bb); + !gsi_end_p (si); gsi_prev (&si)) + { + gimple stmt = gsi_stmt (si); + if (!is_gimple_debug (stmt)) + continue; + + tree var; + + if (gimple_debug_bind_p (stmt)) + var = gimple_debug_bind_get_var (stmt); + else if (gimple_debug_source_bind_p (stmt)) + var = gimple_debug_source_bind_get_var (stmt); + else + gcc_unreachable (); + + /* Discard debug bind overlaps. ??? Unlike stmts from src, + copied into a new block that will precede BB, debug bind + stmts in bypassed BBs may actually be discarded if + they're overwritten by subsequent debug bind stmts, which + might be a problem once we introduce stmt frontier notes + or somesuch. Adding `&& bb == src' to the condition + below will preserve all potentially relevant debug + notes. */ + if (pointer_set_insert (vars, var)) + continue; + + stmt = gimple_copy (stmt); + /* ??? Should we drop the location of the copy to denote + they're artificial bindings? */ + gsi_insert_before (&gsi, stmt, GSI_NEW_STMT); + } + } + while (bb != src && single_pred_p (bb)); + + pointer_set_destroy (vars); +} + /* TAKEN_EDGE represents the an edge taken as a result of jump threading. See if we can thread around TAKEN_EDGE->dest as well. If so, return the edge out of TAKEN_EDGE->dest that we can statically compute will be @@ -637,24 +716,6 @@ thread_around_empty_block (edge taken_edge, if (!single_pred_p (bb)) return NULL; - /* Before threading, copy DEBUG stmts from the predecessor, so that - we don't lose the bindings as we redirect the edges. */ - if (MAY_HAVE_DEBUG_STMTS) - { - gsi = gsi_after_labels (bb); - for (gimple_stmt_iterator si = gsi_last_bb (taken_edge->src); - !gsi_end_p (si); gsi_prev (&si)) - { - stmt = gsi_stmt (si); - if (!is_gimple_debug (stmt)) - continue; - - stmt = gimple_copy (stmt); - /* ??? Should we drop the location of the copy? */ - gsi_insert_before (&gsi, stmt, GSI_NEW_STMT); - } - } - /* This block must have more than one successor. */ if (single_succ_p (bb)) return NULL; @@ -827,6 +888,9 @@ thread_across_edge (gimple dummy_cond, } remove_temporary_equivalences (stack); + if (!taken_edge) + return; + propagate_threaded_block_debug_into (taken_edge->dest, e->dest); register_jump_thread (e, taken_edge, NULL); return; } @@ -892,7 +956,11 @@ thread_across_edge (gimple dummy_cond, same. */ tmp = find_edge (taken_edge->src, e3->dest); if (!tmp || phi_args_equal_on_edges (tmp, e3)) - register_jump_thread (e, taken_edge, e3); + { + propagate_threaded_block_debug_into (e3->dest, + taken_edge->dest); + register_jump_thread (e, taken_edge, e3); + } } }