This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Combine location with block using block_locations
- From: Richard Guenther <richard dot guenther at gmail dot com>
- To: Dehao Chen <dehao at google dot com>
- Cc: Diego Novillo <dnovillo at google dot com>, Dodji Seketeli <dodji at redhat dot com>, gcc-patches at gcc dot gnu dot org, Jakub Jelinek <jakub at redhat dot com>, Jan Hubicka <hubicka at ucw dot cz>, David Li <davidxl at google dot com>, Tom Tromey <tromey at redhat dot com>, Jason Merrill <jason at redhat dot com>, Richard Henderson <rth at redhat dot com>
- Date: Mon, 10 Sep 2012 12:01:24 +0200
- Subject: Re: [PATCH] Combine location with block using block_locations
- References: <CAO2gOZXFkCpuFqXpPw6ju14BB=RBdt-kDhKsJgk4N41=Q19VHw@mail.gmail.com> <CAO2gOZXHVkaW41KN3E4viQEe2bn4bP_LLt-VbmBB3zboPyU_sg@mail.gmail.com> <CAFiYyc0KmntyrbOReSCjLLL0Z-OeYn=6dZZ2AmV7JPgtEV6UbQ@mail.gmail.com> <CAO2gOZXDVyfu15Y=QL74Lq+CP-fJXXdhR=53+sU5-KyN2haDqg@mail.gmail.com> <CAFiYyc02+Ahg-RC5JjJtBDeQPMmUXruaY6yKAi8yz_6ftagHgw@mail.gmail.com> <CAO2gOZV6nHjTiTEtrt64r_Hc5_CGZTqYGMRDXwWULdO5AX43Jg@mail.gmail.com> <m3has6od2w.fsf@redhat.com> <CAO2gOZWbCiFH0TJxxAfzqpZyuE=kRsrtCc0k+WjAoYaafhkSUA@mail.gmail.com> <m3y5lhnbzi.fsf@redhat.com> <CAO2gOZVFTNraCNOS4J1dV9T7XyFY=QHbhZ_i3Jx9yY6EM5+46w@mail.gmail.com> <CAO2gOZWsX_SP+zZX5t_xnXUUVSCz0WpRQy=oJumKY3_mDCXGSA@mail.gmail.com> <CAFiYyc0X7kGj95SvQfBpDxY=UMvyDXCCALwFTXXn+4oYJ8x04g@mail.gmail.com> <CAO2gOZWG_Gw3MejvA58M1GrXj=7s6fkURX3NmGT=cUgMwHz=RQ@mail.gmail.com> <CAFiYyc0c3ZyLkbTGiiOOi0sCC0Ju9=PQLZ-BAOV2RFYVttzvCA@mail.gmail.com> <5049D5AD.5010405@google.com> <CAO2gOZXf+dKd8DP++apv0AuUVxEWnVJC5_Z7z3md5fShH9fYYA@mail.gmail.com> <504A2EE0.8020704@google.com> <CAO2gOZWX+A3R0MGvWXYztBokQ=ezHNiS3vR0Cu9cbGPi_102ww@mail.gmail.com>
On Sun, Sep 9, 2012 at 12:26 AM, Dehao Chen <dehao@google.com> wrote:
> Hi, Diego,
>
> Thanks a lot for the review. I've updated the patch.
>
> This patch is large and may easily break builds because it reserves
> more complete information for TREE_BLOCK as well as gimple_block (may
> trigger bugs that was hided when these info are unavailable). I've
> done more rigorous testing to ensure that most bugs are caught before
> checking in.
>
> * Sync to the head and retest all gcc testsuite.
> * Port the patch to google-4_7 branch to retest all gcc testsuite, as
> well as build many large applications.
>
> Through these tests, I've found two additional bugs that was omitted
> in the original implementation. A new patch is attached (patch.txt) to
> fix these problems. After this fix, all gcc testsuites pass for both
> trunk and google-4_7 branch. I've also copy pasted the new fixes
> (lto.c and tree-cfg.c) below. Now I'd say this patch is in good shape.
> But it may not be perfect. I'll look into build failures as soon as it
> arises.
>
> Richard and Diego, could you help me take a look at the following two fixes?
>
> Thanks,
> Dehao
>
> New fixes:
> --- gcc/lto/lto.c (revision 191083)
> +++ gcc/lto/lto.c (working copy)
> @@ -1559,8 +1559,6 @@ lto_fixup_prevailing_decls (tree t)
> {
> enum tree_code code = TREE_CODE (t);
> LTO_NO_PREVAIL (TREE_TYPE (t));
> - if (CODE_CONTAINS_STRUCT (code, TS_COMMON))
> - LTO_NO_PREVAIL (TREE_CHAIN (t));
That change is odd. Can you show us how it breaks?
> if (DECL_P (t))
> {
> LTO_NO_PREVAIL (DECL_NAME (t));
>
> Index: gcc/tree-cfg.c
> ===================================================================
> --- gcc/tree-cfg.c (revision 191083)
> +++ gcc/tree-cfg.c (working copy)
> @@ -5980,9 +5974,21 @@ move_stmt_op (tree *tp, int *walk_subtrees, void *
> tree t = *tp;
>
> if (EXPR_P (t))
> - /* We should never have TREE_BLOCK set on non-statements. */
> - gcc_assert (!TREE_BLOCK (t));
> -
> + {
> + tree block = TREE_BLOCK (t);
> + if (p->orig_block == NULL_TREE
> + || block == p->orig_block
> + || block == NULL_TREE)
> + TREE_SET_BLOCK (t, p->new_block);
> +#ifdef ENABLE_CHECKING
> + else if (block != p->new_block)
> + {
> + while (block && block != p->orig_block)
> + block = BLOCK_SUPERCONTEXT (block);
> + gcc_assert (block);
> + }
> +#endif
I think what this means is that TREE_BLOCK on non-stmts are meaningless
(thus only gimple_block is interesting on GIMPLE, not BLOCKs on trees).
So instead of setting a BLOCK in some cases you should clear BLOCK
if it happens to be set, or alternatively, only re-set it if there was
a block associated
with it.
Richard.
> + }
> else if (DECL_P (t) || TREE_CODE (t) == SSA_NAME)
> {
> if (TREE_CODE (t) == SSA_NAME)
>
> Whole patch:
> gcc/ChangeLog:
> 2012-09-08 Dehao Chen <dehao@google.com>
>
> * toplev.c (general_init): Init block_locations.
> * tree.c (tree_set_block): New.
> (tree_block): Change to use LOCATION_BLOCK.
> * tree.h (TREE_SET_BLOCK): New.
> * final.c (reemit_insn_block_notes): Change to use LOCATION_BLOCK.
> (final_start_function): Likewise.
> * input.c (expand_location_1): Likewise.
> * input.h (LOCATION_LOCUS): New.
> (LOCATION_BLOCK): New.
> (IS_UNKNOWN_LOCATION): New.
> * fold-const.c (expr_location_or): Change to use new location.
> * reorg.c (emit_delay_sequence): Likewise.
> (try_merge_delay_insns): Likewise.
> * modulo-sched.c (dump_insn_location): Likewise.
> * lto-streamer-out.c (lto_output_location_bitpack): Likewise.
> * jump.c (rtx_renumbered_equal_p): Likewise.
> * ifcvt.c (noce_try_move): Likewise.
> (noce_try_store_flag): Likewise.
> (noce_try_store_flag_constants): Likewise.
> (noce_try_addcc): Likewise.
> (noce_try_store_flag_mask): Likewise.
> (noce_try_cmove): Likewise.
> (noce_try_cmove_arith): Likewise.
> (noce_try_minmax): Likewise.
> (noce_try_abs): Likewise.
> (noce_try_sign_mask): Likewise.
> (noce_try_bitop): Likewise.
> (noce_process_if_block): Likewise.
> (cond_move_process_if_block): Likewise.
> (find_cond_trap): Likewise.
> * dwarf2out.c (add_src_coords_attributes): Likewise.
> * expr.c (expand_expr_real): Likewise.
> * tree-parloops.c (create_loop_fn): Likewise.
> * recog.c (peep2_attempt): Likewise.
> * function.c (free_after_compilation): Likewise.
> (expand_function_end): Likewise.
> (set_insn_locations): Likewise.
> (thread_prologue_and_epilogue_insns): Likewise.
> * print-rtl.c (print_rtx): Likewise.
> * profile.c (branch_prob): Likewise.
> * trans-mem.c (ipa_tm_scan_irr_block): Likewise.
> * gimplify.c (gimplify_call_expr): Likewise.
> * except.c (duplicate_eh_regions_1): Likewise.
> * emit-rtl.c (try_split): Likewise.
> (make_insn_raw): Likewise.
> (make_debug_insn_raw): Likewise.
> (make_jump_insn_raw): Likewise.
> (make_call_insn_raw): Likewise.
> (emit_pattern_after_setloc): Likewise.
> (emit_pattern_after): Likewise.
> (emit_debug_insn_after): Likewise.
> (emit_pattern_before): Likewise.
> (emit_insn_before_setloc): Likewise.
> (emit_jump_insn_before): Likewise.
> (emit_call_insn_before_setloc): Likewise.
> (emit_call_insn_before): Likeise.
> (emit_debug_insn_before_setloc): Likewise.
> (emit_copy_of_insn_after): Likewise.
> (insn_locators_alloc): Remove.
> (insn_locators_finalize): Remove.
> (insn_locators_free): Remove.
> (set_curr_insn_source_location): Remove.
> (get_curr_insn_source_location): Remove.
> (set_curr_insn_block): Remove.
> (get_curr_insn_block): Remove.
> (locator_scope): Remove.
> (insn_scope): Change to use new location.
> (locator_location): Remove.
> (insn_line): Change to use new location.
> (locator_file): Remove.
> (insn_file): Change to use new location.
> (locator_eq): Remove.
> (insn_locations_init): New.
> (insn_locations_finalize): New.
> (set_curr_insn_location): New.
> (curr_insn_location): New.
> * cfgexpand.c (gimple_assign_rhs_to_tree): Change to use new location.
> (expand_gimple_cond): Likewise.
> (expand_call_stmt): Likewise.
> (expand_gimple_stmt_1): Likewise.
> (expand_gimple_basic_block): Likewise.
> (construct_exit_block): Likewise.
> (gimple_expand_cfg): Likewise.
> * cfgcleanup.c (try_forward_edges): Likewise.
> * tree-ssa-live.c (remove_unused_scope_block_p): Likewise.
> (dump_scope_block): Likewise.
> (remove_unused_locals): Likewise.
> * rtl.c (rtx_equal_p_cb): Likewise.
> (rtx_equal_p): Likewise.
> * rtl.h (XUINT): New.
> (INSN_LOCATOR): Remove.
> (CURR_INSN_LOCATION): Remove.
> (INSN_LOCATION): New.
> (INSN_HAS_LOCATION): New.
> * tree-inline.c (remap_gimple_op_r): Change to use new location.
> (copy_tree_body_r): Likewise.
> (copy_phis_for_bb): Likewise.
> (expand_call_inline): Likewise.
> * tree-streamer-in.c (lto_input_ts_exp_tree_pointers): Likewise.
> * tree-streamer-out.c (write_ts_decl_minimal_tree_pointers): Likewise.
> * gimple-streamer-out.c (output_gimple_stmt): Likewise.
> * combine.c (try_combine): Likewise.
> * tree-outof-ssa.c (set_location_for_edge): Likewise.
> (insert_partition_copy_on_edge): Likewise.
> (insert_value_copy_on_edge): Likewise.
> (insert_rtx_to_part_on_edge): Likewise.
> (insert_part_to_rtx_on_edge): Likewise.
> * basic-block.h (edge_def): Remove field.
> * gimple.h (gimple_statement_base): Remove field.
> (gimple_bb): Change to use new location.
> (gimple_set_block): Likewise.
> (gimple_has_location): Likewise.
> * tree-cfg.c (make_cond_expr_edges): Likewise.
> (make_goto_expr_edges): Likewise.
> (gimple_can_merge_blocks_p): Likewise.
> (move_stmt_op): Likewise.
> (move_block_to_fn): Likewise.
> * config/alpha/alpha.c (alpha_output_mi_thunk_osf): Likewise.
> * config/sparc/sparc.c (sparc_output_mi_thunk): Likewise.
> * config/i386/i386.c (x86_output_mi_thunk): Likewise.
> * config/tilegx/tilegx.c (tilegx_output_mi_thunk): Likewise.
> * config/sh/sh.c (sh_output_mi_thunk): Likewise.
> * config/ia64/ia64.c (ia64_output_mi_thunk): Likewise.
> * config/rs6000/rs6000.c (rs6000_output_mi_thunk): Likewise.
> * config/score/score.c (score_output_mi_thunk): Likewise.
> * config/tilepro/tilepro.c (tilepro_asm_output_mi_thunk): Likewise.
> * config/mips/mips.c (mips_output_mi_thunk): Likewise.
> * cfgrtl.c (unique_locus_on_edge_between_p): Likewise.
> (unique_locus_on_edge_between_p): Likewise.
> (emit_nop_for_unique_locus_between): Likewise.
> (force_nonfallthru_and_redirect): Likewise.
> (fixup_reorder_chain): Likewise.
> (cfg_layout_merge_blocks): Likewise.
> * stmt.c (emit_case_nodes): Likewise.
>
> gcc/lto/ChangeLog:
> 2012-09-08 Dehao Chen <dehao@google.com>
>
> * lto/lto.c (lto_fixup_prevailing_decls): Remove tree.exp.block field.
>
> libcpp/ChangeLog:
> 2012-09-08 Dehao Chen <dehao@google.com>
>
> * include/line-map.h (MAX_SOURCE_LOCATION): New value.
> (location_adhoc_data_init): New.
> (location_adhoc_data_fini): New.
> (get_combined_adhoc_loc): New.
> (get_data_from_adhoc_loc): New.
> (get_location_from_adhoc_loc): New.
> (COMBINE_LOCATION_DATA): New.
> (IS_ADHOC_LOC): New.
> (expanded_location): New field.
> * line-map.c (location_adhoc_data): New.
> (location_adhoc_data_htab): New.
> (curr_adhoc_loc): New.
> (location_adhoc_data): New.
> (allocated_location_adhoc_data): New.
> (location_adhoc_data_hash): New.
> (location_adhoc_data_eq): New.
> (location_adhoc_data_update): New.
> (get_combined_adhoc_loc): New.
> (get_data_from_adhoc_loc): New.
> (get_location_from_adhoc_loc): New.
> (location_adhoc_data_init): New.
> (location_adhoc_data_fini): New.
> (linemap_lookup): Change to use new location.
> (linemap_ordinary_map_lookup): Likewise.
> (linemap_macro_map_lookup): Likewise.
> (linemap_macro_map_loc_to_def_point): Likewise.
> (linemap_macro_map_loc_unwind_toward_spel): Likewise.
> (linemap_get_expansion_line): Likewise.
> (linemap_get_expansion_filename): Likewise.
> (linemap_location_in_system_header_p): Likewise.
> (linemap_location_from_macro_expansion_p): Likewise.
> (linemap_macro_loc_to_spelling_point): Likewise.
> (linemap_macro_loc_to_def_point): Likewise.
> (linemap_macro_loc_to_exp_point): Likewise.
> (linemap_resolve_location): Likewise.
> (linemap_unwind_toward_expansion): Likewise.
> (linemap_unwind_to_first_non_reserved_loc): Likewise.
> (linemap_expand_location): Likewise.
> (linemap_dump_location): Likewise.