On Mon, Sep 10, 2012 at 3:01 AM, Richard Guenther <richard.guent...@gmail.com> wrote: > On Sun, Sep 9, 2012 at 12:26 AM, Dehao Chen <de...@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?
This will break LTO build of gcc.c-torture/execute/pr38051.c There is data structure like: union { long int l; char c[sizeof (long int)]; } u; Once the block info is reserved for this, it'll reserve this data structure. And inside this data structure, there is VAR_DECL. Thus LTO_NO_PREVAIL assertion does not satisfy here for TREE_CHAIN (t). > >> 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. Yeah, makes sense. New change: @@ -5980,9 +5974,10 @@ tree t = *tp; if (EXPR_P (t)) - /* We should never have TREE_BLOCK set on non-statements. */ - gcc_assert (!TREE_BLOCK (t)); - + { + if (TREE_BLOCK (t)) + TREE_SET_BLOCK (t, p->new_block); + } else if (DECL_P (t) || TREE_CODE (t) == SSA_NAME) { if (TREE_CODE (t) == SSA_NAME) Thanks, Dehao > > 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 <de...@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 <de...@google.com> >> >> * lto/lto.c (lto_fixup_prevailing_decls): Remove tree.exp.block >> field. >> >> libcpp/ChangeLog: >> 2012-09-08 Dehao Chen <de...@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.