https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82224
--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
One complication when deciding whether the downstream uses are fine is
that we assign the same alias set to union accesses u->x and u->y.
That said, the union read/write pairs we remove essentially act as
an optimization barrier - they "union" the alias sets of all components
of the unions (not only of those accessed). I think that's what other
readings of the standard ask for (there's a bug about this as well).
A mere declaration of a union needs to union the alias-sets and the
following should be valid:
union { long l; long long ll; };
long foo (long *p)
{
*p = 0;
*(long long *)p = 1;
return *p;
}
this means (from an implementation perspective) that iff we do not
want to go this far then accessing a union member via a non-union
type isn't valid.
Anyway ;) I don't see an easy way to fix the testcases in this PR
without removing the optimization entirely. It's currently guarded
like
/* If the TBAA state isn't the same for downstream reads
we cannot value-number the VDEFs the same. */
alias_set_type set = get_alias_set (lhs);
if (! vnresult
|| vnresult->set == set
|| alias_set_subset_of (set, vnresult->set))
but we don't know whether vnresult is from a load or a store so we
don't know whether it fully specifies the "TBAA state". So even
dumbing that down to
if (vnresult->set != set
|| set != get_alias_set (TREE_TYPE (lhs)))
resultsame = false;
where the latter test sees if the alias set for the access and for an
access using a pointer is the same might keep some holes open.
Some of the more interesting cases _did_ involve unions and the above
would likely make them not optimized again (I remember sth with vector
intrinsics and initialization / copying via unions). Oh, even more
trivial union->x = union->x; will stop from being optimized.
Index: gcc/tree-ssa-sccvn.c
===================================================================
--- gcc/tree-ssa-sccvn.c (revision 254135)
+++ gcc/tree-ssa-sccvn.c (working copy)
@@ -3800,11 +3800,11 @@ visit_reference_op_store (tree lhs, tree
resultsame = expressions_equal_p (result, op);
if (resultsame)
{
- /* If the TBAA state isn't compatible for downstream reads
+ /* If the TBAA state isn't the same for downstream reads
we cannot value-number the VDEFs the same. */
alias_set_type set = get_alias_set (lhs);
if (vnresult->set != set
- && ! alias_set_subset_of (set, vnresult->set))
+ || set != get_alias_set (TREE_TYPE (lhs)))
resultsame = false;
}
}
@@ -5628,9 +5628,9 @@ eliminate_dom_walker::before_dom_childre
at least all accesses the later one does or if the store
was to readonly memory storing the same value. */
alias_set_type set = get_alias_set (lhs);
- if (! vnresult
- || vnresult->set == set
- || alias_set_subset_of (set, vnresult->set))
+ if (vnresult
+ && vnresult->set == set
+ && set == get_alias_set (TREE_TYPE (lhs)))
{
if (dump_file && (dump_flags & TDF_DETAILS))
{
Index: gcc/tree-ssa-dom.c
===================================================================
--- gcc/tree-ssa-dom.c (revision 254135)
+++ gcc/tree-ssa-dom.c (working copy)
@@ -46,6 +46,7 @@ along with GCC; see the file COPYING3.
#include "gimplify.h"
#include "tree-cfgcleanup.h"
#include "dbgcnt.h"
+#include "alias.h"
/* This file implements optimizations on the dominator tree. */
@@ -1953,7 +1954,8 @@ dom_opt_dom_walker::optimize_stmt (basic
gimple_set_vuse (new_stmt, gimple_vuse (stmt));
cached_lhs = m_avail_exprs_stack->lookup_avail_expr (new_stmt, false,
false);
- if (cached_lhs && operand_equal_p (rhs, cached_lhs, 0))
+ if (cached_lhs && operand_equal_p (rhs, cached_lhs, 0)
+ && get_alias_set (lhs) == get_alias_set (TREE_TYPE (lhs)))
{
basic_block bb = gimple_bb (stmt);
unlink_stmt_vdef (stmt);
FAILs at least gcc.dg/tree-ssa/ssa-pre-26.c
as said I'm not 100% convinced the more strict handling avoids all of the
cases.
