[Starting with tne VN issue]
> For the VN case the issue is interpreting a read via memcpy (non
> reverse-storage) as a reverse-storage one when matching up with a hashtable
> entry from a regular reverse-storage order store, correct?
It's a read from a scalar (hence native order) combined with a store to an
aggregate type with reverse order:
u = 305419896;
__builtin_memcpy (&tempb, &u, 4);
_1 = tempb.val;
> But likewise whether the pointer destination had the attribute doesn't
> correctly qualify this - instead I think we are already properly matching up
> the reverse storage property?
No, we do not do direct matching for the storage order because you may not
access the same memory location with different scalar storage orders, it's the
fundamental invariant, so it's redundant with e.g. the offset for matching.
You need to byte swap if you want to propagate 305419896 to _1 above. But
it's an easy case: suppose that tempb is made up of 2 scalars with reverse
storage order, then it's more complicated. So memcpy where one of the types
is aggregate with reverse storage order needs to be considered a storage order
barrier, exactly like VIEW_CONVERT_EXPR:
case VIEW_CONVERT_EXPR:
temp.off = 0;
temp.reverse = storage_order_barrier_p (ref);
break;
/* Return true if T is a storage order barrier, i.e. a VIEW_CONVERT_EXPR
that can modify the storage order of objects. Note that, even if the
TYPE_REVERSE_STORAGE_ORDER flag is set on both the inner type and the
outer type, a VIEW_CONVERT_EXPR can modify the storage order because
it can change the partition of the aggregate object into scalars. */
static inline bool
storage_order_barrier_p (const_tree t)
{
if (TREE_CODE (t) != VIEW_CONVERT_EXPR)
return false;
if (AGGREGATE_TYPE_P (TREE_TYPE (t))
&& TYPE_REVERSE_STORAGE_ORDER (TREE_TYPE (t)))
return true;
tree op = TREE_OPERAND (t, 0);
if (AGGREGATE_TYPE_P (TREE_TYPE (op))
&& TYPE_REVERSE_STORAGE_ORDER (TREE_TYPE (op)))
return true;
return false;
}
hence the similar conditions, but I can add some more commentary.
> Hum. How does the pointer type matter at all for memcpy()?
> Isn't the
> issue just that we may not use the TYPE_REVERSE_STORAGE_ORDER
> qualified type in the following case:
>
> gimple *new_stmt;
> if (is_gimple_reg_type (TREE_TYPE (srcvar)))
> {
> tree tem = fold_const_aggregate_ref (srcvar);
> if (tem)
> srcvar = tem;
> if (! is_gimple_min_invariant (srcvar))
> {
> new_stmt = gimple_build_assign (NULL_TREE, srcvar);
> srcvar = create_tmp_reg_or_ssa_name (TREE_TYPE (srcvar),
> new_stmt);
>
> ?
Same rationale as above: if you want to rewrite the copy in the presence of an
aggregate type with reverse storage order, you may need to split the operation
into the appropriate pieces; one large access may be incorrect.
--
Eric Botcazoudiff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index 4e3de95d2d2..2dca9776b38 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -741,15 +741,23 @@ gimple_fold_builtin_memory_op (gimple_stmt_iterator *gsi,
}
else
{
- tree srctype, desttype;
+ /* We cannot (easily) change the type of the copy if it is a storage
+ order barrier, i.e. is equivalent to a VIEW_CONVERT_EXPR that can
+ modify the storage order of objects (see storage_order_barrier_p). */
+ tree srctype
+ = POINTER_TYPE_P (TREE_TYPE (src))
+ ? TREE_TYPE (TREE_TYPE (src)) : NULL_TREE;
+ tree desttype
+ = POINTER_TYPE_P (TREE_TYPE (dest))
+ ? TREE_TYPE (TREE_TYPE (dest)) : NULL_TREE;
unsigned int src_align, dest_align;
- tree off0;
- const char *tmp_str;
unsigned HOST_WIDE_INT tmp_len;
+ const char *tmp_str;
/* Build accesses at offset zero with a ref-all character type. */
- off0 = build_int_cst (build_pointer_type_for_mode (char_type_node,
- ptr_mode, true), 0);
+ tree off0
+ = build_int_cst (build_pointer_type_for_mode (char_type_node,
+ ptr_mode, true), 0);
/* If we can perform the copy efficiently with first doing all loads
and then all stores inline it that way. Currently efficiently
@@ -767,7 +775,13 @@ gimple_fold_builtin_memory_op (gimple_stmt_iterator *gsi,
hack can be removed. */
&& !c_strlen (src, 1)
&& !((tmp_str = c_getstr (src, &tmp_len)) != NULL
- && memchr (tmp_str, 0, tmp_len) == NULL))
+ && memchr (tmp_str, 0, tmp_len) == NULL)
+ && !(srctype
+ && AGGREGATE_TYPE_P (srctype)
+ && TYPE_REVERSE_STORAGE_ORDER (srctype))
+ && !(desttype
+ && AGGREGATE_TYPE_P (desttype)
+ && TYPE_REVERSE_STORAGE_ORDER (desttype)))
{
unsigned ilen = tree_to_uhwi (len);
if (pow2p_hwi (ilen))
@@ -947,8 +961,13 @@ gimple_fold_builtin_memory_op (gimple_stmt_iterator *gsi,
if (!tree_fits_shwi_p (len))
return false;
- if (!POINTER_TYPE_P (TREE_TYPE (src))
- || !POINTER_TYPE_P (TREE_TYPE (dest)))
+ if (!srctype
+ || (AGGREGATE_TYPE_P (srctype)
+ && TYPE_REVERSE_STORAGE_ORDER (srctype)))
+ return false;
+ if (!desttype
+ || (AGGREGATE_TYPE_P (desttype)
+ && TYPE_REVERSE_STORAGE_ORDER (desttype)))
return false;
/* In the following try to find a type that is most natural to be
used for the memcpy source and destination and that allows
@@ -956,11 +975,9 @@ gimple_fold_builtin_memory_op (gimple_stmt_iterator *gsi,
using that type. In theory we could always use a char[len] type
but that only gains us that the destination and source possibly
no longer will have their address taken. */
- srctype = TREE_TYPE (TREE_TYPE (src));
if (TREE_CODE (srctype) == ARRAY_TYPE
&& !tree_int_cst_equal (TYPE_SIZE_UNIT (srctype), len))
srctype = TREE_TYPE (srctype);
- desttype = TREE_TYPE (TREE_TYPE (dest));
if (TREE_CODE (desttype) == ARRAY_TYPE
&& !tree_int_cst_equal (TYPE_SIZE_UNIT (desttype), len))
desttype = TREE_TYPE (desttype);
diff --git a/gcc/tree-ssa-sccvn.c b/gcc/tree-ssa-sccvn.c
index 4b3f31c12cb..e269f7885f4 100644
--- a/gcc/tree-ssa-sccvn.c
+++ b/gcc/tree-ssa-sccvn.c
@@ -3224,8 +3224,10 @@ vn_reference_lookup_3 (ao_ref *ref, tree vuse, void *data_,
return NULL;
}
- /* 6) For memcpy copies translate the reference through them if
- the copy kills ref. */
+ /* 6) For memcpy copies translate the reference through them if the copy
+ kills ref. But we cannot (easily) do this translation if the memcpy is
+ a storage order barrier, i.e. is equivalent to a VIEW_CONVERT_EXPR that
+ can modify the storage order of objects (see storage_order_barrier_p). */
else if (data->vn_walk_kind == VN_WALKREWRITE
&& is_gimple_reg_type (vr->type)
/* ??? Handle BCOPY as well. */
@@ -3275,6 +3277,9 @@ vn_reference_lookup_3 (ao_ref *ref, tree vuse, void *data_,
}
if (TREE_CODE (lhs) == ADDR_EXPR)
{
+ if (AGGREGATE_TYPE_P (TREE_TYPE (TREE_TYPE (lhs)))
+ && TYPE_REVERSE_STORAGE_ORDER (TREE_TYPE (TREE_TYPE (lhs))))
+ return (void *)-1;
tree tem = get_addr_base_and_unit_offset (TREE_OPERAND (lhs, 0),
&lhs_offset);
if (!tem)
@@ -3303,6 +3308,9 @@ vn_reference_lookup_3 (ao_ref *ref, tree vuse, void *data_,
rhs = vn_valueize (rhs);
if (TREE_CODE (rhs) == ADDR_EXPR)
{
+ if (AGGREGATE_TYPE_P (TREE_TYPE (TREE_TYPE (rhs)))
+ && TYPE_REVERSE_STORAGE_ORDER (TREE_TYPE (TREE_TYPE (rhs))))
+ return (void *)-1;
tree tem = get_addr_base_and_unit_offset (TREE_OPERAND (rhs, 0),
&rhs_offset);
if (!tem)
