On Tue, 21 Jun 2016, Jeff Law wrote:
> On 05/24/2016 03:14 AM, Richard Biener wrote:
> > On Wed, 18 May 2016, Richard Biener wrote:
> >
> > >
> > > The following adjusts get_alias_set beahvior when applied to
> > > union accesses to use the union alias-set rather than alias-set
> > > zero. This is in line with behavior from the alias oracle
> > > which (bogously) circumvents alias-set zero with looking at
> > > the alias-sets of the base object. Thus for
> > >
> > > union U { int i; float f; };
> > >
> > > float
> > > foo (union U *u, double *p)
> > > {
> > > u->f = 1.;
> > > *p = 0;
> > > return u->f;
> > > }
> > >
> > > the langhooks ensured u->f has alias-set zero and thus disambiguation
> > > against *p was not allowed. Still the alias-oracle did the disambiguation
> > > by using the alias set of the union here (I think optimizing the
> > > return to return 1. is valid).
> > >
> > > We have a good place in the middle-end to apply such rules which
> > > is component_uses_parent_alias_set_from - this is where I move
> > > the logic that is duplicated in various frontends.
> > >
> > > The Java and Ada frontends do not allow union type punning (LTO does),
> > > so this patch may eventually pessimize them. I don't care anything
> > > about Java but Ada folks might want to chime in.
> > >
> > > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> > >
> > > Ok for trunk?
> >
> > Ping.
> >
> > Thanks,
> > Richard.
> >
> > > Thanks,
> > > Richard.
> > >
> > > 2016-05-18 Richard Biener <rguent...@suse.de>
> > >
> > > * alias.c (component_uses_parent_alias_set_from): Handle
> > > type punning through union accesses by using the union alias set.
> > > * gimple.c (gimple_get_alias_set): Remove union type punning case.
> > >
> > > c-family/
> > > * c-common.c (c_common_get_alias_set): Remove union type punning case.
> > >
> > > fortran/
> > > * f95-lang.c (LANG_HOOKS_GET_ALIAS_SET): Remove (un-)define.
> > > (gfc_get_alias_set): Remove.
> You know the aliasing rules better than I. If you're confident using the
> union's alias set is safe, then it's OK with me.
>
> My only worry is that if we get it wrong, it's likely to be a subtle bug that
> may take a long time to expose itself. But that in and of itself shouldn't
> stop us from going forward.
Ok, I have installed the patch now. I had to adjust
g++.dg/torture/pr71002.C which was on the border of being invalid
(it relied on us using alias-set zero for all union accesses).
Basically it had union { struct A a; struct B a; } u; and accessed
that memory using a type of struct C. The fix to the testcase is trivial,
just put C into the union, the PR talks about that being difficult
for the origin of the testcase as C is said to be not POD while the
union type has to be. Still doesn't make it valid IMHO.
Bootstrapped / tested on x86_64-unknown-linux-gnu, applied.
Richard.
2016-06-29 Richard Biener <rguent...@suse.de>
PR middle-end/71002
* alias.c (component_uses_parent_alias_set_from): Handle
type punning through union accesses by using the union alias set.
* gimple.c (gimple_get_alias_set): Remove union type punning case.
c-family/
* c-common.c (c_common_get_alias_set): Remove union type punning case.
fortran/
* f95-lang.c (LANG_HOOKS_GET_ALIAS_SET): Remove (un-)define.
(gfc_get_alias_set): Remove.
* g++.dg/torture/pr71002.C: Adjust testcase.
Index: trunk/gcc/alias.c
===================================================================
*** trunk.orig/gcc/alias.c 2016-05-18 11:15:41.744792403 +0200
--- trunk/gcc/alias.c 2016-05-18 11:31:40.139709782 +0200
*************** component_uses_parent_alias_set_from (co
*** 619,624 ****
--- 619,632 ----
case COMPONENT_REF:
if (DECL_NONADDRESSABLE_P (TREE_OPERAND (t, 1)))
found = t;
+ /* Permit type-punning when accessing a union, provided the access
+ is directly through the union. For example, this code does not
+ permit taking the address of a union member and then storing
+ through it. Even the type-punning allowed here is a GCC
+ extension, albeit a common and useful one; the C standard says
+ that such accesses have implementation-defined behavior. */
+ else if (TREE_CODE (TREE_TYPE (TREE_OPERAND (t, 0))) == UNION_TYPE)
+ found = t;
break;
case ARRAY_REF:
Index: trunk/gcc/c-family/c-common.c
===================================================================
*** trunk.orig/gcc/c-family/c-common.c 2016-05-18 11:15:41.744792403 +0200
--- trunk/gcc/c-family/c-common.c 2016-05-18 11:31:40.143709828 +0200
*************** static GTY(()) hash_table<c_type_hasher>
*** 4734,4741 ****
alias_set_type
c_common_get_alias_set (tree t)
{
- tree u;
-
/* For VLAs, use the alias set of the element type rather than the
default of alias set 0 for types compared structurally. */
if (TYPE_P (t) && TYPE_STRUCTURAL_EQUALITY_P (t))
--- 4734,4739 ----
*************** c_common_get_alias_set (tree t)
*** 4745,4763 ****
return -1;
}
- /* Permit type-punning when accessing a union, provided the access
- is directly through the union. For example, this code does not
- permit taking the address of a union member and then storing
- through it. Even the type-punning allowed here is a GCC
- extension, albeit a common and useful one; the C standard says
- that such accesses have implementation-defined behavior. */
- for (u = t;
- TREE_CODE (u) == COMPONENT_REF || TREE_CODE (u) == ARRAY_REF;
- u = TREE_OPERAND (u, 0))
- if (TREE_CODE (u) == COMPONENT_REF
- && TREE_CODE (TREE_TYPE (TREE_OPERAND (u, 0))) == UNION_TYPE)
- return 0;
-
/* That's all the expressions we handle specially. */
if (!TYPE_P (t))
return -1;
--- 4743,4748 ----
Index: trunk/gcc/fortran/f95-lang.c
===================================================================
*** trunk.orig/gcc/fortran/f95-lang.c 2016-05-18 11:15:41.744792403 +0200
--- trunk/gcc/fortran/f95-lang.c 2016-05-18 11:31:48.623806334 +0200
*************** static bool global_bindings_p (void);
*** 74,80 ****
static bool gfc_init (void);
static void gfc_finish (void);
static void gfc_be_parse_file (void);
- static alias_set_type gfc_get_alias_set (tree);
static void gfc_init_ts (void);
static tree gfc_builtin_function (tree);
--- 74,79 ----
*************** static const struct attribute_spec gfc_a
*** 110,116 ****
#undef LANG_HOOKS_MARK_ADDRESSABLE
#undef LANG_HOOKS_TYPE_FOR_MODE
#undef LANG_HOOKS_TYPE_FOR_SIZE
- #undef LANG_HOOKS_GET_ALIAS_SET
#undef LANG_HOOKS_INIT_TS
#undef LANG_HOOKS_OMP_PRIVATIZE_BY_REFERENCE
#undef LANG_HOOKS_OMP_PREDETERMINED_SHARING
--- 109,114 ----
*************** static const struct attribute_spec gfc_a
*** 142,148 ****
#define LANG_HOOKS_PARSE_FILE gfc_be_parse_file
#define LANG_HOOKS_TYPE_FOR_MODE gfc_type_for_mode
#define LANG_HOOKS_TYPE_FOR_SIZE gfc_type_for_size
- #define LANG_HOOKS_GET_ALIAS_SET gfc_get_alias_set
#define LANG_HOOKS_INIT_TS gfc_init_ts
#define LANG_HOOKS_OMP_PRIVATIZE_BY_REFERENCE gfc_omp_privatize_by_reference
#define LANG_HOOKS_OMP_PREDETERMINED_SHARING gfc_omp_predetermined_sharing
--- 140,145 ----
*************** gfc_init_decl_processing (void)
*** 503,526 ****
}
- /* Return the typed-based alias set for T, which may be an expression
- or a type. Return -1 if we don't do anything special. */
-
- static alias_set_type
- gfc_get_alias_set (tree t)
- {
- tree u;
-
- /* Permit type-punning when accessing an EQUIVALENCEd variable or
- mixed type entry master's return value. */
- for (u = t; handled_component_p (u); u = TREE_OPERAND (u, 0))
- if (TREE_CODE (u) == COMPONENT_REF
- && TREE_CODE (TREE_TYPE (TREE_OPERAND (u, 0))) == UNION_TYPE)
- return 0;
-
- return -1;
- }
-
/* Builtin function initialization. */
static tree
--- 500,505 ----
Index: trunk/gcc/gimple.c
===================================================================
*** trunk.orig/gcc/gimple.c 2016-05-18 11:15:41.744792403 +0200
--- trunk/gcc/gimple.c 2016-05-18 11:31:40.143709828 +0200
*************** gimple_signed_type (tree type)
*** 2396,2416 ****
alias_set_type
gimple_get_alias_set (tree t)
{
- tree u;
-
- /* Permit type-punning when accessing a union, provided the access
- is directly through the union. For example, this code does not
- permit taking the address of a union member and then storing
- through it. Even the type-punning allowed here is a GCC
- extension, albeit a common and useful one; the C standard says
- that such accesses have implementation-defined behavior. */
- for (u = t;
- TREE_CODE (u) == COMPONENT_REF || TREE_CODE (u) == ARRAY_REF;
- u = TREE_OPERAND (u, 0))
- if (TREE_CODE (u) == COMPONENT_REF
- && TREE_CODE (TREE_TYPE (TREE_OPERAND (u, 0))) == UNION_TYPE)
- return 0;
-
/* That's all the expressions we handle specially. */
if (!TYPE_P (t))
return -1;
--- 2396,2401 ----
Index: gcc/testsuite/g++.dg/torture/pr71002.C
===================================================================
--- gcc/testsuite/g++.dg/torture/pr71002.C (revision 237837)
+++ gcc/testsuite/g++.dg/torture/pr71002.C (working copy)
@@ -16,11 +16,6 @@ struct long_t
char* pointer;
};
-union long_raw_t {
- unsigned char data[sizeof(long_t)];
- struct __attribute__((aligned(alignof(long_t)))) { } align;
-};
-
struct short_header
{
unsigned char is_short : 1;
@@ -35,20 +30,20 @@ struct short_t
union repr_t
{
- long_raw_t r;
+ long_t r;
short_t s;
const short_t& short_repr() const
{ return s; }
const long_t& long_repr() const
- { return *static_cast<const long_t*>(static_cast<const void*>(&r)); }
+ { return r; }
short_t& short_repr()
{ return s; }
long_t& long_repr()
- { return *static_cast<long_t*>(static_cast<void*>(&r)); }
+ { return r; }
};
class string
