[PATCH] middle-end/95493 - bogus MEM_ATTRs from variable ARRAY_REFs
Richard Biener
rguenther@suse.de
Wed Jun 3 11:38:38 GMT 2020
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.
We try to preserve the MEM_EXPR from the base object expansion
but we have to make sure to also preserve the corresponding MEM_ALIAS_SET
then since otherwise path-based disambiguation gets confused.
The other choice we'd have would be to drop the MEM_EXPR but keep
the alias-set from the full reference. Or, of course "improve"
MEM_EXPR generation to not "fail" in case the ARRAY_REF offset is
variable (we keep variable ARRAY_REFs in the MEM_EXPR if there's
a COMPONENT_REF ontop of the ref for example). I
understand the "inheriting" be exactly useful in the
case we otherwise give up (I've added an assert that we're not
inheriting from sth when creating a stack slot).
The expand_assignment fix could be treated separately (in different
code paths we are already preserving MEM_ALIAS_SET this way).
As said the alternative fix for set_mem_attrs_minus_bitpos would
be to remove all the ARRAY_REF special-casing and do the same
as we do for MEM_REF.
Bootstrap and regtest running on x86_64-unknown-linux-gnu.
Any comments?
Thanks,
Richard.
2020-06-03 Richard Biener <rguenther@suse.de>
PR middle-end/95493
* emit-rtl.c (set_mem_attrs_minus_bitpos): Clear
offset-known when the ARRAY_REF is variable. If we end up
inherting the MEM_EXPR from the original MEM_ATTRS then also
inherit the corresponding MEM_ALIAS_SET. Avoid altering
MEM_KEEP_ALIAS_SET_P in that case.
* expr.c (expand_assignment): Preserve MEM_ALIAS_SET when emitting
the store.
* g++.dg/torture/pr95493.C: New testcase.
---
gcc/emit-rtl.c | 59 ++++++++++++++----------
gcc/expr.c | 6 ++-
gcc/testsuite/g++.dg/torture/pr95493.C | 62 ++++++++++++++++++++++++++
3 files changed, 103 insertions(+), 24 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/torture/pr95493.C
diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index 2b790636366..09521b6f85e 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -1964,8 +1964,9 @@ set_mem_attributes_minus_bitpos (rtx ref, tree t, int objectp,
refattrs = MEM_ATTRS (ref);
if (refattrs)
{
- /* ??? Can this ever happen? Calling this routine on a MEM that
- already carries memory attributes should probably be invalid. */
+ gcc_assert (! TYPE_P (t));
+ /* We are calling this function with attributes set up from expanding
+ the base of a memory reference as returned by get_inner_referece. */
attrs.expr = refattrs->expr;
attrs.offset_known_p = refattrs->offset_known_p;
attrs.offset = refattrs->offset;
@@ -2052,11 +2053,6 @@ set_mem_attributes_minus_bitpos (rtx ref, tree t, int objectp,
as = TYPE_ADDR_SPACE (TREE_TYPE (base));
}
- /* If this expression uses it's parent's alias set, mark it such
- that we won't change it. */
- if (component_uses_parent_alias_set_from (t) != NULL_TREE)
- MEM_KEEP_ALIAS_SET_P (ref) = 1;
-
/* If this is a decl, set the attributes of the MEM from it. */
if (DECL_P (t))
{
@@ -2114,6 +2110,9 @@ set_mem_attributes_minus_bitpos (rtx ref, tree t, int objectp,
}
while (TREE_CODE (t2) == ARRAY_REF);
+ attrs.offset_known_p = false;
+ attrs.offset = 0;
+ apply_bitpos = 0;
if (DECL_P (t2)
|| (TREE_CODE (t2) == COMPONENT_REF
/* For trailing arrays t2 doesn't have a size that
@@ -2141,24 +2140,38 @@ set_mem_attributes_minus_bitpos (rtx ref, tree t, int objectp,
apply_bitpos = bitpos;
}
- /* If this is a reference based on a partitioned decl replace the
- base with a MEM_REF of the pointer representative we created
- during stack slot partitioning. */
- if (attrs.expr
- && VAR_P (base)
- && ! is_global_var (base)
- && cfun->gimple_df->decls_to_pointers != NULL)
+ /* If we chose to keep the MEM_EXPR from the inner reference
+ expansion make sure to also keep the corresponding alias-set
+ since otherwise path-based disambiguation is confused. */
+ if (refattrs
+ && attrs.expr == refattrs->expr)
+ attrs.alias = refattrs->alias;
+ else if (attrs.expr)
{
- tree *namep = cfun->gimple_df->decls_to_pointers->get (base);
- if (namep)
+ /* If this expression uses it's parent's alias set, mark it such
+ that we won't change it. */
+ if (component_uses_parent_alias_set_from (t) != NULL_TREE)
+ MEM_KEEP_ALIAS_SET_P (ref) = 1;
+
+ /* If this is a reference based on a partitioned decl replace the
+ base with a MEM_REF of the pointer representative we created
+ during stack slot partitioning. */
+ if (attrs.expr
+ && VAR_P (base)
+ && ! is_global_var (base)
+ && cfun->gimple_df->decls_to_pointers != NULL)
{
- attrs.expr = unshare_expr (attrs.expr);
- tree *orig_base = &attrs.expr;
- while (handled_component_p (*orig_base))
- orig_base = &TREE_OPERAND (*orig_base, 0);
- tree aptrt = reference_alias_ptr_type (*orig_base);
- *orig_base = build2 (MEM_REF, TREE_TYPE (*orig_base), *namep,
- build_int_cst (aptrt, 0));
+ tree *namep = cfun->gimple_df->decls_to_pointers->get (base);
+ if (namep)
+ {
+ attrs.expr = unshare_expr (attrs.expr);
+ tree *orig_base = &attrs.expr;
+ while (handled_component_p (*orig_base))
+ orig_base = &TREE_OPERAND (*orig_base, 0);
+ tree aptrt = reference_alias_ptr_type (*orig_base);
+ *orig_base = build2 (MEM_REF, TREE_TYPE (*orig_base), *namep,
+ build_int_cst (aptrt, 0));
+ }
}
}
diff --git a/gcc/expr.c b/gcc/expr.c
index 6b75028e7f1..2357178fce4 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -5354,6 +5354,7 @@ expand_assignment (tree to, tree from, bool nontemporal)
}
else
{
+ alias_set_type alias_set;
if (MEM_P (to_rtx))
{
/* If the field is at offset zero, we could have been given the
@@ -5362,7 +5363,10 @@ expand_assignment (tree to, tree from, bool nontemporal)
set_mem_attributes_minus_bitpos (to_rtx, to, 0, bitpos);
if (volatilep)
MEM_VOLATILE_P (to_rtx) = 1;
+ alias_set = MEM_ALIAS_SET (to_rtx);
}
+ else
+ alias_set = get_alias_set (to);
gcc_checking_assert (known_ge (bitpos, 0));
if (optimize_bitfield_assignment_op (bitsize, bitpos,
@@ -5373,7 +5377,7 @@ expand_assignment (tree to, tree from, bool nontemporal)
else
result = store_field (to_rtx, bitsize, bitpos,
bitregion_start, bitregion_end,
- mode1, from, get_alias_set (to),
+ mode1, from, alias_set,
nontemporal, reversep);
}
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.26.2
More information about the Gcc-patches
mailing list