From patchwork Thu Dec 15 09:23:27 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 131544 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 A7BCB1007D6 for ; Thu, 15 Dec 2011 20:24:27 +1100 (EST) Received: (qmail 957 invoked by alias); 15 Dec 2011 09:24:22 -0000 Received: (qmail 941 invoked by uid 22791); 15 Dec 2011 09:24:21 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,TW_TM X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 15 Dec 2011 09:24:07 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1Rb7Xo-0004VV-Nq from Tom_deVries@mentor.com ; Thu, 15 Dec 2011 01:24:04 -0800 Received: from SVR-IES-FEM-01.mgc.mentorg.com ([137.202.0.104]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Thu, 15 Dec 2011 01:24:04 -0800 Received: from [127.0.0.1] (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.1.289.1; Thu, 15 Dec 2011 09:24:02 +0000 Message-ID: <4EE9BC8F.8020803@mentor.com> Date: Thu, 15 Dec 2011 10:23:27 +0100 From: Tom de Vries User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.23) Gecko/20110922 Lightning/1.0b2 Thunderbird/3.1.15 MIME-Version: 1.0 To: Jakub Jelinek CC: Tom de Vries , Richard Guenther , "gcc-patches@gcc.gnu.org" Subject: Re: [PATCH, PR51491] add CLOBBER before __builtin_stack_restore when converting vla alloca_with_align to array decl References: <4EE74482.3070503@codesourcery.com> <20111213131357.GY1957@tyan-ft48-01.lab.bos.redhat.com> In-Reply-To: <20111213131357.GY1957@tyan-ft48-01.lab.bos.redhat.com> 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 13/12/11 14:13, Jakub Jelinek wrote: > On Tue, Dec 13, 2011 at 01:26:42PM +0100, Tom de Vries wrote: >> 2011-12-13 Tom de Vries >> >> PR tree-optimization/51491 >> * tree-ssa-ccp.c (insert_clobber_before_stack_restore): New function. >> (ccp_fold_stmt): Use insert_clobber_before_stack_restore after a >> successful fold_builtin_alloca_with_align. > > I don't think this is safe. You don't want to look for any following > __builtin_stack_restore, but for the matching one. Consider: > > int g (int *); > > int > f (int n) > { > int tt = 0; > int t = 4; > { > int a[t > #ifdef DIFFERENT_BB2 > + (tt != 0 ? 6 : 0) > #endif > ]; > tt = g (a); > { > int b[n]; > tt += g (b); > #ifdef DIFFERENT_BB > if (n > 20) > tt += 148 * g (b); > #endif > tt += b[0]; > } > tt += a[0]; > } > { > int a[4]; > tt += g (a); > tt += a[0]; > } > return tt; > } > > Without any defines, this shows that looking for the first > BUILT_IN_STACK_RESTORE is wrong if you ignore BUILT_IN_STACK_SAVE calls. > And with the various defines it shows that neither the corresponding > __builtin_stack_save nor __builtin_stack_restore have to be in the same > bb as __builtin_alloca_with_align that is being folded. Jakub, thanks for pointing that out. Above example is now included as test-case. > Perhaps you want to look for the closest enclosing __builtin_stack_save > (search backwards in current basic block, its immediate dominator etc.?), > remember what SSA_NAME it stores its result into and then just look at > where is the (single?) user of that, which ought to be > __builtin_stack_restore. > this new patch looks for the closest dominating STACK_SAVE with result, and inserts a clobber before each corresponding STACK_RESTORE. The value restored by a STACK_RESTORE may be a phi result, so the patch also deals with this, in a recursive manner, while keeping tracking of visited phi nodes to avoid infinite recursion. bootstrapped and reg-tested (ada inclusive) on x86_64. Ok for stage3? Thanks, - Tom 2011-12-15 Tom de Vries PR tree-optimization/51491 * tree-ssa-ccp.c (insert_clobber_before_stack_restore) (gsi_prev_dom_bb_nondebug, insert_clobbers_for_var): New function. (ccp_fold_stmt): Use insert_clobbers_for_var after a successful fold_builtin_alloca_with_align. (ccp_visit_stmt): Calculate and free dominator info. * gcc.dg/pr51491.c: New test. * gcc.dg/pr51491-2.c: Same. Index: gcc/tree-ssa-ccp.c =================================================================== --- gcc/tree-ssa-ccp.c (revision 182098) +++ gcc/tree-ssa-ccp.c (working copy) @@ -1690,6 +1690,96 @@ evaluate_stmt (gimple stmt) return val; } +/* Given a BUILT_IN_STACK_SAVE value SAVED_VAL, insert a clobber of VAR before + each matching BUILT_IN_STACK_RESTORE. Mark visited phis in VISITED. */ + +static void +insert_clobber_before_stack_restore (tree saved_val, tree var, htab_t *visited) +{ + gimple stmt, clobber_stmt; + tree clobber; + imm_use_iterator iter; + gimple_stmt_iterator i; + gimple *slot; + + FOR_EACH_IMM_USE_STMT (stmt, iter, saved_val) + if (gimple_call_builtin_p (stmt, BUILT_IN_STACK_RESTORE)) + { + clobber = build_constructor (TREE_TYPE (var), NULL); + TREE_THIS_VOLATILE (clobber) = 1; + clobber_stmt = gimple_build_assign (var, clobber); + + i = gsi_for_stmt (stmt); + gsi_insert_before (&i, clobber_stmt, GSI_SAME_STMT); + } + else if (gimple_code (stmt) == GIMPLE_PHI) + { + if (*visited == NULL) + *visited = htab_create (10, htab_hash_pointer, htab_eq_pointer, NULL); + + slot = (gimple *)htab_find_slot (*visited, stmt, INSERT); + if (*slot != NULL) + continue; + + *slot = stmt; + insert_clobber_before_stack_restore (gimple_phi_result (stmt), var, + visited); + } + else + gcc_assert (is_gimple_debug (stmt)); +} + +/* Advance the iterator to the previous non-debug gimple statement in the same + or dominating basic block. */ + +static inline void +gsi_prev_dom_bb_nondebug (gimple_stmt_iterator *i) +{ + basic_block dom; + + gsi_prev_nondebug (i); + while (gsi_end_p (*i)) + { + dom = get_immediate_dominator (CDI_DOMINATORS, i->bb); + if (dom == NULL || dom == ENTRY_BLOCK_PTR) + return; + + *i = gsi_last_bb (dom); + } +} + +/* Find a BUILT_IN_STACK_SAVE dominating gsi_stmt (I), and insert + a clobber of VAR before each matching BUILT_IN_STACK_RESTORE. */ + +static void +insert_clobbers_for_var (gimple_stmt_iterator i, tree var) +{ + bool save_found; + gimple stmt; + tree saved_val; + htab_t visited = NULL; + + for (save_found = false; !gsi_end_p (i); gsi_prev_dom_bb_nondebug (&i)) + { + stmt = gsi_stmt (i); + + if (!gimple_call_builtin_p (stmt, BUILT_IN_STACK_SAVE)) + continue; + save_found = true; + + saved_val = gimple_call_lhs (stmt); + if (saved_val == NULL_TREE) + continue; + + insert_clobber_before_stack_restore (saved_val, var, &visited); + break; + } + + if (visited != NULL) + htab_delete (visited); + gcc_assert (save_found); +} + /* Detects a __builtin_alloca_with_align with constant size argument. Declares fixed-size array and returns the address, if found, otherwise returns NULL_TREE. */ @@ -1824,7 +1914,9 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi if (new_rhs) { bool res = update_call_from_tree (gsi, new_rhs); + tree var = TREE_OPERAND (TREE_OPERAND (new_rhs, 0),0); gcc_assert (res); + insert_clobbers_for_var (*gsi, var); return true; } } @@ -2024,12 +2116,14 @@ ccp_visit_stmt (gimple stmt, edge *taken static unsigned int do_ssa_ccp (void) { + unsigned int todo = 0; + calculate_dominance_info (CDI_DOMINATORS); ccp_initialize (); ssa_propagate (ccp_visit_stmt, ccp_visit_phi_node); if (ccp_finalize ()) - return (TODO_cleanup_cfg | TODO_update_ssa | TODO_remove_unused_locals); - else - return 0; + todo = (TODO_cleanup_cfg | TODO_update_ssa | TODO_remove_unused_locals); + free_dominance_info (CDI_DOMINATORS); + return todo; } Index: gcc/testsuite/gcc.dg/pr51491-2.c =================================================================== --- /dev/null (new file) +++ gcc/testsuite/gcc.dg/pr51491-2.c (revision 0) @@ -0,0 +1,34 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-ccp1" } */ + +int g (int *); + +int +f (int n) +{ + int tt = 0; + int t = 4; + { + int a[t + + (tt != 0 ? 6 : 0) + ]; + tt = g (a); + { + int b[n]; + tt += g (b); + if (n > 20) + tt += 148 * g (b); + tt += b[0]; + } + tt += a[0]; + } + { + int a[4]; + tt += g (a); + tt += a[0]; + } + return tt; +} + +/* { dg-final { scan-tree-dump-times "CLOBBER" 2 "ccp1"} } */ +/* { dg-final { cleanup-treee-dump "ccp1" } } */ Index: gcc/testsuite/gcc.dg/pr51491.c =================================================================== --- /dev/null (new file) +++ gcc/testsuite/gcc.dg/pr51491.c (revision 0) @@ -0,0 +1,25 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-rtl-expand" } */ + + +int g(int*); + +int f(void) +{ + int tt = 0; + int t = 4; + { + int a[t]; + tt = g(a); + tt += a[0]; + } + { + int a[4]; + tt += g(a); + tt += a[0]; + } + return tt; +} + +/* { dg-final { scan-rtl-dump-times "Partition" 1 "expand"} } */ +/* { dg-final { cleanup-rtl-dump "expand" } } */