diff mbox

[PR50869] don't attempt to expand CFA within cselib

Message ID orobx025bt.fsf@livre.localdomain
State New
Headers show

Commit Message

Alexandre Oliva Oct. 28, 2011, 9:07 p.m. UTC 
An assertion check meant to verify that var loc expansions that didn't
involve VALUEs (say constants, REGs, etc) didn't push values onto the
dependency stack failed in an expansion of the argp reg, because
equivalences for it are preserved at cselib table resets, and cselib
later tries to expand it to equivalent expressions.

It's not profitable to expand it within var-tracking, and that's the
only user of the CFA-base special-casing in cselib, so I arranged for
argp to be preserved in expansions, just like other stack base
registers.

While debugging it, I noticed it was theoretically possible for the
expression depth to remain uninitialized, and added an initialization
and an assertion check to make sure it only remains zero when no
location is found.

Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  Ok to install?

Comments

Jakub Jelinek Oct. 31, 2011, 10:11 a.m. UTC | #1 
On Fri, Oct 28, 2011 at 07:07:18PM -0200, Alexandre Oliva wrote:
> for  gcc/ChangeLog
> from  Alexandre Oliva  <aoliva@redhat.com>
> 
> 	PR debug/50869
> 	* cselib.c (cfa_base_preserved_regno): Initialize.
> 	(cselib_expand_value_rtx_1): Don't expand it.
> 	* var-tracking.c (vt_expand_var_loc_chain): Initialize depth.
> 	Check it's only zero if result is NULL.

Ok for trunk, thanks.

	Jakub
Jeff Law Oct. 31, 2011, 6:09 p.m. UTC | #2 
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/28/11 15:07, Alexandre Oliva wrote:
> An assertion check meant to verify that var loc expansions that
> didn't involve VALUEs (say constants, REGs, etc) didn't push values
> onto the dependency stack failed in an expansion of the argp reg,
> because equivalences for it are preserved at cselib table resets,
> and cselib later tries to expand it to equivalent expressions.
> 
> It's not profitable to expand it within var-tracking, and that's
> the only user of the CFA-base special-casing in cselib, so I
> arranged for argp to be preserved in expansions, just like other
> stack base registers.
> 
> While debugging it, I noticed it was theoretically possible for
> the expression depth to remain uninitialized, and added an
> initialization and an assertion check to make sure it only remains
> zero when no location is found.
> 
> Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  Ok to
> install?
OK.
jeff

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJOruRVAAoJEBRtltQi2kC7JlIH/3zxv5lhZ2VaGGVFjntIZO2T
AlANqcP3UZRsbBcIQ4J/3MA19ob4QTw5gQq7nFxX1OGUlRag9mFzE00L3Q2uLCSn
z7OVZGNwL48eN5G36HH9UY5ktQmy14UPQfE1d4P+X3h/bhLAMHfaQuMIl2+/QK60
nhaGYQMx0qlv2Ndof+HNwo/6s/o4oX3bWS5EavPFyPCHuy7dGrlcY10C2gZnund8
JYA4byxtFKNybiji5WNFO2XxzjVCxGe0+XWAPqO2jNj3CBEfzMyUbZhVP3llOJBI
9Mcjn3k/kTp/3h9aGzoGPssYR9DpMxyU+IQlSPyhR9ZiNGCC7Udj/aALtI6/b/U=
=689Y
-----END PGP SIGNATURE-----
diff mbox

Patch

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/50869
	* cselib.c (cfa_base_preserved_regno): Initialize.
	(cselib_expand_value_rtx_1): Don't expand it.
	* var-tracking.c (vt_expand_var_loc_chain): Initialize depth.
	Check it's only zero if result is NULL.

Index: gcc/cselib.c
===================================================================
--- gcc/cselib.c.orig	2011-10-27 18:32:20.137366314 -0200
+++ gcc/cselib.c	2011-10-27 18:27:05.387597000 -0200
@@ -185,7 +185,7 @@  static cselib_val dummy_val;
    that is constant through the whole function and should never be
    eliminated.  */
 static cselib_val *cfa_base_preserved_val;
-static unsigned int cfa_base_preserved_regno;
+static unsigned int cfa_base_preserved_regno = INVALID_REGNUM;
 
 /* Used to list all values that contain memory reference.
    May or may not contain the useless values - the list is compacted
@@ -1451,7 +1451,7 @@  cselib_expand_value_rtx_1 (rtx orig, str
 	  if (GET_MODE (l->elt->val_rtx) == GET_MODE (orig))
 	    {
 	      rtx result;
-	      int regno = REGNO (orig);
+	      unsigned regno = REGNO (orig);
 
 	      /* The only thing that we are not willing to do (this
 		 is requirement of dse and if others potential uses
@@ -1471,7 +1471,8 @@  cselib_expand_value_rtx_1 (rtx orig, str
 		 make the frame assumptions.  */
 	      if (regno == STACK_POINTER_REGNUM
 		  || regno == FRAME_POINTER_REGNUM
-		  || regno == HARD_FRAME_POINTER_REGNUM)
+		  || regno == HARD_FRAME_POINTER_REGNUM
+		  || regno == cfa_base_preserved_regno)
 		return orig;
 
 	      bitmap_set_bit (evd->regs_active, regno);
Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c.orig	2011-10-27 18:32:20.141366261 -0200
+++ gcc/var-tracking.c	2011-10-27 18:28:03.823813000 -0200
@@ -7764,7 +7764,7 @@  vt_expand_var_loc_chain (variable var, b
   bool pending_recursion;
   rtx loc_from = NULL;
   struct elt_loc_list *cloc = NULL;
-  int depth, saved_depth = elcd->depth;
+  int depth = 0, saved_depth = elcd->depth;
 
   /* Clear all backlinks pointing at this, so that we're not notified
      while we're active.  */
@@ -7842,6 +7842,8 @@  vt_expand_var_loc_chain (variable var, b
   VAR_LOC_FROM (var) = loc_from;
   VAR_LOC_DEPTH (var) = depth;
 
+  gcc_checking_assert (!depth == !result);
+
   elcd->depth = update_depth (saved_depth, depth);
 
   /* Indicate whether any of the dependencies are pending recursion