| Message ID | orboke1vyr.fsf@livre.localdomain |
|---|---|
| State | New |
| Headers | show |
On Wed, Jun 20, 2012 at 12:52:12AM -0300, Alexandre Oliva wrote: > On Jun 16, 2012, "H.J. Lu" <hjl.tools@gmail.com> wrote: > from Alexandre Oliva <aoliva@redhat.com> > > PR debug/53671 > PR debug/49888 > * alias.c (memrefs_conflict_p): Improve handling of AND for > alignment. > from Alexandre Oliva <aoliva@redhat.com> > > PR debug/53671 > PR debug/49888 > * var-tracking.c (vt_initialize): Record initial offset between > arg pointer and stack pointer. > > from Alexandre Oliva <aoliva@redhat.com> > > PR debug/53671 > PR debug/49888 > * var-tracking.c (vt_get_canonicalize_base): New. > (vt_canonicalize_addr, vt_stack_offset_p): New. > (vt_canon_true_dep): New. > (drop_overlapping_mem_locs): Use vt_canon_true_dep. > (clobber_overlaping_mems): Use vt_canonicalize_addr. > > from Alexandre Oliva <aoliva@redhat.com> > > PR debug/53671 > PR debug/49888 > * var-tracking.c (vt_init_cfa_base): Drop redundant recording of > CFA base. Ok, thanks. Jakub
On Wed, Jun 20, 2012 at 5:52 AM, Alexandre Oliva <aoliva@redhat.com> wrote: > On Jun 16, 2012, "H.J. Lu" <hjl.tools@gmail.com> wrote: > >> If I understand it correctly, the new approach fails to handle push >> properly. > > It's actually cselib that didn't deal with push properly, so it thinks > incoming stack arguments may be clobbered by them. But that's not the > whole story, unfortunately. I still don't have a complete fix for the > problem, but I have some patches that restore nearly all of the passes. > > The first one extends RTX alias analysis so that cselib can recognize > that (mem:SI ARGP) and (mem:SI (plus (and (plus ARGP #-4) #-32) #-4)) > don't alias. Before the patch, we'd go for infinite sized objects upon > AND. Just looking at this patch. It looks reasonable, but I have a question on the pre-existing condition - if (GET_CODE (y) == AND || ysize < -INTVAL (XEXP (x, 1))) xsize = -1; so if this condition is not true then we simply strip off the AND of X and do not adjust xsize at all? Likewise we do not adjust c? How can that be conservatively correct? Thus, I'd rather see if (GET_CODE (x) == AND && CONST_INT_P (XEXP (x, 1))) { + HOST_WIDE_INT sc = INTVAL (XEXP (x, 1)); + unsigned HOST_WIDE_INT uc = sc; + if (xsize > 0 && sc < 0 && -uc == (uc & -uc)) + { + xsize -= sc + 1; + c -= sc; return memrefs_conflict_p (xsize, canon_rtx (XEXP (x, 0)), ysize, y, c); } } as the sole supported case. The patch is ok with the above change if it passes bootstrap & regtest. I suppose part of the comment that maybe tries to explain the existing code (but does not match what it does IMHO), Assume that references besides AND are aligned, so if the size of the other reference is at least as large as the alignment, assume no other overlap. */ should be removed then. Thanks, Richard. > > The second introduces an entry-point equivalence between ARGP and SP, so > that SP references in push and stack-align sequences can be > canonicalized to ARGP-based. > > The third introduces address canonicalization that uses information in > the dataflow variable set in addition to the static cselib table. This > is the one I'm still working on, because some expressions still fail to > canonicalize to ARGP although they could. > > The fourth removes a now-redundant equivalence from the dynamic table; > the required information is always preserved in the static table. > > I've regstrapped (and checked results! :-) all of these on > x86_64-linux-gnu and i686-linux-gnu. It fixes all visible regressions > in x86_64-linux-gnu, and nearly all on i686-linux-gnu. > > May I check these in and keep on working to complete the fix, or should > I revert the original patch and come back only with a patchset that > fixes all debug info regressions? > > > > -- > Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/ > You must be the change you wish to see in the world. -- Gandhi > Be Free! -- http://FSFLA.org/ FSF Latin America board member > Free Software Evangelist Red Hat Brazil Compiler Engineer >
for gcc/ChangeLog from Alexandre Oliva <aoliva@redhat.com> PR debug/53671 PR debug/49888 * var-tracking.c (vt_init_cfa_base): Drop redundant recording of CFA base. Index: gcc/var-tracking.c =================================================================== --- gcc/var-tracking.c.orig 2012-06-18 08:44:13.459569497 -0300 +++ gcc/var-tracking.c 2012-06-18 08:55:31.023984364 -0300 @@ -9582,9 +9582,6 @@ vt_init_cfa_base (void) VOIDmode, get_insns ()); preserve_value (val); cselib_preserve_cfa_base_value (val, REGNO (cfa_base_rtx)); - var_reg_decl_set (&VTI (ENTRY_BLOCK_PTR)->out, cfa_base_rtx, - VAR_INIT_STATUS_INITIALIZED, dv_from_value (val->val_rtx), - 0, NULL_RTX, INSERT); } /* Allocate and initialize the data structures for variable tracking