public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Improve TBAA with unions
@ 2016-05-18 11:04 Richard Biener
  2016-05-18 13:00 ` Eric Botcazou
  2016-05-24  9:16 ` Richard Biener
  0 siblings, 2 replies; 5+ messages in thread
From: Richard Biener @ 2016-05-18 11:04 UTC (permalink / raw)
  To: gcc-patches; +Cc: Joseph S. Myers, ebotcazou


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?

Thanks,
Richard.

2016-05-18  Richard Biener  <rguenther@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.

	
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 ----

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Improve TBAA with unions
  2016-05-18 11:04 [PATCH] Improve TBAA with unions Richard Biener
@ 2016-05-18 13:00 ` Eric Botcazou
  2016-05-24  9:16 ` Richard Biener
  1 sibling, 0 replies; 5+ messages in thread
From: Eric Botcazou @ 2016-05-18 13:00 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches, Joseph S. Myers

> 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.

The role of UNION_TYPE is negligible in Ada, it's only used for unchecked 
unions, which are quite rare.  Moreover, they are explicitly designed to be 
compatible with C unions so behaving like them kind of makes sense I think.

-- 
Eric Botcazou

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Improve TBAA with unions
  2016-05-18 11:04 [PATCH] Improve TBAA with unions Richard Biener
  2016-05-18 13:00 ` Eric Botcazou
@ 2016-05-24  9:16 ` Richard Biener
  2016-06-21 22:13   ` Jeff Law
  1 sibling, 1 reply; 5+ messages in thread
From: Richard Biener @ 2016-05-24  9:16 UTC (permalink / raw)
  To: gcc-patches; +Cc: Joseph S. Myers, jason

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  <rguenther@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.
> 
> 	
> 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 ----
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Improve TBAA with unions
  2016-05-24  9:16 ` Richard Biener
@ 2016-06-21 22:13   ` Jeff Law
  2016-06-29  7:43     ` Richard Biener
  0 siblings, 1 reply; 5+ messages in thread
From: Jeff Law @ 2016-06-21 22:13 UTC (permalink / raw)
  To: Richard Biener, gcc-patches; +Cc: Joseph S. Myers, jason

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  <rguenther@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.


Jeff

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Improve TBAA with unions
  2016-06-21 22:13   ` Jeff Law
@ 2016-06-29  7:43     ` Richard Biener
  0 siblings, 0 replies; 5+ messages in thread
From: Richard Biener @ 2016-06-29  7:43 UTC (permalink / raw)
  To: Jeff Law; +Cc: gcc-patches, Joseph S. Myers, jason

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  <rguenther@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  <rguenther@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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-06-29  7:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-18 11:04 [PATCH] Improve TBAA with unions Richard Biener
2016-05-18 13:00 ` Eric Botcazou
2016-05-24  9:16 ` Richard Biener
2016-06-21 22:13   ` Jeff Law
2016-06-29  7:43     ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).