| Message ID | 20121029204328.GM1752@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Oct 29, 2012 at 9:43 PM, Jakub Jelinek wrote: > The following patch should fix the var-tracking slowness on the > reflect/check testcases where one of the test routines was compiling for > almost 50 minutes. Thanks for working on this. Will you also add the test case? Ciao! Steven
On Mon, Oct 29, 2012 at 09:46:24PM +0100, Steven Bosscher wrote:
> Will you also add the test case?
There already is, libgo make reflect/check. I don't see what other kind of
testcase should I provide for that, unless you mean scanning -dA assembly
for DW_OP_fbreg (which would be inherently target specific).
BTW, only the go testcase has been worked on, I haven't been able to
reproduce any significant slowness on the #c0 testcase (perhaps I've been
trying wrong target or options), and the clobber_overlapping_mems
routine, while certainly desirable for correct debug info (without it
debug info might say something is available in a mem where it isn't
available), is still quite expensive and it would be nice to find some
ways to speed it up.
Jakub
On Mon, Oct 29, 2012 at 1:43 PM, Jakub Jelinek <jakub@redhat.com> wrote: > > The following patch should fix the var-tracking slowness on the > reflect/check testcases where one of the test routines was compiling for > almost 50 minutes. The problem was that there is a huge routine that is > shrink wrapped (or perhaps just the split stack prologue is multi-bb), > and I was trying to be too conservative when adding frame_pointer_needed > hard_frame_pointer_rtx VTA replacements. Without those replacements we > weren't using DW_OP_fbreg for the hard frame pointer based mem accesses (so > unnecessarily large debug info), but what's worse is that on the hfp VALUE > we got almost 1200 reverse ops in the locs chain, so find_base_term as well > as get_addr has been quite costly. Thanks for working on this. The patch looks fine to me but I don't know this code well. Ian
On Oct 29, 2012, Jakub Jelinek <jakub@redhat.com> wrote: > PR debug/54402 > * var-tracking.c (fp_setter): Return false if there is REG_CFA_RESTORE > hfp note. > (vt_initialize): Look for fp_setter in any bb, not just successor of > entry bb. This looks reasonable, except that... > --- gcc/var-tracking.c.jj 2012-10-29 12:16:23.000000000 +0100 > +++ gcc/var-tracking.c 2012-10-29 17:58:06.629025339 +0100 > @@ -9535,24 +9535,33 @@ vt_add_function_parameters (void) > static bool > fp_setter (rtx insn) > { > - rtx pat = PATTERN (insn); > + rtx pat = PATTERN (insn), expr; > if (RTX_FRAME_RELATED_P (insn)) > { > - rtx expr = find_reg_note (insn, REG_FRAME_RELATED_EXPR, NULL_RTX); > + expr = find_reg_note (insn, REG_FRAME_RELATED_EXPR, NULL_RTX); > if (expr) > pat = XEXP (expr, 0); ... I don't get why you moved expr out of the only block in which it's used. Ok with or without this change. Thanks,
--- gcc/var-tracking.c.jj 2012-10-29 12:16:23.000000000 +0100 +++ gcc/var-tracking.c 2012-10-29 17:58:06.629025339 +0100 @@ -9535,24 +9535,33 @@ vt_add_function_parameters (void) static bool fp_setter (rtx insn) { - rtx pat = PATTERN (insn); + rtx pat = PATTERN (insn), expr; if (RTX_FRAME_RELATED_P (insn)) { - rtx expr = find_reg_note (insn, REG_FRAME_RELATED_EXPR, NULL_RTX); + expr = find_reg_note (insn, REG_FRAME_RELATED_EXPR, NULL_RTX); if (expr) pat = XEXP (expr, 0); } if (GET_CODE (pat) == SET) - return SET_DEST (pat) == hard_frame_pointer_rtx; + { + if (SET_DEST (pat) != hard_frame_pointer_rtx) + return false; + } else if (GET_CODE (pat) == PARALLEL) { int i; for (i = XVECLEN (pat, 0) - 1; i >= 0; i--) if (GET_CODE (XVECEXP (pat, 0, i)) == SET && SET_DEST (XVECEXP (pat, 0, i)) == hard_frame_pointer_rtx) - return true; + break; + if (i < 0) + return false; } - return false; + else + return false; + if (find_reg_note (insn, REG_CFA_RESTORE, hard_frame_pointer_rtx)) + return false; + return true; } /* Initialize cfa_base_rtx, create a preserved VALUE for it and @@ -9600,7 +9609,7 @@ vt_init_cfa_base (void) static bool vt_initialize (void) { - basic_block bb, prologue_bb = single_succ (ENTRY_BLOCK_PTR); + basic_block bb; HOST_WIDE_INT fp_cfa_offset = -1; alloc_aux_for_blocks (sizeof (struct variable_tracking_info_def)); @@ -9858,8 +9867,7 @@ vt_initialize (void) VTI (bb)->out.stack_adjust += post; } - if (bb == prologue_bb - && fp_cfa_offset != -1 + if (fp_cfa_offset != -1 && hard_frame_pointer_adjustment == -1 && RTX_FRAME_RELATED_P (insn) && fp_setter (insn))