middle-end/95493 - bogus MEM_ATTRS for variable array access

The following patch avoids keeping the inherited MEM_ATTRS when
set_mem_attributes_minus_bitpos is called with a variable ARRAY_REF.
The inherited ones may not reflect the correct offset and neither
does the updated alias-set match the inherited MEM_EXPR.  This all
ends up confusing path-based alias-analysis, causing wrong-code.

The fix is to stop not adopting a MEM_EXPR for certain kinds of
expressions and instead handle everything we can.  There's still
the constant kind trees case which I'm too lazy to look into right
now.  I did refrain from adding SSA_NAME there and instead avoided
calling set_mem_attributes_minus_bitpos when debug expression
expansion ended up expanding a SSA definition RHS which should
already have taken care of setting the appropriate MEM_ATTRS.

It also avoids calling set_mem_attributes on the
DECL_INITIAL of a CONST_DECL which seems pointless since there
cannot be a sensible MEM_EXPR derived from that.  We're overwriting
both other possibly useful info, alias-set and alignment immediately
so the following patch simply removes the call instead of making
the function deal with even more (unexpected) trees that are not
memory accesses.

2020-06-23  Richard Biener  <rguenther@suse.de>

	PR middle-end/95493
	PR middle-end/95690
	* cfgexpand.c (expand_debug_expr): Avoid calling
	set_mem_attributes_minus_bitpos when we were expanding
	an SSA name.
	* emit-rtl.c (set_mem_attributes_minus_bitpos): Remove
	ARRAY_REF special-casing, add CONSTRUCTOR to the set of
	special-cases we do not want MEM_EXPRs for.  Assert
	we end up with reasonable MEM_EXPRs.
	* varasm.c (build_constant_desc): Remove set_mem_attributes call.

	* g++.dg/torture/pr95493.C: New testcase.
	* g++.dg/torture/pr95493-1.C: Likewise.
	* gfortran.dg/pr95690.f90: Likewise.

gcc/cfgexpand.c

@@ -4616,7 +4616,8 @@ expand_debug_expr (tree exp)
 	      op0 = copy_rtx (op0);
 	    if (op0 == orig_op0)
 	      op0 = shallow_copy_rtx (op0);
-	    set_mem_attributes (op0, exp, 0);
+	    if (TREE_CODE (tem) != SSA_NAME)
+	      set_mem_attributes (op0, exp, 0);
 	  }
 
 	if (known_eq (bitpos, 0) && mode == GET_MODE (op0))

gcc/emit-rtl.c

@@ -2065,8 +2065,10 @@ set_mem_attributes_minus_bitpos (rtx ref, tree t, int objectp,
 	  new_size = DECL_SIZE_UNIT (t);
 	}
 
-      /* ???  If we end up with a constant here do record a MEM_EXPR.  */
-      else if (CONSTANT_CLASS_P (t))
+      /* ???  If we end up with a constant or a descriptor do not
+	 record a MEM_EXPR.  */
+      else if (CONSTANT_CLASS_P (t)
+	       || TREE_CODE (t) == CONSTRUCTOR)
 	;
 
       /* If this is a field reference, record it.  */
@@ -2080,59 +2082,12 @@ set_mem_attributes_minus_bitpos (rtx ref, tree t, int objectp,
 	    new_size = DECL_SIZE_UNIT (TREE_OPERAND (t, 1));
 	}
 
-      /* If this is an array reference, look for an outer field reference.  */
-      else if (TREE_CODE (t) == ARRAY_REF)
-	{
-	  tree off_tree = size_zero_node;
-	  /* We can't modify t, because we use it at the end of the
-	     function.  */
-	  tree t2 = t;
-
-	  do
-	    {
-	      tree index = TREE_OPERAND (t2, 1);
-	      tree low_bound = array_ref_low_bound (t2);
-	      tree unit_size = array_ref_element_size (t2);
-
-	      /* We assume all arrays have sizes that are a multiple of a byte.
-		 First subtract the lower bound, if any, in the type of the
-		 index, then convert to sizetype and multiply by the size of
-		 the array element.  */
-	      if (! integer_zerop (low_bound))
-		index = fold_build2 (MINUS_EXPR, TREE_TYPE (index),
-				     index, low_bound);
-
-	      off_tree = size_binop (PLUS_EXPR,
-				     size_binop (MULT_EXPR,
-						 fold_convert (sizetype,
-							       index),
-						 unit_size),
-				     off_tree);
-	      t2 = TREE_OPERAND (t2, 0);
-	    }
-	  while (TREE_CODE (t2) == ARRAY_REF);
-
-	  if (DECL_P (t2)
-	      || (TREE_CODE (t2) == COMPONENT_REF
-		  /* For trailing arrays t2 doesn't have a size that
-		     covers all valid accesses.  */
-		  && ! array_at_struct_end_p (t)))
-	    {
-	      attrs.expr = t2;
-	      attrs.offset_known_p = false;
-	      if (poly_int_tree_p (off_tree, &attrs.offset))
-		{
-		  attrs.offset_known_p = true;
-		  apply_bitpos = bitpos;
-		}
-	    }
-	  /* Else do not record a MEM_EXPR.  */
-	}
-
-      /* If this is an indirect reference, record it.  */
-      else if (TREE_CODE (t) == MEM_REF 
-	       || TREE_CODE (t) == TARGET_MEM_REF)
+      /* Else record it.  */
+      else
 	{
+	  gcc_assert (handled_component_p (t)
+		      || TREE_CODE (t) == MEM_REF
+		      || TREE_CODE (t) == TARGET_MEM_REF);
 	  attrs.expr = t;
 	  attrs.offset_known_p = true;
 	  attrs.offset = 0;

gcc/testsuite/g++.dg/torture/pr95493-1.C

+// { dg-do run { target c++11 } }
+// PR rtl-optimization/95493 comment 8
+
+#include <array>
+#include <iostream>
+
+struct Point
+{
+    std::array<int, 3> array;
+
+    Point(int x, int y, int z) : array{x, y, z} {}
+    
+    Point(const Point & other) : array{other.array} {} // OK if commented
+    //Point(const Point &) = default; // OK
+
+    //Point(Point && other) = default; // OK
+
+    int  operator[] (std::size_t i) const { return array[i]; }
+    int& operator[] (std::size_t i)       { return array[i]; }
+};
+
+//using Point = std::array<int, 3>; // OK
+
+struct Cell
+{
+    Point point;
+    Cell(Point const& pt) : point(pt) {}
+    int   operator[] (std::size_t i) const { return point[i]; }
+    int&  operator[] (std::size_t i)       { return point[i]; }
+};
+
+//using Cell = Point; // OK
+
+std::ostream & operator<< (std::ostream & out, Cell const& object)
+//std::ostream & operator<< (std::ostream & out, Cell object) // Fails with f2 too
+{
+    for ( std::size_t i = 0; i < 3; ++i )
+        out << object[ i ] << " ";
+    return out;
+}
+
+
+struct DirIterator
+{
+    std::size_t dir;
+    Cell cell;
+
+    DirIterator(Cell c)
+        : dir(0), cell(c)
+    {
+        find(); // OK if commented
+    }
+
+    void find()
+    {
+        //while (dir < 3) // Fails with f2 too
+        while (dir < 3 && (cell[dir] % 2) == 0)
+            ++dir;
+    }
+};
+
+Cell uIncident(Cell c, std::size_t k)
+//Cell uIncident(Cell& c, std::size_t k) // OK
+{
+    --c[k];
+    return c;
+}
+
+Cell uSpel(Point p)
+{
+    for (std::size_t i = 0; i < 3; ++i)
+        p[i] += p[i] + 1;
+    return Cell(p);
+}
+
+
+int main ()
+{
+    Cell c = uSpel(Point{0, 0, 0}); // Fails
+    //Cell c( Point(1, 1, 1) ); // OK
+
+    auto q = DirIterator( c );
+
+    Cell f1 = uIncident( c, q.dir ); // Fails
+    //Cell f1 = uIncident( c, 0 ); // OK
+
+    Cell f2 = f1; // f2 is the right cell that f1 should be
+
+    std::cout << "q.dir = " << q.dir << " ; f1 = " << f1 << " ; f2 = " << f2 << std::endl;
+    //std::cout << "q.dir = " << q.dir << " ; f1 = " << f1[0] << " " << f1[1] << " " << f1[2] << " ; f2 = " << f2[0] << " " << f2[1] << " " << f2[2] << std::endl; // OK
+
+    for (int i = 0; i < 3; ++i)
+      if (f1[i] != f2[i])
+        __builtin_abort();
+}

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

+// { dg-do run }
+// { dg-additional-options "-std=c++17" }
+
+struct verify
+{
+  const bool m_failed = false;
+
+  [[gnu::noinline]] void print_hex(const void* x, int n) const
+  {
+    const auto* bytes = static_cast<const unsigned char*>(x);
+    for (int i = 0; i < n; ++i)
+      __builtin_printf((i && i % 4 == 0) ? "'%02x" : "%02x", bytes[i]);
+    __builtin_printf("\n");
+  }
+
+  template <typename... Ts>
+  verify(bool ok, const Ts&... extra_info) : m_failed(!ok)
+  {
+    if (m_failed)
+      (print_hex(&extra_info, sizeof(extra_info)), ...);
+  }
+
+  ~verify()
+  {
+    if (m_failed)
+      __builtin_abort();
+  }
+};
+
+using K [[gnu::vector_size(16)]] = int;
+
+int
+main()
+{
+  int count = 1;
+  asm("" : "+m"(count));
+  verify(count == 1, 0, "", 0);
+
+  {
+    struct SW
+    {
+      K d;
+    };
+    struct
+    {
+      SW d;
+    } xx;
+    SW& x = xx.d;
+    x = SW(); // [0, 0, 0, 0]
+    for (int i = 3; i >= 2; --i)
+      {
+        x.d[i] = -1; // [0, 0, 0, -1] ...
+        int a = [](K y) {
+          for (int j = 0; j < 4; ++j)
+            if (y[j] != 0)
+              return j;
+          return -1;
+        }(x.d);
+        verify(a == i, 0, 0, 0, 0, i, x);
+      }
+  }
+}

gcc/testsuite/gfortran.dg/pr95690.f90

+! { dg-do compile }
+module m
+contains
+   subroutine s
+      print *, (erfc) ! { dg-error "not a floating constant" }
+   end
+   function erfc()
+   end
+end

gcc/varasm.c

@@ -3425,7 +3425,6 @@ build_constant_desc (tree exp)
   TREE_CONSTANT_POOL_ADDRESS_P (symbol) = 1;
 
   rtl = gen_const_mem (TYPE_MODE (TREE_TYPE (exp)), symbol);
-  set_mem_attributes (rtl, exp, 1);
   set_mem_alias_set (rtl, 0);
 
   /* Putting EXP into the literal pool might have imposed a different