Fix alias.c wrt aliases and anchors

Fix alias.c wrt aliases and anchors

[Jan Hubicka]

Hi,
the alias-2.c testcase fails on targets with anchors.  The reason is that
the variable itself is anchored while the alias is not and they point to the
same location.   I folllowed the docs of SYMBOL_REF claiming that
SYMBOL_REF_DECL if the symbol is label and tought it is safe to disambiguate
them.

This patch commonizes the logic to compare_base_symbol_refs which acts
equivalently to compare_base_decls but on tRTL's SYMBOL_REF and can handle
querries when one parameter is DECL while other is anchor.

I am not fully sure about the case where we know that the variable is
in a block and we know its offset.  With current implementation I do not think
it is safe to use offset orracle because one offset is offset inside a variable,
while other is offset inside of the block.  This can be compensated for.
Also it seems that the alias code should not ignore the base informaiton it has
in memory attributes as it can be more precise.

This patch fixes the alias-2.c testcase and was bootstrapped/regtsted on arm.
OK?

I will look into anchor code - it seems it should create anchor for the alias,
too because it is non-interposable, but for some reason it does not.

Honza

	PR middle-end/68832
	* alias.c (compare_base_symbol_refs): New function.
	(rtx_equal_for_memref_p, base_alias_check, memrefs_conflict_p): Use it.
	
Index: alias.c
===================================================================
--- alias.c	(revision 231722)
+++ alias.c	(working copy)
@@ -158,6 +158,7 @@
 static int write_dependence_p (const_rtx,
 			       const_rtx, machine_mode, rtx,
 			       bool, bool, bool);
+static int compare_base_symbol_refs (const_rtx, const_rtx);
 
 static void memory_modified_1 (rtx, const_rtx, void *);
 
@@ -1756,16 +1757,8 @@
       return LABEL_REF_LABEL (x) == LABEL_REF_LABEL (y);
 
     case SYMBOL_REF:
-      {
-	tree x_decl = SYMBOL_REF_DECL (x);
-	tree y_decl = SYMBOL_REF_DECL (y);
+      return compare_base_symbol_refs (x, y) == 1;
 
-	if (!x_decl || !y_decl)
-	  return XSTR (x, 0) == XSTR (y, 0);
-	else
-	  return compare_base_decls (x_decl, y_decl) == 1;
-      }
-
     case ENTRY_VALUE:
       /* This is magic, don't go through canonicalization et al.  */
       return rtx_equal_p (ENTRY_VALUE_EXP (x), ENTRY_VALUE_EXP (y));
@@ -2052,6 +2045,65 @@
   return ret;
 }
 
+/* Same as compare_base_decls but for SYMBOL_REF.
+   Return -2 if the offset based oracle can not be used (i.e.
+   we have a symbol and section anchor which is located insite
+   the same block.  */
+
+static int
+compare_base_symbol_refs (const_rtx x_base, const_rtx y_base)
+{
+  tree x_decl = SYMBOL_REF_DECL (x_base);
+  tree y_decl = SYMBOL_REF_DECL (y_base);
+  bool binds_def = true;
+  if (x_decl && y_decl)
+    return compare_base_decls (x_decl, y_decl);
+  if (x_decl || y_decl)
+    {
+      if (!x_decl)
+	{
+	  std::swap (x_decl, y_decl);
+	  std::swap (x_base, y_base);
+	}
+      /* Variable and anchor representing the variable alias.  If x_base
+	 is not a static variable or y_base is not an anchor (it is a label)
+	 we are safe.  */
+      if (!SYMBOL_REF_HAS_BLOCK_INFO_P (y_base)
+	  || (TREE_CODE (x_decl) != VAR_DECL
+	      || (!TREE_STATIC (x_decl) && !TREE_PUBLIC (x_decl))))
+	return 0;
+      symtab_node *x_node = symtab_node::get_create (x_decl)
+			    ->ultimate_alias_target ();
+      /* External variable can not be in section anchor.  */
+      if (!x_node->definition)
+	return 0;
+      x_base = XEXP (DECL_RTL (x_node->decl), 0);
+      /* If not in anchor, we can disambiguate.  */
+      if (!SYMBOL_REF_HAS_BLOCK_INFO_P (x_base))
+	return 0;
+
+      /* We have an alias of anchored variable.  If it can be interposed;
+ 	 we must assume it may or may not alias its anchor.  */
+      binds_def = decl_binds_to_current_def_p (x_decl);
+    }
+  /* If we have variable in section anchor, we can compare by offset.  */
+  if (SYMBOL_REF_HAS_BLOCK_INFO_P (x_base)
+      && SYMBOL_REF_HAS_BLOCK_INFO_P (y_base))
+    {
+      if (SYMBOL_REF_BLOCK (x_base) != SYMBOL_REF_BLOCK (y_base))
+	return 0;
+      if (SYMBOL_REF_BLOCK_OFFSET (x_base) == SYMBOL_REF_BLOCK_OFFSET (y_base))
+	return binds_def ? 1 : -1;
+      /* Mixing anchors and non-anchors may result to false negative.
+	 We probably never do that.  */
+      if (SYMBOL_REF_ANCHOR_P (x_base) != SYMBOL_REF_ANCHOR_P (y_base))
+	return -2;
+      return 0;
+    }
+  /* Label and label or label and section anchor. Copare symbol name.  */
+  return XSTR (x_base, 0) == XSTR (y_base, 0);
+}
+
 /* Return 0 if the addresses X and Y are known to point to different
    objects, 1 if they might be pointers to the same object.  */
 
@@ -2090,16 +2142,8 @@
     return 1;
 
   if (GET_CODE (x_base) == SYMBOL_REF && GET_CODE (y_base) == SYMBOL_REF)
-    {
-      tree x_decl = SYMBOL_REF_DECL (x_base);
-      tree y_decl = SYMBOL_REF_DECL (y_base);
+    return compare_base_symbol_refs (x_base, y_base) != 0;
 
-      /* We can assume that no stores are made to labels.  */
-      if (!x_decl || !y_decl)
-	return 0;
-      return compare_base_decls (x_decl, y_decl) != 0;
-    }
-
   /* The base addresses are different expressions.  If they are not accessed
      via AND, there is no conflict.  We can bring knowledge of object
      alignment into play here.  For example, on alpha, "char a, b;" can
@@ -2323,26 +2367,14 @@
 
   if (GET_CODE (x) == SYMBOL_REF && GET_CODE (y) == SYMBOL_REF)
     {
-      tree x_decl = SYMBOL_REF_DECL (x);
-      tree y_decl = SYMBOL_REF_DECL (y);
-      int cmp;
+      int cmp = compare_base_symbol_refs (x,y);
 
-      if (!x_decl || !y_decl)
-	{
-	  /* Label and normal symbol are never the same. */
-	  if (x_decl != y_decl)
-	    return 0;
-	  return offset_overlap_p (c, xsize, ysize);
-	}
-      else
-        cmp = compare_base_decls (x_decl, y_decl);
-
       /* If both decls are the same, decide by offsets.  */
       if (cmp == 1)
         return offset_overlap_p (c, xsize, ysize);
       /* If decls are different or we know by offsets that there is no overlap,
 	 we win.  */
-      if (!cmp || !offset_overlap_p (c, xsize, ysize))
+      if (!cmp || (cmp != -2 && !offset_overlap_p (c, xsize, ysize)))
 	return 0;
       /* Decls may or may not be different and offsets overlap....*/
       return -1;

Re: Fix alias.c wrt aliases and anchors

[Richard Sandiford]

> Hi,
> the alias-2.c testcase fails on targets with anchors.  The reason is that
> the variable itself is anchored while the alias is not and they point to the
> same location.   I folllowed the docs of SYMBOL_REF claiming that
> SYMBOL_REF_DECL if the symbol is label and tought it is safe to disambiguate
> them.

What kind of distinction do you mean between "label" and non-label here?

I don't think !SYMBOL_REF_DECL tells us anything useful about the
symbol.  I think it's more a case of: if SYMBOF_REF_DECL is present,
we can assume that what it says is accurate.  If it isn't present we
can't assume anything.

So was it...

Jan Hubicka <hubicka@ucw.cz> writes:
> @@ -2323,26 +2367,14 @@
>  
>    if (GET_CODE (x) == SYMBOL_REF && GET_CODE (y) == SYMBOL_REF)
>      {
> -      tree x_decl = SYMBOL_REF_DECL (x);
> -      tree y_decl = SYMBOL_REF_DECL (y);
> -      int cmp;
> +      int cmp = compare_base_symbol_refs (x,y);
>  
> -      if (!x_decl || !y_decl)
> -	{
> -	  /* Label and normal symbol are never the same. */
> -	  if (x_decl != y_decl)
> -	    return 0;
> -	  return offset_overlap_p (c, xsize, ysize);

...this part of the code that was causing the problem?  That doesn't
seem valid even without the anchor stuff.  I think the starting position
should be that we can only use offset_overlap_p if XSTR (x) == XSTR (y)
and need to assume a conflict otherwise.  E.g. I think in principle it's
valid for a target to create symbol_refs for different parts of a single
artificial object.

I agree there are other refinements you can do on top of that,
e.g. that two block symbols in different blocks can never conflict.
But the patch seems to be treating anchors as an exception to the rule,
whereas I think they're just one instance of the rule.

Thanks,
Richard

Re: Fix alias.c wrt aliases and anchors

[Jan Hubicka]

> > Hi,
> > the alias-2.c testcase fails on targets with anchors.  The reason is that
> > the variable itself is anchored while the alias is not and they point to the
> > same location.   I folllowed the docs of SYMBOL_REF claiming that
> > SYMBOL_REF_DECL if the symbol is label and tought it is safe to disambiguate
> > them.
> 
> What kind of distinction do you mean between "label" and non-label here?
> 
> I don't think !SYMBOL_REF_DECL tells us anything useful about the
> symbol.  I think it's more a case of: if SYMBOF_REF_DECL is present,
> we can assume that what it says is accurate.  If it isn't present we
> can't assume anything.
> 
> So was it...
> 
> Jan Hubicka <hubicka@ucw.cz> writes:
> > @@ -2323,26 +2367,14 @@
> >  
> >    if (GET_CODE (x) == SYMBOL_REF && GET_CODE (y) == SYMBOL_REF)
> >      {
> > -      tree x_decl = SYMBOL_REF_DECL (x);
> > -      tree y_decl = SYMBOL_REF_DECL (y);
> > -      int cmp;
> > +      int cmp = compare_base_symbol_refs (x,y);
> >  
> > -      if (!x_decl || !y_decl)
> > -	{
> > -	  /* Label and normal symbol are never the same. */
> > -	  if (x_decl != y_decl)
> > -	    return 0;
> > -	  return offset_overlap_p (c, xsize, ysize);
> 
> ...this part of the code that was causing the problem?  That doesn't
> seem valid even without the anchor stuff.  I think the starting position

Yep, I misread the docs assuming that SYMBOL_REF_DECL == NULL only
for code labels.  It is always safe to disambiguate these as they access
readonly memory anyway.

> should be that we can only use offset_overlap_p if XSTR (x) == XSTR (y)
> and need to assume a conflict otherwise.  E.g. I think in principle it's
> valid for a target to create symbol_refs for different parts of a single
> artificial object.

The code original was:
  if (rtx_equal_for_memref_p (x, y))
    {
      return offset_overlap_p (c, xsize, ysize);
    }
and
rtx_equal_for_memref_p (const_rtx x, const_rtx y)
{
...
    case SYMBOL_REF:
      return XSTR (x, 0) == XSTR (y, 0);

So it assumed base+offset check for all labels with same XSTR.  My patch
makes this strictly weaker: it is possible that the same variable is
accessed via its own symbol or via offset from variable anchor.
> 
> I agree there are other refinements you can do on top of that,
> e.g. that two block symbols in different blocks can never conflict.
> But the patch seems to be treating anchors as an exception to the rule,
> whereas I think they're just one instance of the rule.

Can you think of some testcase?
Doing XSTR==XSTR test and assuming a conflict otherwise will cause a conflict between
every external variable read/write and static variable read/write as one will be anchored
and other not.

Honza
> 
> Thanks,
> Richard

Re: Fix alias.c wrt aliases and anchors

[Richard Sandiford]

Jan Hubicka <hubicka@ucw.cz> writes:
>> > Hi,
>> > the alias-2.c testcase fails on targets with anchors.  The reason is that
>> > the variable itself is anchored while the alias is not and they point to the
>> > same location.   I folllowed the docs of SYMBOL_REF claiming that
>> > SYMBOL_REF_DECL if the symbol is label and tought it is safe to disambiguate
>> > them.
>> 
>> What kind of distinction do you mean between "label" and non-label here?
>> 
>> I don't think !SYMBOL_REF_DECL tells us anything useful about the
>> symbol.  I think it's more a case of: if SYMBOF_REF_DECL is present,
>> we can assume that what it says is accurate.  If it isn't present we
>> can't assume anything.
>> 
>> So was it...
>> 
>> Jan Hubicka <hubicka@ucw.cz> writes:
>> > @@ -2323,26 +2367,14 @@
>> >  
>> >    if (GET_CODE (x) == SYMBOL_REF && GET_CODE (y) == SYMBOL_REF)
>> >      {
>> > -      tree x_decl = SYMBOL_REF_DECL (x);
>> > -      tree y_decl = SYMBOL_REF_DECL (y);
>> > -      int cmp;
>> > +      int cmp = compare_base_symbol_refs (x,y);
>> >  
>> > -      if (!x_decl || !y_decl)
>> > -	{
>> > -	  /* Label and normal symbol are never the same. */
>> > -	  if (x_decl != y_decl)
>> > -	    return 0;
>> > -	  return offset_overlap_p (c, xsize, ysize);
>> 
>> ...this part of the code that was causing the problem?  That doesn't
>> seem valid even without the anchor stuff.  I think the starting position
>
> Yep, I misread the docs assuming that SYMBOL_REF_DECL == NULL only
> for code labels.  It is always safe to disambiguate these as they access
> readonly memory anyway.
>
>> should be that we can only use offset_overlap_p if XSTR (x) == XSTR (y)
>> and need to assume a conflict otherwise.  E.g. I think in principle it's
>> valid for a target to create symbol_refs for different parts of a single
>> artificial object.
>
> The code original was:
>   if (rtx_equal_for_memref_p (x, y))
>     {
>       return offset_overlap_p (c, xsize, ysize);
>     }
> and
> rtx_equal_for_memref_p (const_rtx x, const_rtx y)
> {
> ...
>     case SYMBOL_REF:
>       return XSTR (x, 0) == XSTR (y, 0);
>
> So it assumed base+offset check for all labels with same XSTR.  My patch
> makes this strictly weaker: it is possible that the same variable is
> accessed via its own symbol or via offset from variable anchor.

Well, to me "weaker" means "makes more conservative assumptions",
which in this context would be assuming a conflict in cases where
the old code didn't (i.e. returning -1 more often).  I'm not sure
whether the first patch was strictly weaker in that sense, since
if the symbol_refs were not equal according to rtx_equal_for_memref_p,
the old code would fall through to the end of the function and return -1.

>> I agree there are other refinements you can do on top of that,
>> e.g. that two block symbols in different blocks can never conflict.
>> But the patch seems to be treating anchors as an exception to the rule,
>> whereas I think they're just one instance of the rule.
>
> Can you think of some testcase?

Not a specific one, sorry, but it seems like the kind of thing that
could happen with extra ABI-mandated constructs.

But maybe the lack of a specific testcase is a good thing.  If in practice
anchors make up the vast majority of cases where (a) SYMBOL_REF_DECL
is null and (b) XSTR is too weak, there should no harm in relying on
the old XSTR comparison for the non-anchor null-decl cases.

> Doing XSTR==XSTR test and assuming a conflict otherwise will cause a
> conflict between
> every external variable read/write and static variable read/write as one
> will be anchored
> and other not.

Yeah, I think handling anchors is a good thing.  It just seems that
logically the correctness fix is to replace:

	  /* Label and normal symbol are never the same. */
 	  if (x_decl != y_decl)
	    return 0;
	  return offset_overlap_p (c, xsize, ysize);

with something like:

	  if (XSTR (x, 0) == XSTR (y, 0))
	    return offset_overlap_p (c, xsize, ysize);
	  /* Symbols might conflict.  */
	  return -1;

Handling anchors would then be a very useful optimisation on top of that.

Thanks,
Richard

Re: Fix alias.c wrt aliases and anchors

[Jan Hubicka]

> 
> Well, to me "weaker" means "makes more conservative assumptions",
> which in this context would be assuming a conflict in cases where
> the old code didn't (i.e. returning -1 more often).  I'm not sure
> whether the first patch was strictly weaker in that sense, since
> if the symbol_refs were not equal according to rtx_equal_for_memref_p,
> the old code would fall through to the end of the function and return -1.
> 
> >> I agree there are other refinements you can do on top of that,
> >> e.g. that two block symbols in different blocks can never conflict.
> >> But the patch seems to be treating anchors as an exception to the rule,
> >> whereas I think they're just one instance of the rule.
> >
> > Can you think of some testcase?
> 
> Not a specific one, sorry, but it seems like the kind of thing that
> could happen with extra ABI-mandated constructs.
> 
> But maybe the lack of a specific testcase is a good thing.  If in practice
> anchors make up the vast majority of cases where (a) SYMBOL_REF_DECL
> is null and (b) XSTR is too weak, there should no harm in relying on
> the old XSTR comparison for the non-anchor null-decl cases.
> 
> > Doing XSTR==XSTR test and assuming a conflict otherwise will cause a
> > conflict between
> > every external variable read/write and static variable read/write as one
> > will be anchored
> > and other not.
> 
> Yeah, I think handling anchors is a good thing.  It just seems that
> logically the correctness fix is to replace:
> 
> 	  /* Label and normal symbol are never the same. */
>  	  if (x_decl != y_decl)
> 	    return 0;
> 	  return offset_overlap_p (c, xsize, ysize);
> 
> with something like:
> 
> 	  if (XSTR (x, 0) == XSTR (y, 0))
> 	    return offset_overlap_p (c, xsize, ysize);
> 	  /* Symbols might conflict.  */
> 	  return -1;
> 
> Handling anchors would then be a very useful optimisation on top of that.

Ah, OK, now I get your point :)
Yep, I have no problem beling conservative for non-anchor cases !SYMBOL_REF_DECL case.
Pretty much all cases that matter are IMO either anchors or SYMBOL_REF_DECL != NULL.
(i.e. user variables).

I will update the patch and also look into the Alpha AND issues.

Honza
> 
> Thanks,
> Richard

Re: Fix alias.c wrt aliases and anchors

[Jan Hubicka]

Hi,
here is updated patch that assumes by default that RTL SYMBOL_REFs may overlap
in arbitrary way and only disambiguate refs with declarations (same way as tree
oracle does) and the anchors.

Bootstrapped/regtested ppc64-linux.
Does it look resonable?
Honza

	* alias.c (compare_base_symbol_refs): New function.
	(rtx_equal_for_memref_p, base_alias_check, memrefs_conflict_p): Use it.
Index: alias.c
===================================================================
--- alias.c	(revision 232293)
+++ alias.c	(working copy)
@@ -158,6 +158,7 @@ static tree decl_for_component_ref (tree
 static int write_dependence_p (const_rtx,
 			       const_rtx, machine_mode, rtx,
 			       bool, bool, bool);
+static int compare_base_symbol_refs (const_rtx, const_rtx);
 
 static void memory_modified_1 (rtx, const_rtx, void *);
 
@@ -1756,15 +1757,7 @@ rtx_equal_for_memref_p (const_rtx x, con
       return LABEL_REF_LABEL (x) == LABEL_REF_LABEL (y);
 
     case SYMBOL_REF:
-      {
-	tree x_decl = SYMBOL_REF_DECL (x);
-	tree y_decl = SYMBOL_REF_DECL (y);
-
-	if (!x_decl || !y_decl)
-	  return XSTR (x, 0) == XSTR (y, 0);
-	else
-	  return compare_base_decls (x_decl, y_decl) == 1;
-      }
+      return compare_base_symbol_refs (x, y) == 1;
 
     case ENTRY_VALUE:
       /* This is magic, don't go through canonicalization et al.  */
@@ -2056,6 +2049,66 @@ compare_base_decls (tree base1, tree bas
   return ret;
 }
 
+/* Same as compare_base_decls but for SYMBOL_REF.  */
+
+static int
+compare_base_symbol_refs (const_rtx x_base, const_rtx y_base)
+{
+  tree x_decl = SYMBOL_REF_DECL (x_base);
+  tree y_decl = SYMBOL_REF_DECL (y_base);
+  bool binds_def = true;
+
+  if (x_decl && y_decl)
+    return compare_base_decls (x_decl, y_decl);
+  if (x_decl || y_decl)
+    {
+      if (!x_decl)
+	{
+	  std::swap (x_decl, y_decl);
+	  std::swap (x_base, y_base);
+	}
+      /* We handle specially only section anchors and assume that other
+ 	 labels may overlap with user variables in an arbitrary way.  */
+      if (!SYMBOL_REF_HAS_BLOCK_INFO_P (y_base))
+        return -1;
+      /* Anchors contains static VAR_DECLs and CONST_DECLs.  We are safe
+	 to ignore CONST_DECLs because they are readonly.  */
+      if (TREE_CODE (x_decl) != VAR_DECL
+	  || (!TREE_STATIC (x_decl) && !TREE_PUBLIC (x_decl)))
+	return 0;
+
+      symtab_node *x_node = symtab_node::get_create (x_decl)
+			    ->ultimate_alias_target ();
+      /* External variable can not be in section anchor.  */
+      if (!x_node->definition)
+	return false;
+      x_base = XEXP (DECL_RTL (x_node->decl), 0);
+      /* If not in anchor, we can disambiguate.  */
+      if (!SYMBOL_REF_HAS_BLOCK_INFO_P (x_base))
+	return false;
+
+      /* We have an alias of anchored variable.  If it can be interposed;
+ 	 we must assume it may or may not alias its anchor.  */
+      binds_def = decl_binds_to_current_def_p (x_decl);
+    }
+  /* If we have variable in section anchor, we can compare by offset.  */
+  if (SYMBOL_REF_HAS_BLOCK_INFO_P (x_base)
+      && SYMBOL_REF_HAS_BLOCK_INFO_P (y_base))
+    {
+      if (SYMBOL_REF_BLOCK (x_base) != SYMBOL_REF_BLOCK (y_base))
+	return 0;
+      if (SYMBOL_REF_BLOCK_OFFSET (x_base) == SYMBOL_REF_BLOCK_OFFSET (y_base))
+	return binds_def ? 1 : -1;
+      /* Mixing anchors and non-anchors may result to false negative.
+	 We probably never do that.  */
+      gcc_assert (SYMBOL_REF_ANCHOR_P (x_base) == SYMBOL_REF_ANCHOR_P (y_base));
+      return 0;
+    }
+  /* In general we assume that memory locations pointed to by different labels
+     may overlap in undefined ways.  */
+  return XSTR (x_base, 0) == XSTR (y_base, 0) ? 1 : -1;
+}
+
 /* Return 0 if the addresses X and Y are known to point to different
    objects, 1 if they might be pointers to the same object.  */
 
@@ -2111,18 +2164,10 @@ base_alias_check (rtx x, rtx x_base, rtx
 	  || (int) GET_MODE_UNIT_SIZE (x_mode) < -INTVAL (XEXP (y, 1))))
     return 1;
 
+  /* Differing symbols not accessed via AND never alias.  */
   if (GET_CODE (x_base) == SYMBOL_REF && GET_CODE (y_base) == SYMBOL_REF)
-    {
-      tree x_decl = SYMBOL_REF_DECL (x_base);
-      tree y_decl = SYMBOL_REF_DECL (y_base);
-
-      /* We can assume that no stores are made to labels.  */
-      if (!x_decl || !y_decl)
-	return 0;
-      return compare_base_decls (x_decl, y_decl) != 0;
-    }
+    return compare_base_symbol_refs (x_base, y_base) != 0;
 
-  /* Differing symbols not accessed via AND never alias.  */
   if (GET_CODE (x_base) != ADDRESS && GET_CODE (y_base) != ADDRESS)
     return 0;
 
@@ -2327,19 +2372,7 @@ memrefs_conflict_p (int xsize, rtx x, in
 
   if (GET_CODE (x) == SYMBOL_REF && GET_CODE (y) == SYMBOL_REF)
     {
-      tree x_decl = SYMBOL_REF_DECL (x);
-      tree y_decl = SYMBOL_REF_DECL (y);
-      int cmp;
-
-      if (!x_decl || !y_decl)
-	{
-	  /* Label and normal symbol are never the same. */
-	  if (x_decl != y_decl)
-	    return 0;
-	  return offset_overlap_p (c, xsize, ysize);
-	}
-      else
-        cmp = compare_base_decls (x_decl, y_decl);
+      int cmp = compare_base_symbol_refs (x,y);
 
       /* If both decls are the same, decide by offsets.  */
       if (cmp == 1)

Re: Fix alias.c wrt aliases and anchors

[Richard Sandiford]

> here is updated patch that assumes by default that RTL SYMBOL_REFs may overlap
> in arbitrary way and only disambiguate refs with declarations (same way as tree
> oracle does) and the anchors.
>
> Bootstrapped/regtested ppc64-linux.
> Does it look resonable?

The logic looks ok to me FWIW.  I think it might be better (and easier
to follow) if we do the XSTR test first though.  I.e.:

  if (XSTR (x_base, 0) == XSTR (y_base, 0))
    return 1;
  if (x_decl && y_decl)
    return compare_base_decls (x_decl, y_decl);
  ...

Also:

Jan Hubicka <hubicka@ucw.cz> writes:
> +  /* If we have variable in section anchor, we can compare by offset.  */
> +  if (SYMBOL_REF_HAS_BLOCK_INFO_P (x_base)
> +      && SYMBOL_REF_HAS_BLOCK_INFO_P (y_base))
> +    {
> +      if (SYMBOL_REF_BLOCK (x_base) != SYMBOL_REF_BLOCK (y_base))
> +	return 0;
> +      if (SYMBOL_REF_BLOCK_OFFSET (x_base) == SYMBOL_REF_BLOCK_OFFSET (y_base))
> +	return binds_def ? 1 : -1;
> +      /* Mixing anchors and non-anchors may result to false negative.
> +	 We probably never do that.  */
> +      gcc_assert (SYMBOL_REF_ANCHOR_P (x_base) == SYMBOL_REF_ANCHOR_P (y_base));

...although that's the usual case, I don't think there's any practical
way of enforcing an absolute guarantee.  I think instead we should just
return -1 if either symbol is not an anchor.

Nit, but: there were a couple of 'return false's that I think
should be 'return 0's.

Thanks,
Richard

Re: Fix alias.c wrt aliases and anchors

[Uros Bizjak]

[-- Attachment #1: Type: text/plain, Size: 1778 bytes --]

Hello!

>> Yeah, I think handling anchors is a good thing.  It just seems that
>> logically the correctness fix is to replace:
>>
>>  /* Label and normal symbol are never the same. */
>>    if (x_decl != y_decl)
>>    return 0;
>>  return offset_overlap_p (c, xsize, ysize);
>>
>> with something like:
>>
>>  if (XSTR (x, 0) == XSTR (y, 0))
>>    return offset_overlap_p (c, xsize, ysize);
>>  /* Symbols might conflict.  */
>>  return -1;
>>
>> Handling anchors would then be a very useful optimisation on top of that.
>
> Ah, OK, now I get your point :)
> Yep, I have no problem beling conservative for non-anchor cases !SYMBOL_REF_DECL case.
> Pretty much all cases that matter are IMO either anchors or SYMBOL_REF_DECL != NULL.
> (i.e. user variables).
>
> I will update the patch and also look into the Alpha AND issues.

I have another version of the patch that deals with AND addresses in
testing, please see attached. The difference from the previous patch
is:

@@ -2339,6 +2337,12 @@ memrefs_conflict_p (int xsize, rtx x, int ysize, r
       /* If both decls are the same, decide by offsets.  */
       if (cmp == 1)
         return offset_overlap_p (c, xsize, ysize);
+      /* Assume a potential overlap for symbolic addresses that went
+        through alignment adjustments (i.e., that have negative
+        sizes), because we can't know how far they are from each
+        other.  */
+      if (xsize < 0 || ysize < 0)
+       return -1;
       /* If decls are different or we know by offsets that there is no overlap,
         we win.  */
       if (!cmp || !offset_overlap_p (c, xsize, ysize))

So, we simply return unknown from memrefs_conflict_p when realignment
is in play.

(We still need early return for AND addresses in base_alias_check, though).

Uros.

[-- Attachment #2: p.diff.txt --]
[-- Type: text/plain, Size: 3982 bytes --]

Index: alias.c
===================================================================
--- alias.c	(revision 231971)
+++ alias.c	(working copy)
@@ -2046,8 +2046,6 @@ compare_base_decls (tree base1, tree base2)
 
   ret = symtab_node::get_create (base1)->equal_address_to
 		 (symtab_node::get_create (base2), true);
-  if (ret == 2)
-    return -1;
   return ret;
 }
 
@@ -2088,17 +2086,6 @@ base_alias_check (rtx x, rtx x_base, rtx y, rtx y_
   if (rtx_equal_p (x_base, y_base))
     return 1;
 
-  if (GET_CODE (x_base) == SYMBOL_REF && GET_CODE (y_base) == SYMBOL_REF)
-    {
-      tree x_decl = SYMBOL_REF_DECL (x_base);
-      tree y_decl = SYMBOL_REF_DECL (y_base);
-
-      /* We can assume that no stores are made to labels.  */
-      if (!x_decl || !y_decl)
-	return 0;
-      return compare_base_decls (x_decl, y_decl) != 0;
-    }
-
   /* The base addresses are different expressions.  If they are not accessed
      via AND, there is no conflict.  We can bring knowledge of object
      alignment into play here.  For example, on alpha, "char a, b;" can
@@ -2117,6 +2104,17 @@ base_alias_check (rtx x, rtx x_base, rtx y, rtx y_
 	  || (int) GET_MODE_UNIT_SIZE (x_mode) < -INTVAL (XEXP (y, 1))))
     return 1;
 
+  if (GET_CODE (x_base) == SYMBOL_REF && GET_CODE (y_base) == SYMBOL_REF)
+    {
+      tree x_decl = SYMBOL_REF_DECL (x_base);
+      tree y_decl = SYMBOL_REF_DECL (y_base);
+
+      /* We can assume that no stores are made to labels.  */
+      if (!x_decl || !y_decl)
+	return 0;
+      return compare_base_decls (x_decl, y_decl) != 0;
+    }
+
   /* Differing symbols not accessed via AND never alias.  */
   if (GET_CODE (x_base) != ADDRESS && GET_CODE (y_base) != ADDRESS)
     return 0;
@@ -2339,6 +2337,12 @@ memrefs_conflict_p (int xsize, rtx x, int ysize, r
       /* If both decls are the same, decide by offsets.  */
       if (cmp == 1)
         return offset_overlap_p (c, xsize, ysize);
+      /* Assume a potential overlap for symbolic addresses that went
+	 through alignment adjustments (i.e., that have negative
+	 sizes), because we can't know how far they are from each
+	 other.  */
+      if (xsize < 0 || ysize < 0)
+	return -1;
       /* If decls are different or we know by offsets that there is no overlap,
 	 we win.  */
       if (!cmp || !offset_overlap_p (c, xsize, ysize))
Index: symtab.c
===================================================================
--- symtab.c	(revision 231971)
+++ symtab.c	(working copy)
@@ -1877,7 +1877,7 @@ symtab_node::nonzero_address ()
 
 /* Return 0 if symbol is known to have different address than S2,
    Return 1 if symbol is known to have same address as S2,
-   return 2 otherwise.  
+   return -1 otherwise.  
 
    If MEMORY_ACCESSED is true, assume that both memory pointer to THIS
    and S2 is going to be accessed.  This eliminates the situations when
@@ -1941,7 +1941,7 @@ symtab_node::equal_address_to (symtab_node *s2, bo
   /* If both symbols may resolve to NULL, we can not really prove them
      different.  */
   if (!memory_accessed && !nonzero_address () && !s2->nonzero_address ())
-    return 2;
+    return -1;
 
   /* Except for NULL, functions and variables never overlap.  */
   if (TREE_CODE (decl) != TREE_CODE (s2->decl))
@@ -1949,7 +1949,7 @@ symtab_node::equal_address_to (symtab_node *s2, bo
 
   /* If one of the symbols is unresolved alias, punt.  */
   if (rs1->alias || rs2->alias)
-    return 2;
+    return -1;
 
   /* If we have a non-interposale definition of at least one of the symbols
      and the other symbol is different, we know other unit can not interpose
@@ -1976,7 +1976,7 @@ symtab_node::equal_address_to (symtab_node *s2, bo
      We probably should be consistent and use this fact here, too, but for
      the moment return false only when we are called from the alias oracle.  */
 
-  return memory_accessed && rs1 != rs2 ? 0 : 2;
+  return memory_accessed && rs1 != rs2 ? 0 : -1;
 }
 
 /* Worker for call_for_symbol_and_aliases.  */