middle-end: Fix wrong code caused by disagreemed between FRE and access path oracle [PR 92152]

FRE is checking stores for equivalence based on their address, value and
base+ref alias sets.  Because ref alias set is not always the alias set of
innermost type, but it may be one of refs in the access path (as decided by
component_uses_parent_alias_set_from) it means that we can not really rely on
the remaining part of access path to be meaningful in any way except for
offset+size computation.

The patch makes alias (which is used by FRE to validate transform) and
tree-ssa-alias to share same logic for ending the access path relevant for
TBAA. tree-ssa-alias previously ended access paths on VIEW_CONVERT_EXPR and
BIT_FIELD_REF so it is not hard to wire in common predicate.  However it led to
additional issues (I tried to read the code quite carefully for possible extra
fun, so I hope I found it all):

  1) alias_component_refs_walk compares base and reference sizes to see
     if one access path may continue by another.  This check can be confused
     by an union containing structure with zero sized array.  In this case we
     no longer see the refernece to zero sized array and think that ref size
     is 0.

     In an access path there can be at most one (valid) trailing/zero sized
     array access, so the sizes in the access path are decreasing with the
     this exception. This is already handled by the logic, however the access
     is not expected to happen past the end of TBAA segment.  I suppose this
     was kind of latent problem before because one can think of access path
     doing traling array past VIEW_CONVERT_EXPR, but since in C code we don't
     VCE and in non-C we don't do trailing arrays, we did not hit the problem.

     I fixed this by tracking if the trailing array references appearing after
     the end of TBAA access path and mostly punt in the second case (because we
     need to support kind of all type puning here). I do not think we can assume
     much of sanity here, in particular, we no longer know there is only one
     because FRE may mix things up.

     An exception is the walk that looks for occurence of basetype of path1
     within TBAA relevant part of path2.  Here we realy care about TBAA
     relevant parts of paths and thus do not need to give up.

     I broke out the logic into ends_tbaa_access_path_p to avoid duplication and
     to let me stick some detailed comments. This became much more complex
     than I originally imagined (still it is useful to make oracle both faster
     and more precise).

     Note that logic in aliasing_component_refs_walk is safe since it works
     on TBAA relevant segments of paths only.
  2) nonoverlapping_refs_since_match_p is using TBAA only in the corner case
     that the paths got out of sync and re-synchronize of types of same size
     are found.  I thus extended it to whole paths (not only TBAA relevant
     parts) and track if the TBAA part can be used by counting of number of
     TBAA relevant res on the stack.

     I have noticed that in one case we call nonoverlapping_refs_since_match_p
     before checking for view converting MEM_REFs and in others we check
     after.  I think we want to just disable TBAA part if view convert
     is in there but still disambiguate.  I will do this incrementaly.
  3) nonoverlapping_component_refs_p uses TBAA so it needs to punt on
     end of TBAA path. It deals with no sizes and thus there is not the issue
     as in 1).

I am also attaching one (most probably) valid C++ testcase (by Mark Williams)
where we incorrectly disambiguated while the code is valid by the common
initial sequence rule.  This happens to be fixed by same patch. Here one access
goes through union and follows by access path trhough one filed, while other
access path start by different field of the union with common initial sequence.
This made aliasing_component_refs_p to not find the overlapping type (because
there is none) and disambiguate.  Now we cut the first access path by the union
reference and this makes us to find the path continuation in
alias_component_refs_walk.

If FRE is ever made more careful about access paths past the fist union
reference (I think that would be good idea since unions are quite common in C++
and we throw away quite useful info) then we will need to teach access path
oracle about the common initial sequence rule (which, as Mark pointed out, is
part of both C and C++ standards).

Only argument that can possibly invalidate this testcase is that I do not see
that stadnard is clear about the situation where one access path contains the
union but other starts after the union.

Clearly if both start after the union reference we are right to disambiguate
(since there is no union unvolved).  If both starts before union then there is
common initial sequence and by standard it is defined. This case works on current
trunk because aliasing_component_refs_p resorts to base+offset after finding
the match. But even that is more or less an accident I would say.

I had to xfail three testcases.  While alias-access-path ones are artificial
and odd, 20030807-7 is derived from gcc and shows that we give up on
disambiguations of tree_node union, so this patch disables useful transform
in real world code.

I am still planning to collect some data on the effect of this change to TBAA,
but unless we want to reorganize FRE, I do not think there is better solution.

gcc/ChangeLog:

2020-02-26  Jan Hubicka  <hubicka@ucw.cz>

	PR middle-end/92152
	* alias.c (ends_tbaa_access_path_p): Break out from ...
	(component_uses_parent_alias_set_from): ... here.
	* alias.h (ends_tbaa_access_path_p): Declare.
	* tree-ssa-alias.c (access_path_may_continue_p): Break out from ...;
	handle trailing arrays past end of tbaa access path.
	(aliasing_component_refs_p): ... here; likewise.
	(nonoverlapping_refs_since_match_p): Track TBAA segment of the access
	path; disambiguate also past end of it.
	(nonoverlapping_component_refs_p): Use only TBAA segment of the access
	path.

gcc/testsuite/ChangeLog:

2020-02-26  Jan Hubicka  <hubicka@ucw.cz>

	PR middle-end/92152
	* gcc.dg/tree-ssa/alias-access-path-12.c: New testcase.
	* g++.dg/torture/pr92152.C: New testcase.
	* gcc.dg/torture/pr92152.c: New testcase.
	* gcc.dg/tree-ssa/20030807-7.c: xfail.
	* gcc.dg/tree-ssa/alias-access-path-4.c: xfail one case.
	* gcc.dg/tree-ssa/alias-access-path-5.c: xfail one case.

gcc/ChangeLog

+2020-02-27  Jan Hubicka  <hubicka@ucw.cz>
+
+	PR middle-end/92152
+	* alias.c (ends_tbaa_access_path_p): Break out from ...
+	(component_uses_parent_alias_set_from): ... here.
+	* alias.h (ends_tbaa_access_path_p): Declare.
+	* tree-ssa-alias.c (access_path_may_continue_p): Break out from ...;
+	handle trailing arrays past end of tbaa access path.
+	(aliasing_component_refs_p): ... here; likewise.
+	(nonoverlapping_refs_since_match_p): Track TBAA segment of the access
+	path; disambiguate also past end of it.
+	(nonoverlapping_component_refs_p): Use only TBAA segment of the access
+	path.
+
 2020-02-27  Mihail Ionescu  <mihail.ionescu@arm.com>
 
 	* (__ARM_NUM_LANES, __arm_lane, __arm_lane_q): Move to the

gcc/alias.c

@@ -587,6 +587,49 @@ objects_must_conflict_p (tree t1, tree t2)
   return alias_sets_must_conflict_p (set1, set2);
 }
 
+/* Return true if T is an end of the access path which can be used
+   by type based alias oracle.  */
+
+bool
+ends_tbaa_access_path_p (const_tree t)
+{
+  switch (TREE_CODE (t))
+    {
+    case COMPONENT_REF:
+      if (DECL_NONADDRESSABLE_P (TREE_OPERAND (t, 1)))
+	return true;
+      /* Permit type-punning when accessing a union, provided the access
+	 is directly through the union.  For example, this code does not
+	 permit taking the address of a union member and then storing
+	 through it.  Even the type-punning allowed here is a GCC
+	 extension, albeit a common and useful one; the C standard says
+	 that such accesses have implementation-defined behavior.  */
+      else if (TREE_CODE (TREE_TYPE (TREE_OPERAND (t, 0))) == UNION_TYPE)
+	return true;
+      break;
+
+    case ARRAY_REF:
+    case ARRAY_RANGE_REF:
+      if (TYPE_NONALIASED_COMPONENT (TREE_TYPE (TREE_OPERAND (t, 0))))
+	return true;
+      break;
+
+    case REALPART_EXPR:
+    case IMAGPART_EXPR:
+      break;
+
+    case BIT_FIELD_REF:
+    case VIEW_CONVERT_EXPR:
+      /* Bitfields and casts are never addressable.  */
+      return true;
+      break;
+
+    default:
+      gcc_unreachable ();
+    }
+  return false;
+}
+
 /* Return the outermost parent of component present in the chain of
    component references handled by get_inner_reference in T with the
    following property:
@@ -601,40 +644,8 @@ component_uses_parent_alias_set_from (const_tree t)
 
   while (handled_component_p (t))
     {
-      switch (TREE_CODE (t))
-	{
-	case COMPONENT_REF:
-	  if (DECL_NONADDRESSABLE_P (TREE_OPERAND (t, 1)))
-	    found = t;
-	  /* Permit type-punning when accessing a union, provided the access
-	     is directly through the union.  For example, this code does not
-	     permit taking the address of a union member and then storing
-	     through it.  Even the type-punning allowed here is a GCC
-	     extension, albeit a common and useful one; the C standard says
-	     that such accesses have implementation-defined behavior.  */
-	  else if (TREE_CODE (TREE_TYPE (TREE_OPERAND (t, 0))) == UNION_TYPE)
-	    found = t;
-	  break;
-
-	case ARRAY_REF:
-	case ARRAY_RANGE_REF:
-	  if (TYPE_NONALIASED_COMPONENT (TREE_TYPE (TREE_OPERAND (t, 0))))
-	    found = t;
-	  break;
-
-	case REALPART_EXPR:
-	case IMAGPART_EXPR:
-	  break;
-
-	case BIT_FIELD_REF:
-	case VIEW_CONVERT_EXPR:
-	  /* Bitfields and casts are never addressable.  */
-	  found = t;
-	  break;
-
-	default:
-	  gcc_unreachable ();
-	}
+      if (ends_tbaa_access_path_p (t))
+	found = t;
 
       t = TREE_OPERAND (t, 0);
     }

gcc/alias.h

@@ -26,6 +26,7 @@ extern alias_set_type get_deref_alias_set (tree);
 extern alias_set_type get_varargs_alias_set (void);
 extern alias_set_type get_frame_alias_set (void);
 extern tree component_uses_parent_alias_set_from (const_tree);
+extern bool ends_tbaa_access_path_p (const_tree);
 extern bool alias_set_subset_of (alias_set_type, alias_set_type);
 extern void record_alias_subset (alias_set_type, alias_set_type);
 extern void record_component_aliases (tree);

gcc/testsuite/ChangeLog

+2020-02-27  Jan Hubicka  <hubicka@ucw.cz>
+
+	PR middle-end/92152
+	* gcc.dg/tree-ssa/alias-access-path-12.c: New testcase.
+	* g++.dg/torture/pr92152.C: New testcase.
+	* gcc.dg/torture/pr92152.c: New testcase.
+	* gcc.dg/tree-ssa/20030807-7.c: xfail.
+	* gcc.dg/tree-ssa/alias-access-path-4.c: xfail one case.
+	* gcc.dg/tree-ssa/alias-access-path-5.c: xfail one case.
+
 2020-02-27  Mihail Ionescu  <mihail.ionescu@arm.com>
 
 	* gcc.target/arm/bf16_dup.c: New test.

gcc/testsuite/g++.dg/torture/pr92152.C

+/* { dg-do run } */
+using size_t = unsigned long;
+using uint64_t = unsigned long;
+
+namespace HPHP {
+
+inline void bitvec_set(uint64_t* bits, size_t index) {
+  bits[index / 64] |= 1ull << (index % 64);
+}
+
+namespace jit {
+///////////////////////////////////////////////////////////////////////////////
+
+struct VregSet {
+  struct Block;
+
+  VregSet() : blocks{} { blocks.data[1] = 1LL << 0x30; };
+
+  VregSet(const VregSet& o)
+    : blocks{o.blocks}
+  {
+  }
+
+  bool any() const {
+    if (!isExtended()) return blocks.any();
+    return true;
+  }
+  void removePhys() {
+    auto const b = !isExtended() ? &blocks : extended.blocks;
+    b->data[0] = 0;
+    b->data[1] = 0;
+  }
+  static constexpr size_t kBitsPerBlock = 256;
+  bool isExtended() const {
+    return extended.data[1] & (1ULL << (kExtendedBit % 64));
+  }
+
+  static constexpr size_t kExtendedBit = 127;
+
+  struct Block {
+    bool any() const {
+      return data[0] | data[1];
+    }
+    uint64_t data[2];
+  };
+  struct Extended {
+    uint64_t data[2];
+    Block* blocks;
+  };
+
+  union {
+    Block blocks{};
+    Extended extended;
+  };
+};
+
+//////////////////////////////////////////////////////////////////////
+
+
+__attribute__((noinline))
+bool test(VregSet&& c) {
+  auto copy = c;
+  copy.removePhys();
+  return copy.any();
+}
+///////////////////////////////////////////////////////////////////////////////
+}}
+///////////////////////////////////////////////////////////////////////////////
+
+int main() {
+  if (HPHP::jit::test(HPHP::jit::VregSet{}))
+    __builtin_abort ();
+  return 0;
+} 

gcc/testsuite/gcc.dg/torture/pr92152.c

+/* { dg-do run } */
+union U
+{
+  long long i;
+  long f;
+} v;
+ 	
+long
+foo (long *f)
+{
+  *f = 1;
+  v.i = 0;
+  v.f = 0;
+  return *f;
+}
+ 	
+int
+main ()
+{
+  if (foo (&v.f) != 0)
+    __builtin_abort ();
+  return 0;
+}

gcc/testsuite/gcc.dg/tree-ssa/20030807-7.c

@@ -34,4 +34,4 @@ simplify_condition (cond_p)
 }
 
 /* There should be exactly one IF conditional.  */
-/* { dg-final { scan-tree-dump-times "if " 1 "vrp1" } } */
+/* { dg-final { scan-tree-dump-times "if " 1 "vrp1" { xfail *-*-* } } } */

gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-12.c

+/* { dg-do compile } */
+/* { dg-options "-O1 -fno-strict-aliasing -fdump-tree-optimized" } */
+
+struct S
+{
+  int i;
+  int j;
+};
+union U
+{
+  struct S a[10];
+};
+int
+foo (union U *u, int n, int i, int j)
+{
+  u->a[i].i = 123;
+  u->a[j].j = j;
+  return u->a[i].i;
+}
+/* { dg-final { scan-tree-dump-times "return 123" 1 "optimized"} } */

gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-4.c

@@ -20,5 +20,5 @@ test2 (struct b *bptr1, union c *cptr, int i, int j)
   cptr->b.a[j].v1=1;
   return bptr1->a[i].v1;
 }
-/* { dg-final { scan-tree-dump-times "return 123" 1 "optimized"} } */
+/* { dg-final { scan-tree-dump-times "return 123" 1 "optimized"  { xfail *-*-* } } } */
 /* { dg-final { scan-tree-dump-not "return 124" "optimized"} } */

gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-5.c

@@ -21,5 +21,5 @@ test2 (struct b *bptr1, union c *cptr, int i, int j)
   cptr->b.a[j].v1=1;
   return bptr1->a[i].v1;
 }
-/* { dg-final { scan-tree-dump-times "return 123" 1 "optimized"} } */
+/* { dg-final { scan-tree-dump-times "return 123" 1 "optimized"  { xfail *-*-* } } } */
 /* { dg-final { scan-tree-dump-not "return 124" "optimized"} } */