diff mbox

Fix thinko in var-tracking.c

Message ID 201105091331.46507.ebotcazou@adacore.com
State New
Headers show

Commit Message

Eric Botcazou May 9, 2011, 11:31 a.m. UTC 
There is a known pitfall with REG_OFFSET/MEM_OFFSET: the former is an integer 
whereas the latter is a RTX.  So a test like MEM_OFFSET (node->loc) == 0 is 
essentially never true for user MEMs since you have const0_rtx instead.

The attached patch fixes 3 occurrences introduced in the VTA merge; since this 
doesn't affect code generation, I think it's appropriate for all branches.  It 
seems to introduce a couple of XPASSes in the guality testsuite on i586:

XPASS: gcc.dg/guality/inline-params.c  -O2 -flto  execution test
XPASS: gcc.dg/guality/pr41447-1.c  -O2 -flto  execution test

Tested on i586-suse-linux and x86_64-suse-linux, applied on mainline, 4.6 and 
4.5 branches.


2011-05-09  Eric Botcazou  <ebotcazou@adacore.com>

	* var-tracking.c (find_mem_expr_in_1pdv): Fix thinko.
	(dataflow_set_preserve_mem_locs): Likewise.

Comments

Joseph Myers May 9, 2011, 1:05 p.m. UTC | #1 
On Mon, 9 May 2011, Eric Botcazou wrote:

> doesn't affect code generation, I think it's appropriate for all branches.  It 
> seems to introduce a couple of XPASSes in the guality testsuite on i586:
> 
> XPASS: gcc.dg/guality/inline-params.c  -O2 -flto  execution test
> XPASS: gcc.dg/guality/pr41447-1.c  -O2 -flto  execution test

The guality tests have random results.  I don't know if that's the issue 
here, but perhaps someone can review Jeff's patch 
<http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01060.html> to make them 
more predictable.  Without that, I generally just ignore them in 
comparisons of test results.
Mike Stump May 9, 2011, 8:28 p.m. UTC | #2 
On May 9, 2011, at 6:05 AM, Joseph S. Myers wrote:
> The guality tests have random results.  I don't know if that's the issue 
> here, but perhaps someone can review Jeff's patch 
> <http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01060.html> to make them 
> more predictable.  Without that, I generally just ignore them in 
> comparisons of test results.

:-(  Thanks for the heads up.  I've approved it.  If people find those testcases useful, please, ensure they are deterministic and test something useful.  If they aren't up to standards as a whole, I'd entertain just adding a return 0 near the top of the .exp file to prompt people that would like them in the tree to address outstanding issues.
diff mbox

Patch

Index: var-tracking.c
===================================================================
--- var-tracking.c	(revision 173545)
+++ var-tracking.c	(working copy)
@@ -4113,8 +4113,9 @@  find_mem_expr_in_1pdv (tree expr, rtx va
   VALUE_RECURSED_INTO (val) = true;
 
   for (node = var->var_part[0].loc_chain; node; node = node->next)
-    if (MEM_P (node->loc) && MEM_EXPR (node->loc) == expr
-	&& MEM_OFFSET (node->loc) == 0)
+    if (MEM_P (node->loc)
+	&& MEM_EXPR (node->loc) == expr
+	&& INT_MEM_OFFSET (node->loc) == 0)
       {
 	where = node;
 	break;
@@ -4177,11 +4178,10 @@  dataflow_set_preserve_mem_locs (void **s
 	{
 	  for (loc = var->var_part[0].loc_chain; loc; loc = loc->next)
 	    {
-	      /* We want to remove dying MEMs that doesn't refer to
-		 DECL.  */
+	      /* We want to remove dying MEMs that doesn't refer to DECL.  */
 	      if (GET_CODE (loc->loc) == MEM
 		  && (MEM_EXPR (loc->loc) != decl
-		      || MEM_OFFSET (loc->loc))
+		      || INT_MEM_OFFSET (loc->loc) != 0)
 		  && !mem_dies_at_call (loc->loc))
 		break;
 	      /* We want to move here MEMs that do refer to DECL.  */
@@ -4225,7 +4225,7 @@  dataflow_set_preserve_mem_locs (void **s
 
 	  if (GET_CODE (loc->loc) != MEM
 	      || (MEM_EXPR (loc->loc) == decl
-		  && MEM_OFFSET (loc->loc) == 0)
+		  && INT_MEM_OFFSET (loc->loc) == 0)
 	      || !mem_dies_at_call (loc->loc))
 	    {
 	      if (old_loc != loc->loc && emit_notes)