This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
Hi! First of all, I'm worried about generated debug info quality. obj81{4,5,6,7}/gcc/cc1plus below are x86_64/i686 unpatched trunk stage3 compilers (with var-tracking.c patched afterwards and make cc1plus in the stage3 gcc directory, so it is the same code, just built with different compiler) and x86_64/i686 patched trunk stage3, all of them --enable-checking=yes,rtl. What worries me is that we suddenly emit far less than half of the previous call_site_value parameters: for i in 81{4,5,6,7}; do readelf -wio obj$i/gcc/cc1plus | grep 'DW_AT_GNU_call_site_value' | wc -l; done 945514 901463 398378 211542 Those need to be emitted uncached, while it seems you are now caching them too. Another thing, is it really necessary to add the cselib locations to the hash tables, and if yes, can't it be postponed to start of vt_emit* phase as opposed to making all the hash table data larger already during the second phase which is for many testcases very memory hungry? And, do you do anything special for ENTRY_VALUEs? The current code is trying hard to emit something non-ENTRY_VALUE if at all possible, those really should be the last options if nothing else can be used. Looking at the counts for i in 81{4,5,6,7}; do readelf -wio obj$i/gcc/cc1plus | grep 'DW_OP_GNU_entry_value' | wc -l; done 121713 36158 116585 37687 suggests that the entry_value count increased for i686 and somewhat decreased for x86_64 (but wonder if the decreasing isn't because of less than half of DW_AT_GNU_call_site_value which often use entry_value). Attached below are for i in 81{4,5,6,7}; do locstats --tabulate=0.0:5,99:1 obj$i/gcc/cc1plus 2>/dev/null > $i; done numbers, from quick look the number of vars with 0.0% coverage went slightly down, but the cumulative numbers for all other percentages are worse with the patch (i.e. fewer variables with 100% coverage, etc.), though not by a huge margin. And just a few comments so far: On Mon, Sep 19, 2011 at 06:17:08PM -0300, Alexandre Oliva wrote: > +/* A vector of loc_exp_dep holds the active dependencies of a one-part > + DV on VALUEs, i.e., the VALUEs expanded so as to form the current > + location of DV. Each entry is also part of VALUE' s linked-list of > + backlinks back to DV. */ > +typedef struct loc_exp_dep_s > +{ > + /* The dependent DV. */ > + decl_or_value dv; > + /* The dependency VALUE or DECL_DEBUG. */ DEBUG_EXPR. > +/* A narrower type used to hold a onepart_enum while saving > + memory. */ > +typedef char onepart_enum_t; > + > /* Structure describing where the variable is located. */ > typedef struct variable_def > { > @@ -330,10 +397,8 @@ typedef struct variable_def > /* Number of variable parts. */ > char n_var_parts; > > - /* True if this variable changed (any of its) cur_loc fields > - during the current emit_notes_for_changes resp. > - emit_notes_for_differences call. */ > - bool cur_loc_changed; > + /* What type of DV this is, according to enum onepart_enum. */ > + onepart_enum_t onepart; Can't you use ENUM_BITFIELD (onepart_enum) onepart : 8; instead? > @@ -420,7 +515,6 @@ static void stack_adjust_offset_pre_post > static void insn_stack_adjust_offset_pre_post (rtx, HOST_WIDE_INT *, > HOST_WIDE_INT *); > static bool vt_stack_adjustments (void); > -static void note_register_arguments (rtx); > static hashval_t variable_htab_hash (const void *); > static int variable_htab_eq (const void *, const void *); > static void variable_htab_free (void *); > @@ -672,15 +766,11 @@ vt_stack_adjustments (void) > for (insn = BB_HEAD (dest); > insn != NEXT_INSN (BB_END (dest)); > insn = NEXT_INSN (insn)) > - { > - if (INSN_P (insn)) > - { > - insn_stack_adjust_offset_pre_post (insn, &pre, &post); > - offset += pre + post; > - } > - if (CALL_P (insn)) > - note_register_arguments (insn); > - } > + if (INSN_P (insn)) > + { > + insn_stack_adjust_offset_pre_post (insn, &pre, &post); > + offset += pre + post; > + } > > VTI (dest)->out.stack_adjust = offset; > > @@ -5020,9 +5026,6 @@ log_op_type (rtx x, basic_block bb, rtx > /* All preserved VALUEs. */ > static VEC (rtx, heap) *preserved_values; > > -/* Registers used in the current function for passing parameters. */ > -static HARD_REG_SET argument_reg_set; > - > /* Ensure VAL is preserved and remember it in a vector for vt_emit_notes. */ > > static void > @@ -5382,8 +5396,6 @@ add_stores (rtx loc, const_rtx expr, voi > mo.u.loc = loc; > if (GET_CODE (expr) == SET > && SET_DEST (expr) == loc > - && REGNO (loc) < FIRST_PSEUDO_REGISTER > - && TEST_HARD_REG_BIT (argument_reg_set, REGNO (loc)) > && find_use_val (loc, mode, cui) > && GET_CODE (SET_SRC (expr)) != ASM_OPERANDS) > { Why the above? Isn't it too expensive to enter it for all the registers, even when they aren't going to be used for argument passing nor needed explicitly for anything debug info related? > @@ -5893,7 +5905,7 @@ prepare_call_arguments (basic_block bb, > tree dtemp = VEC_index (tree, *debug_args, ix + 1); > enum machine_mode mode = DECL_MODE (dtemp); > item = gen_rtx_DEBUG_PARAMETER_REF (mode, param); > - item = gen_rtx_CONCAT (mode, item, DECL_RTL (dtemp)); > + item = gen_rtx_CONCAT (mode, item, DECL_RTL_IF_SET (dtemp)); Generating CONCAT with NULL second argument? Ugh. Plus, when will that happen? Jakub
Attachment:
814
Description: Text document
Attachment:
815
Description: Text document
Attachment:
816
Description: Text document
Attachment:
817
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |