| Message ID | 20121004163346.GC1787@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Oct 4, 2012 at 6:33 PM, Jakub Jelinek wrote: > This patch fixes a few FAILs in the ix86 guality testsuite (mainly -Os), > by better disambiguating sp based VALUEs (which usually have no MEM_EXPR > and thus the alias Oracle can't be used for them) from frame pointer > based ones or global vars. Hi, Does this also help for some of the var-tracking compile-time-hog bugs? ISTR PR53958 was one with very long chains of sp based values... Ciao! Steven
On Thu, Oct 04, 2012 at 06:42:47PM +0200, Steven Bosscher wrote: > On Thu, Oct 4, 2012 at 6:33 PM, Jakub Jelinek wrote: > > This patch fixes a few FAILs in the ix86 guality testsuite (mainly -Os), > > by better disambiguating sp based VALUEs (which usually have no MEM_EXPR > > and thus the alias Oracle can't be used for them) from frame pointer > > based ones or global vars. > > Does this also help for some of the var-tracking compile-time-hog > bugs? ISTR PR53958 was one with very long chains of sp based values... Very unlikely. That PR has been reported against 4.7, which doesn't have clobber_overlapping_mems at all, the patch could only affect compile times if there were huge chains of sp based values and find_base_term appeared anywhere significantly in the profiles. That PR seems to create so huge VTA hash tables that var-tracking gives up, unfortunately the current give up heuristics doesn't estimate the compile time and compile memory needed accurately enough and lowering the limit (which is a param)'s default would on the other side result in sane sized programs to trigger no-VTA way too often. Jakub
On Oct 4, 2012, Jakub Jelinek <jakub@redhat.com> wrote: > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? This is ok, except for one nit. > 2012-10-04 Jakub Jelinek <jakub@redhat.com> > PR debug/54796 > * rtl.h: Document jump flag on VALUE. > * cselib.h (cselib_set_value_sp_based, > cselib_sp_based_value_p): New prototypes. > * alias.c (find_base_term): For cselib_sp_based_value_p > return static_reg_base_value[STACK_POINTER_REGNUM]. > * cselib.c (SP_BASED_VALUE_P): Define. > (cselib_set_value_sp_based, cselib_sp_based_value_p): New functions. > * var-tracking.c (add_stores): Call cselib_set_value_sp_based > for not yet preserved VALUEs of sp on sp assignments if > hard_frame_pointer_adjustment != -1. > (vt_initialize): When setting hard_frame_pointer_adjustment, > disassociate sp from its previous value and call > cselib_set_value_sp_based on a new VALUE created for sp. > * gcc.dg/guality/pr54796.c: New test. > +/* Test whether a value is preserved. */ > + > +bool > +cselib_sp_based_value_p (cselib_val *v) The function comment needs rewriting after the cut&paste.
--- gcc/rtl.h.jj 2012-09-28 14:16:57.000000000 +0200 +++ gcc/rtl.h 2012-09-28 14:16:57.000000000 +0200 @@ -267,7 +267,8 @@ struct GTY((chain_next ("RTX_NEXT (&%h)" 1 in a CALL_INSN if it is a sibling call. 1 in a SET that is for a return. In a CODE_LABEL, part of the two-bit alternate entry field. - 1 in a CONCAT is VAL_EXPR_IS_COPIED in var-tracking.c. */ + 1 in a CONCAT is VAL_EXPR_IS_COPIED in var-tracking.c. + 1 in a VALUE is SP_BASED_VALUE_P in cselib.c. */ unsigned int jump : 1; /* In a CODE_LABEL, part of the two-bit alternate entry field. 1 in a MEM if it cannot trap. --- gcc/cselib.h.jj 2012-03-06 17:02:12.000000000 +0100 +++ gcc/cselib.h 2012-10-04 08:42:04.879335722 +0200 @@ -99,6 +99,8 @@ extern void cselib_preserve_only_values extern void cselib_preserve_cfa_base_value (cselib_val *, unsigned int); extern void cselib_add_permanent_equiv (cselib_val *, rtx, rtx); extern bool cselib_have_permanent_equivalences (void); +extern void cselib_set_value_sp_based (cselib_val *); +extern bool cselib_sp_based_value_p (cselib_val *); extern void dump_cselib_table (FILE *); --- gcc/alias.c.jj 2012-09-12 10:57:03.000000000 +0200 +++ gcc/alias.c 2012-10-04 08:45:22.893221350 +0200 @@ -1641,6 +1641,9 @@ find_base_term (rtx x) if (!val) return ret; + if (cselib_sp_based_value_p (val)) + return static_reg_base_value[STACK_POINTER_REGNUM]; + f = val->locs; /* Temporarily reset val->locs to avoid infinite recursion. */ val->locs = NULL; --- gcc/cselib.c.jj 2012-08-24 23:44:03.000000000 +0200 +++ gcc/cselib.c 2012-10-04 08:45:12.722278664 +0200 @@ -210,6 +210,9 @@ void (*cselib_record_sets_hook) (rtx ins #define PRESERVED_VALUE_P(RTX) \ (RTL_FLAG_CHECK1("PRESERVED_VALUE_P", (RTX), VALUE)->unchanging) +#define SP_BASED_VALUE_P(RTX) \ + (RTL_FLAG_CHECK1("SP_BASED_VALUE_P", (RTX), VALUE)->jump) + /* Allocate a struct elt_list and fill in its two elements with the @@ -739,6 +742,23 @@ cselib_preserve_only_values (void) gcc_assert (first_containing_mem == &dummy_val); } +/* Arrange for a value to be marked as based on stack pointer + for find_base_term purposes. */ + +void +cselib_set_value_sp_based (cselib_val *v) +{ + SP_BASED_VALUE_P (v->val_rtx) = 1; +} + +/* Test whether a value is preserved. */ + +bool +cselib_sp_based_value_p (cselib_val *v) +{ + return SP_BASED_VALUE_P (v->val_rtx); +} + /* Return the mode in which a register was last set. If X is not a register, return its mode. If the mode in which the register was set is not known, or the value was already clobbered, return --- gcc/var-tracking.c.jj 2012-10-03 09:01:36.297902370 +0200 +++ gcc/var-tracking.c 2012-10-04 15:35:57.276697832 +0200 @@ -5769,6 +5769,11 @@ add_stores (rtx loc, const_rtx expr, voi resolve = preserve = !cselib_preserved_value_p (v); + if (loc == stack_pointer_rtx + && hard_frame_pointer_adjustment != -1 + && preserve) + cselib_set_value_sp_based (v); + nloc = replace_expr_with_values (oloc); if (nloc) oloc = nloc; @@ -9866,6 +9871,19 @@ vt_initialize (void) { vt_init_cfa_base (); hard_frame_pointer_adjustment = fp_cfa_offset; + /* Disassociate sp from fp now. */ + if (MAY_HAVE_DEBUG_INSNS) + { + cselib_val *v; + cselib_invalidate_rtx (stack_pointer_rtx); + v = cselib_lookup (stack_pointer_rtx, Pmode, 1, + VOIDmode); + if (v && !cselib_preserved_value_p (v)) + { + cselib_set_value_sp_based (v); + preserve_value (v); + } + } } } } --- gcc/testsuite/gcc.dg/guality/pr54796.c.jj 2012-10-04 15:31:03.314328898 +0200 +++ gcc/testsuite/gcc.dg/guality/pr54796.c 2012-10-04 15:30:38.000000000 +0200 @@ -0,0 +1,25 @@ +/* PR debug/54796 */ +/* { dg-do run } */ +/* { dg-options "-g" } */ + +__attribute__((noinline, noclone)) void +bar (char *a, int b) +{ + __asm volatile ("" : "+r" (a), "+r" (b) : : "memory"); +} + +__attribute__((noinline, noclone)) void +foo (int a, int b) +{ + int c = a; + char d[b]; /* { dg-final { gdb-test 17 "a" "5" } } */ + bar (d, 2); /* { dg-final { gdb-test 17 "b" "6" } } */ + bar (d, 4); /* { dg-final { gdb-test 17 "c" "5" } } */ +} + +int +main () +{ + foo (5, 6); + return 0; +}