[Bug tree-optimization/82224] Strict-aliasing not noticing valid aliasing of two unions with active members

["rguenth at gcc dot gnu.org" <gcc-bugzilla at gcc dot gnu dot org>]

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.