From patchwork Thu Jul 22 10:44:08 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Jambor X-Patchwork-Id: 59561 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 C6208B70C8 for ; Thu, 22 Jul 2010 20:44:21 +1000 (EST) Received: (qmail 29220 invoked by alias); 22 Jul 2010 10:44:19 -0000 Received: (qmail 29209 invoked by uid 22791); 22 Jul 2010 10:44:17 -0000 X-SWARE-Spam-Status: No, hits=-3.7 required=5.0 tests=AWL,BAYES_00,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; Thu, 22 Jul 2010 10:44:12 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.221.2]) by mx2.suse.de (Postfix) with ESMTP id A643086A2E for ; Thu, 22 Jul 2010 12:44:09 +0200 (CEST) Date: Thu, 22 Jul 2010 12:44:08 +0200 From: Martin Jambor To: GCC Patches Cc: Richard Guenther Subject: [PATCH] Fix 44891 - required type conversion in load elimination in SRA Message-ID: <20100722104408.GB8513@virgil.arch.suse.de> Mail-Followup-To: GCC Patches , Richard Guenther MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.20 (2009-06-14) X-IsSubscribed: yes 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 Hi, with MEM_REF, the code in SRA that performs removal of loads from pieces of aggregates that are known to be uninitialized can produce statements with incompatible types when it creates a default-definition replacement SSA_NAME for register loads. In order to try to keep the code complexity at least a bit sane, I removed propagation of the LHS SSA_NAME with the replacement (fwprop should do that just fine) and I no longer actually remove the statement but rather simply adjust the RHS, potentially also generating type conversion. I have also added a simple message dump (and specifically want it to be also in the non-detailed dumps) that tells a load is being removed so that we can quickly see that this code path triggered when analyzing SRA output. I have bootstrapped and regression tested this patch on x86_64-linux without any problems, OK for trunk? Thanks, Martin 2010-07-21 Martin Jambor PR tree-optimization/44891 * tree-sra.c: Include gimple-pretty-print.h. (replace_uses_with_default_def_ssa_name): Renamed to get_repl_default_def_ssa_name, return the new SSA name instead of replacing the old one. (sra_modify_assign): Dump a message when removing a load, if the LHS is an SSA_NAME, do not do any propagation, just set the RHS to a default definition SSA NAME, type convert if necessary. * Makefile.in (tree-sra.o): Add gimple-pretty-print.h to dependencies. * testsuite/gcc.c-torture/compile/pr44891.c: New test. Index: mine/gcc/tree-sra.c =================================================================== --- mine.orig/gcc/tree-sra.c +++ mine/gcc/tree-sra.c @@ -90,6 +90,7 @@ along with GCC; see the file COPYING3. #include "flags.h" #include "dbgcnt.h" #include "tree-inline.h" +#include "gimple-pretty-print.h" /* Enumeration of all aggregate reductions we can do. */ enum sra_mode { SRA_MODE_EARLY_IPA, /* early call regularization */ @@ -2542,12 +2543,12 @@ sra_modify_constructor_assign (gimple *s } } -/* Create a new suitable default definition SSA_NAME and replace all uses of - SSA with it, RACC is access describing the uninitialized part of an - aggregate that is being loaded. */ +/* Create and return a new suitable default definition SSA_NAME for RACC which + is an access describing an uninitialized part of an aggregate that is being + loaded. */ -static void -replace_uses_with_default_def_ssa_name (tree ssa, struct access *racc) +static tree +get_repl_default_def_ssa_name (struct access *racc) { tree repl, decl; @@ -2560,7 +2561,7 @@ replace_uses_with_default_def_ssa_name ( set_default_def (decl, repl); } - replace_uses_by (ssa, repl); + return repl; } /* Examine both sides of the assignment statement pointed to by STMT, replace @@ -2737,18 +2738,33 @@ sra_modify_assign (gimple *stmt, gimple_ { if (!racc->grp_to_be_replaced && !racc->grp_unscalarized_data) { - if (racc->first_child) - generate_subtree_copies (racc->first_child, lhs, - racc->offset, 0, 0, gsi, - false, false); - gcc_assert (*stmt == gsi_stmt (*gsi)); - if (TREE_CODE (lhs) == SSA_NAME) - replace_uses_with_default_def_ssa_name (lhs, racc); + if (dump_file) + { + fprintf (dump_file, "Removing load: "); + print_gimple_stmt (dump_file, *stmt, 0, 0); + } - unlink_stmt_vdef (*stmt); - gsi_remove (gsi, true); - sra_stats.deleted++; - return SRA_AM_REMOVED; + if (TREE_CODE (lhs) == SSA_NAME) + { + rhs = get_repl_default_def_ssa_name (racc); + if (!useless_type_conversion_p (TREE_TYPE (lhs), + TREE_TYPE (rhs))) + rhs = fold_build1_loc (loc, VIEW_CONVERT_EXPR, + TREE_TYPE (lhs), rhs); + } + else + { + if (racc->first_child) + generate_subtree_copies (racc->first_child, lhs, + racc->offset, 0, 0, gsi, + false, false); + + gcc_assert (*stmt == gsi_stmt (*gsi)); + unlink_stmt_vdef (*stmt); + gsi_remove (gsi, true); + sra_stats.deleted++; + return SRA_AM_REMOVED; + } } else if (racc->first_child) generate_subtree_copies (racc->first_child, lhs, Index: mine/gcc/Makefile.in =================================================================== --- mine.orig/gcc/Makefile.in +++ mine/gcc/Makefile.in @@ -3132,7 +3132,7 @@ tree-sra.o : tree-sra.c $(CONFIG_H) $(SY $(TM_H) $(TREE_H) $(GIMPLE_H) $(CGRAPH_H) $(TREE_FLOW_H) $(IPA_PROP_H) \ $(DIAGNOSTIC_H) statistics.h $(TREE_DUMP_H) $(TIMEVAR_H) $(PARAMS_H) \ $(TARGET_H) $(FLAGS_H) $(EXPR_H) tree-pretty-print.h $(DBGCNT_H) \ - $(TREE_INLINE_H) + $(TREE_INLINE_H) gimple-pretty-print.h tree-switch-conversion.o : tree-switch-conversion.c $(CONFIG_H) $(SYSTEM_H) \ $(TREE_H) $(TM_P_H) $(TREE_FLOW_H) $(DIAGNOSTIC_H) $(TREE_INLINE_H) \ $(TIMEVAR_H) $(TM_H) coretypes.h $(TREE_DUMP_H) $(GIMPLE_H) \ Index: mine/gcc/testsuite/gcc.c-torture/compile/pr44891.c =================================================================== --- /dev/null +++ mine/gcc/testsuite/gcc.c-torture/compile/pr44891.c @@ -0,0 +1,26 @@ +struct S +{ + float f; + long l; +}; + +extern int gi; +extern float gf; + +long foo (long p) +{ + struct S s; + float *pf; + + s.l = p; + + pf = &s.f; + + pf++; + pf--; + + gf = *pf + 3.3; + gi = *((int *)pf) + 2; + + return s.l + 6; +}