Patchwork Improve var-tracking memory disambiguation with frame pointer (PR debug/54796)

login
register
mail settings
Submitter Jakub Jelinek
Date Oct. 4, 2012, 4:33 p.m.
Message ID <20121004163346.GC1787@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/189204/
State New
Headers show

Comments

Jakub Jelinek - Oct. 4, 2012, 4:33 p.m.
Hi!

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.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

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.


	Jakub
Steven Bosscher - Oct. 4, 2012, 4:42 p.m.
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
Jakub Jelinek - Oct. 4, 2012, 5:08 p.m.
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
Alexandre Oliva - Oct. 11, 2012, 5:26 p.m.
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.

Patch

--- 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;
+}