From patchwork Fri Jun 3 01:32:43 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexandre Oliva X-Patchwork-Id: 98506 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 5E2ADB6F5F for ; Fri, 3 Jun 2011 11:37:14 +1000 (EST) Received: (qmail 4457 invoked by alias); 3 Jun 2011 01:37:12 -0000 Received: (qmail 4447 invoked by uid 22791); 3 Jun 2011 01:37:10 -0000 X-SWARE-Spam-Status: No, hits=-5.7 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, SPF_HELO_PASS, TW_TM, T_RP_MATCHES_RCVD, 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; Fri, 03 Jun 2011 01:36:52 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p531aqHH023594 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 2 Jun 2011 21:36:52 -0400 Received: from freie.oliva.athome.lsd.ic.unicamp.br (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p531aksd000836 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Thu, 2 Jun 2011 21:36:51 -0400 Received: from livre.localdomain (livre-to-gw.oliva.athome.lsd.ic.unicamp.br [172.31.160.19]) by freie.oliva.athome.lsd.ic.unicamp.br (8.14.4/8.14.4) with ESMTP id p531ahIq007124 for ; Thu, 2 Jun 2011 22:36:45 -0300 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 p531WiCm008624; Thu, 2 Jun 2011 22:32:44 -0300 Received: (from aoliva@localhost) by livre.localdomain (8.14.3/8.14.3/Submit) id p531WhOo008622; Thu, 2 Jun 2011 22:32:43 -0300 From: Alexandre Oliva To: gcc-patches@gcc.gnu.org Subject: Re: [PR48866] three alternative fixes In-Reply-To: (Alexandre Oliva's message of "Mon, 30 May 2011 09:11:25 -0300") References: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) Date: Thu, 02 Jun 2011 22:32:43 -0300 Message-ID: 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 Ugh, failed to refresh the patch file, resending with the correct one. On May 30, 2011, Alexandre Oliva wrote: > On May 30, 2011, Alexandre Oliva wrote: >> 2. emit placeholders for replaceable DEFs and, when the DEFs are >> expanded at their point of use, emit the expansion next to the >> placeholder, rather than at the current stream. The result of the >> expansion is saved and used in debug insns that reference the >> replaceable DEF. If the result is forced into a REG shortly thereafter, >> the code resulting from this is also emitted next to the placeholder, >> and the saved expansion is updated. If the USE is expanded before the >> DEF, the insn stream resulting from the expansion is saved and emitted >> at the point of the DEF. > IMHO this is the riskiest of the 3 patches, for shuffling expansions > around isn't exactly something I'm comfortable with. There's a very > real risk that moving the expansion of sub-expressions to their > definition points may end up moving uses before definitions. Upthread, I posted the wrong patch: instead of the one that tolerated expanding DEFs before or after USEs, I posted a simplifying experiment that seemed to fail, but it looks like I misinterpreted the results. This revised and retested patch also records expansions in an array rather than a pointer_map, and it avoids re-expanding DEFs when a USE is expanded for the second time. Although replaceable DEFs can only have one USE, when the single USE appears in a call stmt, it can be expanded twice. I'm not sure whether it would be better to expand it twice and let RTL optimizations drop any redundancies, or reuse the result of the first expansion, like this patch does. for gcc/ChangeLog from Alexandre Oliva PR debug/48866 * cfgexpand.c (def_expansions): New. (def_expansion_recent_tree, def_expansion_recent_rtx): New. (def_expansions_init): New. (def_expansions_remove_placeholder, def_expansions_fini): New. (def_get_expansion_ptr): New. (def_expansion_recent, def_expansion_record_recent): New. (def_expansion_add_insns): New. (expand_debug_expr): Use recorded expansion if available. (expand_gimple_basic_block): Prepare to record expansion of replaceable defs. Reset recent expansions at the end of the block. (gimple_expand_cfg): Initialize and finalize expansions cache. * expr.c: Include gimple-pretty-print.h. (store_expr): Forget recent expansions upon nontemporal moves. (expand_expr_real_1): Reuse or record expansion of replaceable defs. * expr.h (def_get_expansion_ptr, def_expansion_recent): Declare. (def_expansion_record_recent, def_expansion_add_insns): Declare. * explow.c (force_recent): New. (force_reg): Use it. Split into... (force_reg_1): ... this. * Makefile.in (expr.o): Depend on gimple-pretty-print.h. Index: gcc/cfgexpand.c =================================================================== --- gcc/cfgexpand.c.orig 2011-06-02 16:43:03.596818720 -0300 +++ gcc/cfgexpand.c 2011-06-02 17:18:10.217974612 -0300 @@ -2337,6 +2337,144 @@ convert_debug_memory_address (enum machi return x; } +/* Map replaceable SSA_NAMEs to NOTE_INSN_VAR_LOCATIONs that hold + their RTL expansions (once available) in their NOTE_VAR_LOCATIONs + (without a VAR_LOCATION rtx). The SSA_NAME DEF is expanded before + its single USE, so the NOTE is inserted in the insn stream, marking + the location where the non-replaceable portion of the expansion is + to be inserted. When the single USE is expanded, it will be + emitted before the NOTE. */ +static rtx *def_expansions; + +/* The latest expanded SSA name, and its corresponding RTL expansion. + These are used to enable the insertion of the insn that stores the + expansion in a register at the end of the sequence expanded for the + SSA DEF. */ +static tree def_expansion_recent_tree; +static rtx def_expansion_recent_rtx; + +/* Initialize the def_expansions data structure. This is to be called + before expansion of a function starts. */ + +static void +def_expansions_init (void) +{ + gcc_checking_assert (!def_expansions); + def_expansions = XCNEWVEC (rtx, num_ssa_names); + + gcc_checking_assert (!def_expansion_recent_tree); + gcc_checking_assert (!def_expansion_recent_rtx); +} + +/* Remove the NOTE that marks the insertion location of the expansion + of a replaceable SSA note. */ + +static bool +def_expansions_remove_placeholder (rtx note) +{ + if (!note) + return true; + + gcc_checking_assert (NOTE_P (note)); + remove_insn (note); + + return true; +} + +/* Finalize the def_expansions data structure. This is to be called + at the end of the expansion of a function. */ + +static void +def_expansions_fini (void) +{ + int i = num_ssa_names; + + gcc_checking_assert (def_expansions); + + while (i--) + if (def_expansions[i]) + def_expansions_remove_placeholder (def_expansions[i]); + XDELETEVEC (def_expansions); + def_expansions = NULL; + def_expansion_recent_tree = NULL; + def_expansion_recent_rtx = NULL; +} + +/* Return a pointer to the NOTE marking the insertion point for the + expansion of EXP. EXP must be a replaceable SSA_NAME. */ + +rtx * +def_get_expansion_ptr (tree exp) +{ + gcc_checking_assert (def_expansions); + gcc_checking_assert (TREE_CODE (exp) == SSA_NAME); + gcc_checking_assert (bitmap_bit_p (SA.values, SSA_NAME_VERSION (exp))); + return &def_expansions[SSA_NAME_VERSION (exp)]; +} + +/* Return an SSA name, if any, that was recently expanded to the value + X. */ + +tree +def_expansion_recent (rtx x) +{ + if (!def_expansion_recent_rtx) + return NULL; + + if (x == def_expansion_recent_rtx + || rtx_equal_p (x, def_expansion_recent_rtx)) + return def_expansion_recent_tree; + + return NULL; +} + +/* Record that DEF was recently expanded to the value X. */ + +void +def_expansion_record_recent (tree def, rtx x) +{ + def_expansion_recent_tree = def; + def_expansion_recent_rtx = x; +} + +/* Forget recent expansions. */ + +void +def_expansion_reset_recent (void) +{ + def_expansion_record_recent (NULL, NULL); +} + +/* Add INSNS to the insn seq generated for DEF, and update the + RTL value of DEF to VAL. */ + +void +def_expansion_add_insns (tree def, rtx insns, rtx val) +{ + rtx note = *def_get_expansion_ptr (def); + + if (dump_file && (dump_flags & TDF_DETAILS)) + { + fprintf (dump_file, "\n;; (cont) "); + print_gimple_stmt (dump_file, SSA_NAME_DEF_STMT (def), 0, + TDF_SLIM | (dump_flags & TDF_LINENO)); + fprintf (dump_file, "\n"); + + print_rtl (dump_file, insns); + + fprintf (dump_file, "\n=> "); + print_rtl (dump_file, val); + } + + gcc_checking_assert (note); + emit_insn_before (insns, note); + gcc_checking_assert (NOTE_VAR_LOCATION (note)); + NOTE_VAR_LOCATION (note) = val; + + def_expansion_recent_tree = NULL; + def_expansion_recent_rtx = NULL; +} + /* Return an RTX equivalent to the value of the tree expression EXP. */ @@ -3131,7 +3269,20 @@ expand_debug_expr (tree exp) gimple g = get_gimple_for_ssa_name (exp); if (g) { - op0 = expand_debug_expr (gimple_assign_rhs_to_tree (g)); + rtx *xp = def_get_expansion_ptr (exp); + + if (xp) + { + rtx note = *xp; + gcc_checking_assert (NOTE_P (note)); + op0 = NOTE_VAR_LOCATION (note); + } + else + op0 = NULL; + + if (!op0) + op0 = expand_debug_expr (gimple_assign_rhs_to_tree (g)); + if (!op0) return NULL; } @@ -3621,22 +3772,34 @@ expand_gimple_basic_block (basic_block b def_operand_p def_p; def_p = SINGLE_SSA_DEF_OPERAND (stmt, SSA_OP_DEF); - if (def_p != NULL) + /* Ignore this stmt if it is in the list of + replaceable expressions. */ + if (def_p != NULL + && SA.values + && bitmap_bit_p (SA.values, + SSA_NAME_VERSION (DEF_FROM_PTR (def_p)))) { - /* Ignore this stmt if it is in the list of - replaceable expressions. */ - if (SA.values - && bitmap_bit_p (SA.values, - SSA_NAME_VERSION (DEF_FROM_PTR (def_p)))) - continue; + tree def = DEF_FROM_PTR (def_p); + rtx *xp = def_get_expansion_ptr (def); + rtx note; + + last = get_last_insn (); + + gcc_checking_assert (!*xp); + + note = emit_note (NOTE_INSN_VAR_LOCATION); + NOTE_VAR_LOCATION (note) = NULL; + *xp = note; } - last = expand_gimple_stmt (stmt); + else + last = expand_gimple_stmt (stmt); maybe_dump_rtl_for_gimple_stmt (stmt, last); } } } currently_expanding_gimple_stmt = NULL; + def_expansion_reset_recent (); /* Expand implicit goto and convert goto_locus. */ FOR_EACH_EDGE (e, ei, bb->succs) @@ -4112,11 +4275,14 @@ gimple_expand_cfg (void) e->flags &= ~EDGE_EXECUTABLE; lab_rtx_for_bb = pointer_map_create (); + def_expansions_init (); + FOR_BB_BETWEEN (bb, init_block->next_bb, EXIT_BLOCK_PTR, next_bb) bb = expand_gimple_basic_block (bb); if (MAY_HAVE_DEBUG_INSNS) expand_debug_locations (); + def_expansions_fini (); execute_free_datastructures (); timevar_push (TV_OUT_OF_SSA); Index: gcc/expr.c =================================================================== --- gcc/expr.c.orig 2011-06-02 02:16:06.027421293 -0300 +++ gcc/expr.c 2011-06-02 18:15:47.941701093 -0300 @@ -48,6 +48,7 @@ along with GCC; see the file COPYING3. #include "tree-iterator.h" #include "tree-pass.h" #include "tree-flow.h" +#include "gimple-pretty-print.h" #include "target.h" #include "timevar.h" #include "df.h" @@ -4675,6 +4676,9 @@ store_expr (tree exp, rtx target, int ca (call_param_p ? EXPAND_STACK_PARM : EXPAND_NORMAL), &alt_rtl); + /* Don't risk moving loads before stores. */ + if (!nontemporal) + def_expansion_reset_recent (); } /* If TEMP is a VOIDmode constant and the mode of the type of EXP is not @@ -8424,10 +8428,63 @@ expand_expr_real_1 (tree exp, rtx target && !SSA_NAME_IS_DEFAULT_DEF (exp) && (optimize || DECL_IGNORED_P (SSA_NAME_VAR (exp))) && stmt_is_replaceable_p (SSA_NAME_DEF_STMT (exp))) - g = SSA_NAME_DEF_STMT (exp); + { + g = SSA_NAME_DEF_STMT (exp); + if (g) + return expand_expr_real (gimple_assign_rhs_to_tree (g), + target, tmode, modifier, NULL); + } if (g) - return expand_expr_real (gimple_assign_rhs_to_tree (g), target, tmode, - modifier, NULL); + { + rtx retval; + rtx insns; + rtx note = *def_get_expansion_ptr (exp); + + gcc_assert (NOTE_P (note)); + /* Call parameters may be expanded twice. Reuse the result + of the first expansion. */ + if (NOTE_VAR_LOCATION (note)) + return NOTE_VAR_LOCATION (note); + + start_sequence (); + + retval = expand_expr_real (gimple_assign_rhs_to_tree (g), + target, tmode, modifier, NULL); + + insns = get_insns (); + + if (retval == target) + { + end_sequence (); + emit_insn (insns); + return retval; + } + + if (dump_file && (dump_flags & TDF_DETAILS)) + { + fprintf (dump_file, "\n;; "); + print_gimple_stmt (dump_file, g, 0, + TDF_SLIM | (dump_flags & TDF_LINENO)); + fprintf (dump_file, "\n"); + + print_rtl (dump_file, insns); + + fprintf (dump_file, "\n=> "); + print_rtl (dump_file, retval); + } + + end_sequence (); + + /* Make sure this is in the instruction stream. */ + gcc_checking_assert (PREV_INSN (note)); + emit_insn_before (insns, note); + gcc_checking_assert (!NOTE_VAR_LOCATION (note)); + + NOTE_VAR_LOCATION (note) = retval; + def_expansion_record_recent (exp, retval); + + return retval; + } ssa_name = exp; decl_rtl = get_rtx_for_ssa_name (ssa_name); Index: gcc/expr.h =================================================================== --- gcc/expr.h.orig 2011-06-02 02:16:06.332420537 -0300 +++ gcc/expr.h 2011-06-02 16:43:13.254766180 -0300 @@ -693,4 +693,11 @@ extern tree build_libfunc_function (cons /* Get the personality libfunc for a function decl. */ rtx get_personality_function (tree); +/* In cfgexpand.c. */ +rtx *def_get_expansion_ptr (tree); +tree def_expansion_recent (rtx); +void def_expansion_record_recent (tree, rtx); +void def_expansion_reset_recent (void); +void def_expansion_add_insns (tree, rtx, rtx); + #endif /* GCC_EXPR_H */ Index: gcc/explow.c =================================================================== --- gcc/explow.c.orig 2011-06-01 20:39:58.896953301 -0300 +++ gcc/explow.c 2011-06-02 16:43:13.352765648 -0300 @@ -638,22 +638,62 @@ copy_to_mode_reg (enum machine_mode mode return temp; } +/* If X is the value of a recently-expanded SSA def, emit the insns + generated by FN (MODE, X) at the end of the expansion of that def, + otherwise emit them in the current insn seq. */ + +static inline rtx +force_recent (enum machine_mode mode, rtx x, + rtx (*fn) (enum machine_mode, rtx)) +{ + tree def = def_expansion_recent (x); + rtx retval; + rtx insns; + + if (!def) + return fn (mode, x); + + start_sequence (); + retval = fn (mode, x); + insns = get_insns (); + end_sequence (); + + def_expansion_add_insns (def, insns, retval); + + return retval; +} + +static rtx force_reg_1 (enum machine_mode, rtx); + /* Load X into a register if it is not already one. Use mode MODE for the register. X should be valid for mode MODE, but it may be a constant which is valid for all integer modes; that's why caller must specify MODE. + If X is the value of a recent SSA def expansion, emit the insns at + the end of that expansion, rather than into the current seq. + The caller must not alter the value in the register we return, since we mark it as a "constant" register. */ rtx force_reg (enum machine_mode mode, rtx x) { - rtx temp, insn, set; - if (REG_P (x)) return x; + return force_recent (mode, x, force_reg_1); +} + +/* Load X into a register if it is not already one. + Use mode MODE for the register. + Internal implementation of force_reg. */ + +static rtx +force_reg_1 (enum machine_mode mode, rtx x) +{ + rtx temp, insn, set; + if (general_operand (x, mode)) { temp = gen_reg_rtx (mode); Index: gcc/Makefile.in =================================================================== --- gcc/Makefile.in.orig 2011-06-01 20:39:59.074953270 -0300 +++ gcc/Makefile.in 2011-06-02 16:43:13.484764930 -0300 @@ -2942,7 +2942,8 @@ except.o : except.c $(CONFIG_H) $(SYSTEM expr.o : expr.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(RTL_H) \ $(TREE_H) $(FLAGS_H) $(FUNCTION_H) $(REGS_H) $(EXPR_H) $(OPTABS_H) \ $(LIBFUNCS_H) $(INSN_ATTR_H) insn-config.h $(RECOG_H) output.h \ - typeclass.h hard-reg-set.h toplev.h $(DIAGNOSTIC_CORE_H) hard-reg-set.h $(EXCEPT_H) \ + typeclass.h hard-reg-set.h toplev.h $(DIAGNOSTIC_CORE_H) \ + gimple-pretty-print.h hard-reg-set.h $(EXCEPT_H) \ reload.h langhooks.h intl.h $(TM_P_H) $(TARGET_H) \ tree-iterator.h gt-expr.h $(MACHMODE_H) $(TIMEVAR_H) $(TREE_FLOW_H) \ $(TREE_PASS_H) $(DF_H) $(DIAGNOSTIC_H) vecprim.h $(SSAEXPAND_H)