So this is a variant cleaning up set_mem_attributes_minus_bitpos
instead.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

I'll go with this variant since it is more obvious unless I hear
otherwise.

Thanks,
Richard.


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.

2020-06-04  Richard Biener  <rguent...@suse.de>

        PR middle-end/95493
        * 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.

        * g++.dg/torture/pr95493.C: New testcase.
---
 gcc/cfgexpand.c                        |  3 +-
 gcc/emit-rtl.c                         | 63 ++++----------------------
 gcc/testsuite/g++.dg/torture/pr95493.C | 62 +++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 55 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/torture/pr95493.C

diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 2f6ec97ed04..b270a4ddb9d 100644
--- a/gcc/cfgexpand.c
+++ b/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))
diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index 2b790636366..f9b0e9714d9 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -2067,8 +2067,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.  */
@@ -2082,59 +2084,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;
diff --git a/gcc/testsuite/g++.dg/torture/pr95493.C 
b/gcc/testsuite/g++.dg/torture/pr95493.C
new file mode 100644
index 00000000000..5e05056854d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/torture/pr95493.C
@@ -0,0 +1,62 @@
+// { 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);
+      }
+  }
+}
-- 
2.25.1

Reply via email to