From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 36688 invoked by alias); 18 Apr 2019 09:20:37 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 36679 invoked by uid 89); 18 Apr 2019 09:20:36 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-10.6 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,KAM_ASCII_DIVIDERS,SPF_PASS autolearn=ham version=3.3.1 spammy=forwarder X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 18 Apr 2019 09:20:35 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id ECDBAAD38 for ; Thu, 18 Apr 2019 09:20:32 +0000 (UTC) Date: Thu, 18 Apr 2019 10:21:00 -0000 From: Richard Biener To: gcc-patches@gcc.gnu.org Subject: [PATCH] Fix PR90131, wrong-debug Message-ID: User-Agent: Alpine 2.20 (LSU 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-SW-Source: 2019-04/txt/msg00755.txt.bz2 This fixes another case similar to the fixed PR89892, mergephi via remove_forwarder_block_with_phi caused out-of-date debug binds to become live and thus needs similar treatment as remove_forwarder_block. Previously it didn't even bother to move debug stmts because the destination always has multiple predecessors. Now we have to move and reset them. Fixed by factoring out a worker from remove_forwarder_block and using that from remove_forwarder_block_with_phi as well. Bootstrap & regtest running on x86_64-unknown-linux-gnu. Richard. 2019-04-18 Richard Biener PR debug/90131 * tree-cfgcleanup.c (move_debug_stmts_from_forwarder): Split out from ... (remove_forwarder_block): ... here. (remove_forwarder_block_with_phi): Also move debug stmts here. * gcc.dg/guality/pr90131.c: New testcase. Index: gcc/tree-cfgcleanup.c =================================================================== --- gcc/tree-cfgcleanup.c (revision 270437) +++ gcc/tree-cfgcleanup.c (working copy) @@ -444,6 +444,45 @@ phi_alternatives_equal (basic_block dest return true; } +/* Move debug stmts from the forwarder block SRC to DEST. */ + +static void +move_debug_stmts_from_forwarder (basic_block src, basic_block dest) +{ + if (!MAY_HAVE_DEBUG_STMTS) + return; + + bool can_move_debug_stmts = single_pred_p (dest); + gimple_stmt_iterator gsi_to = gsi_after_labels (dest); + for (gimple_stmt_iterator gsi = gsi_after_labels (src); !gsi_end_p (gsi);) + { + gimple *debug = gsi_stmt (gsi); + gcc_assert (is_gimple_debug (debug)); + /* Move debug binds anyway, but not anything else like begin-stmt + markers unless they are always valid at the destination. */ + if (can_move_debug_stmts + || gimple_debug_bind_p (debug)) + { + gsi_move_before (&gsi, &gsi_to); + /* Reset debug-binds that are not always valid at the destination. + Simply dropping them can cause earlier values to become live, + generating wrong debug information. + ??? There are several things we could improve here. For + one we might be able to move stmts to the predecessor. + For anther, if the debug stmt is immediately followed by a + (debug) definition in the destination (on a post-dominated path?) + we can elide it without any bad effects. */ + if (!can_move_debug_stmts) + { + gimple_debug_bind_reset_value (debug); + update_stmt (debug); + } + } + else + gsi_next (&gsi); + } +} + /* Removes forwarder block BB. Returns false if this failed. */ static bool @@ -454,7 +493,6 @@ remove_forwarder_block (basic_block bb) gimple *stmt; edge_iterator ei; gimple_stmt_iterator gsi, gsi_to; - bool can_move_debug_stmts; /* We check for infinite loops already in tree_forwarder_block_p. However it may happen that the infinite loop is created @@ -503,8 +541,6 @@ remove_forwarder_block (basic_block bb) } } - can_move_debug_stmts = MAY_HAVE_DEBUG_STMTS && single_pred_p (dest); - basic_block pred = NULL; if (single_pred_p (bb)) pred = single_pred (bb); @@ -566,40 +602,7 @@ remove_forwarder_block (basic_block bb) /* Move debug statements. Reset them if the destination does not have a single predecessor. */ - if (!gsi_end_p (gsi)) - { - gsi_to = gsi_after_labels (dest); - do - { - gimple *debug = gsi_stmt (gsi); - gcc_assert (is_gimple_debug (debug)); - /* Move debug binds anyway, but not anything else - like begin-stmt markers unless they are always - valid at the destination. */ - if (can_move_debug_stmts - || gimple_debug_bind_p (debug)) - { - gsi_move_before (&gsi, &gsi_to); - /* Reset debug-binds that are not always valid at the - destination. Simply dropping them can cause earlier - values to become live, generating wrong debug information. - ??? There are several things we could improve here. For - one we might be able to move stmts to the predecessor. - For anther, if the debug stmt is immediately followed - by a (debug) definition in the destination (on a - post-dominated path?) we can elide it without any bad - effects. */ - if (!can_move_debug_stmts) - { - gimple_debug_bind_reset_value (debug); - update_stmt (debug); - } - } - else - gsi_next (&gsi); - } - while (!gsi_end_p (gsi)); - } + move_debug_stmts_from_forwarder (bb, dest); bitmap_set_bit (cfgcleanup_altered_bbs, dest->index); @@ -1282,6 +1285,10 @@ remove_forwarder_block_with_phi (basic_b redirect_edge_var_map_clear (e); } + /* Move debug statements. Reset them if the destination does not + have a single predecessor. */ + move_debug_stmts_from_forwarder (bb, dest); + /* Update the dominators. */ dombb = get_immediate_dominator (CDI_DOMINATORS, bb); domdest = get_immediate_dominator (CDI_DOMINATORS, dest); Index: gcc/testsuite/gcc.dg/guality/pr90131.c =================================================================== --- gcc/testsuite/gcc.dg/guality/pr90131.c (nonexistent) +++ gcc/testsuite/gcc.dg/guality/pr90131.c (working copy) @@ -0,0 +1,35 @@ +/* { dg-do run } */ +/* { dg-options "-g" } */ + +void __attribute__((noinline)) +optimize_me_not () +{ + __asm__ volatile ("" : : : "memory"); +} +volatile long a; +int b[9][1]; +static short c[2][1] = {3}; +int main() +{ + int i, d, e; + i = 0; + for (; i < 9; i++) + a = b[i][0]; + i = 0; + for (; i < 2; i++) + { + d = 0; + for (; d < 1; d++) + { + e = 0; + for (; e < 1; e++) + a = c[i][e]; + /* i may very well be optimized out, so we cannot test for i == 0. + Instead test i + 1 which will make the test UNSUPPORTED if i + is optimized out. Since the test previously had wrong debug + with i == 9 this is acceptable. */ + optimize_me_not(); /* { dg-final { gdb-test . "i + 1" "1" } } */ + } + } + return 0; +}