From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23525 invoked by alias); 19 Oct 2011 10:09:04 -0000 Received: (qmail 23510 invoked by uid 22791); 19 Oct 2011 10:09:02 -0000 X-SWARE-Spam-Status: No, hits=-3.4 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,TW_CP,TW_TM X-Spam-Check-By: sourceware.org Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 19 Oct 2011 10:08:29 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.221.2]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id CC8328A908 for ; Wed, 19 Oct 2011 12:08:27 +0200 (CEST) Date: Wed, 19 Oct 2011 12:14:00 -0000 From: Richard Guenther To: gcc-patches@gcc.gnu.org Subject: [PATCH] Fix PR50768, rewrite gimplify_and_update_call_from_tree Message-ID: User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII 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 X-SW-Source: 2011-10/txt/msg01714.txt.bz2 So my previous patch has caused some bugs - not unreasonable given the complexity of that function. Thus the following patch makes it readable and understandable again, by exchanging the intricate one-stage processing with a two-stage one, first assigning VDEFs and then assigning VUSEs in a second stage. Also doing the lhs assignment processing upfront which removes the need for all the duplicate code at the tail. Bootstrap and regtest ongoing on x86_64-unknown-linux-gnu, let's hope it can stay this simple ;) Thanks, Richard. 2011-10-19 Richard Guenther PR middle-end/50768 * gimple-fold.c (gimplify_and_update_call_from_tree): Rewrite. * gcc.dg/torture/pr50768.c: New testcase. Index: gcc/gimple-fold.c =================================================================== *** gcc/gimple-fold.c (revision 180166) --- gcc/gimple-fold.c (working copy) *************** void *** 534,558 **** gimplify_and_update_call_from_tree (gimple_stmt_iterator *si_p, tree expr) { tree lhs; - tree tmp = NULL_TREE; /* Silence warning. */ gimple stmt, new_stmt; gimple_stmt_iterator i; gimple_seq stmts = gimple_seq_alloc(); struct gimplify_ctx gctx; ! gimple last = NULL; ! gimple laststore = NULL; tree reaching_vuse; stmt = gsi_stmt (*si_p); gcc_assert (is_gimple_call (stmt)); - lhs = gimple_call_lhs (stmt); - reaching_vuse = gimple_vuse (stmt); - push_gimplify_context (&gctx); gctx.into_ssa = gimple_in_ssa_p (cfun); if (lhs == NULL_TREE) { gimplify_and_add (expr, &stmts); --- 534,555 ---- gimplify_and_update_call_from_tree (gimple_stmt_iterator *si_p, tree expr) { tree lhs; gimple stmt, new_stmt; gimple_stmt_iterator i; gimple_seq stmts = gimple_seq_alloc(); struct gimplify_ctx gctx; ! gimple last; ! gimple laststore; tree reaching_vuse; stmt = gsi_stmt (*si_p); gcc_assert (is_gimple_call (stmt)); push_gimplify_context (&gctx); gctx.into_ssa = gimple_in_ssa_p (cfun); + lhs = gimple_call_lhs (stmt); if (lhs == NULL_TREE) { gimplify_and_add (expr, &stmts); *************** gimplify_and_update_call_from_tree (gimp *** 571,675 **** } } else ! tmp = get_initialized_tmp_var (expr, &stmts, NULL); pop_gimplify_context (NULL); if (gimple_has_location (stmt)) annotate_all_with_location (stmts, gimple_location (stmt)); ! /* The replacement can expose previously unreferenced variables. */ for (i = gsi_start (stmts); !gsi_end_p (i); gsi_next (&i)) { if (last) { gsi_insert_before (si_p, last, GSI_NEW_STMT); gsi_next (si_p); } new_stmt = gsi_stmt (i); if (gimple_in_ssa_p (cfun)) find_new_referenced_vars (new_stmt); /* If the new statement possibly has a VUSE, update it with exact SSA name we know will reach this one. */ if (gimple_has_mem_ops (new_stmt)) ! { ! /* If we've also seen a previous store create a new VDEF for ! the latter one, and make that the new reaching VUSE. */ ! if (laststore) ! { ! reaching_vuse = make_ssa_name (gimple_vop (cfun), laststore); ! gimple_set_vdef (laststore, reaching_vuse); ! update_stmt (laststore); ! laststore = NULL; ! } ! gimple_set_vuse (new_stmt, reaching_vuse); ! gimple_set_modified (new_stmt, true); ! } ! if (gimple_assign_single_p (new_stmt) ! && !is_gimple_reg (gimple_assign_lhs (new_stmt))) ! { ! laststore = new_stmt; ! } last = new_stmt; } ! if (lhs == NULL_TREE) { ! /* If we replace a call without LHS that has a VDEF and our new ! sequence ends with a store we must make that store have the same ! vdef in order not to break the sequencing. This can happen ! for instance when folding memcpy calls into assignments. */ ! if (gimple_vdef (stmt) && laststore) ! { ! gimple_set_vdef (laststore, gimple_vdef (stmt)); ! if (TREE_CODE (gimple_vdef (stmt)) == SSA_NAME) ! SSA_NAME_DEF_STMT (gimple_vdef (stmt)) = laststore; ! update_stmt (laststore); ! } ! else if (gimple_in_ssa_p (cfun)) { unlink_stmt_vdef (stmt); ! release_defs (stmt); ! } ! new_stmt = last; ! } ! else ! { ! if (last) ! { ! gsi_insert_before (si_p, last, GSI_NEW_STMT); ! gsi_next (si_p); ! } ! if (laststore && is_gimple_reg (lhs)) ! { ! gimple_set_vdef (laststore, gimple_vdef (stmt)); ! update_stmt (laststore); ! if (TREE_CODE (gimple_vdef (stmt)) == SSA_NAME) ! SSA_NAME_DEF_STMT (gimple_vdef (stmt)) = laststore; ! laststore = NULL; ! } ! else if (laststore) ! { ! reaching_vuse = make_ssa_name (gimple_vop (cfun), laststore); ! gimple_set_vdef (laststore, reaching_vuse); ! update_stmt (laststore); ! laststore = NULL; ! } ! new_stmt = gimple_build_assign (lhs, tmp); ! if (!is_gimple_reg (tmp)) ! gimple_set_vuse (new_stmt, reaching_vuse); ! if (!is_gimple_reg (lhs)) ! { ! gimple_set_vdef (new_stmt, gimple_vdef (stmt)); ! if (TREE_CODE (gimple_vdef (stmt)) == SSA_NAME) ! SSA_NAME_DEF_STMT (gimple_vdef (stmt)) = new_stmt; } - else if (reaching_vuse == gimple_vuse (stmt)) - unlink_stmt_vdef (stmt); } ! gimple_set_location (new_stmt, gimple_location (stmt)); ! gsi_replace (si_p, new_stmt, false); } /* Return the string length, maximum string length or maximum value of --- 568,648 ---- } } else ! { ! tree tmp = get_initialized_tmp_var (expr, &stmts, NULL); ! new_stmt = gimple_build_assign (lhs, tmp); ! i = gsi_last (stmts); ! gsi_insert_after_without_update (&i, new_stmt, ! GSI_CONTINUE_LINKING); ! } pop_gimplify_context (NULL); if (gimple_has_location (stmt)) annotate_all_with_location (stmts, gimple_location (stmt)); ! /* First iterate over the replacement statements backward, assigning ! virtual operands to their defining statements. */ ! laststore = NULL; ! for (i = gsi_last (stmts); !gsi_end_p (i); gsi_prev (&i)) ! { ! new_stmt = gsi_stmt (i); ! if (gimple_assign_single_p (new_stmt) ! && !is_gimple_reg (gimple_assign_lhs (new_stmt))) ! { ! tree vdef; ! if (!laststore) ! vdef = gimple_vdef (stmt); ! else ! vdef = make_ssa_name (gimple_vop (cfun), new_stmt); ! gimple_set_vdef (new_stmt, vdef); ! SSA_NAME_DEF_STMT (vdef) = new_stmt; ! laststore = new_stmt; ! } ! } ! ! /* Second iterate over the statements forward, assigning virtual ! operands to their uses. */ ! last = NULL; ! reaching_vuse = gimple_vuse (stmt); for (i = gsi_start (stmts); !gsi_end_p (i); gsi_next (&i)) { + /* Do not insert the last stmt in this loop but remember it + for replacing the original statement. */ if (last) { gsi_insert_before (si_p, last, GSI_NEW_STMT); gsi_next (si_p); } new_stmt = gsi_stmt (i); + /* The replacement can expose previously unreferenced variables. */ if (gimple_in_ssa_p (cfun)) find_new_referenced_vars (new_stmt); /* If the new statement possibly has a VUSE, update it with exact SSA name we know will reach this one. */ if (gimple_has_mem_ops (new_stmt)) ! gimple_set_vuse (new_stmt, reaching_vuse); ! gimple_set_modified (new_stmt, true); ! if (gimple_vdef (new_stmt)) ! reaching_vuse = gimple_vdef (new_stmt); last = new_stmt; } ! /* If the new sequence does not do a store release the virtual ! definition of the original statement. */ ! if (reaching_vuse ! && reaching_vuse == gimple_vuse (stmt)) { ! tree vdef = gimple_vdef (stmt); ! if (vdef) { unlink_stmt_vdef (stmt); ! release_ssa_name (vdef); } } ! /* Finally replace rhe original statement with the last. */ ! gsi_replace (si_p, last, false); } /* Return the string length, maximum string length or maximum value of Index: gcc/testsuite/gcc.dg/torture/pr50768.c =================================================================== *** gcc/testsuite/gcc.dg/torture/pr50768.c (revision 0) --- gcc/testsuite/gcc.dg/torture/pr50768.c (revision 0) *************** *** 0 **** --- 1,17 ---- + /* { dg-do compile } */ + /* { dg-options "-ftracer" } */ + + char data[8]; + int l1; + + void + test1 (void) + { + char buf[8]; + __builtin___mempcpy_chk (buf, data, l1 ? sizeof (buf) : 4, + __builtin_object_size (buf, 0)); + if (__builtin___memmove_chk + (buf, data, l1 ? sizeof (buf) : 4, + __builtin_object_size (buf, 0)) != buf) + __builtin_abort (); + }