Re: Incorrect bitfield aliasing with Tree SSA

[Adam Nemet <anemet at caviumnetworks dot com>]

Richard Kenner writes:
> I continue to strongly feel that the field type shouldn't be used
> for ANYTHING!

I still think there is value in a separate alias set but in the hope
that this thread will end soon, here is a patch to special-case
non-addressable fields in the SFT code.  I will test this overnight
but it fixes my issue with bitfields and preserves the RTL
optimization on Eric's Ada testcase.

Adam

	* tree.h (struct tree_struct_field_tag): Add new field alias_set.
	(SFT_NONADDRESSABLE_P, SFT_ALIAS_SET): New macros.
	* tree-flow.h (struct fieldoff): Add new field alias_set.
	* tree-ssa-structalias.c (push_fields_onto_fieldstack): Add new
	argument addressable_type.  Set alias_set of fieldoff.
	* tree-ssa-alias.c (create_sft): Add new argument alias_set.
	(create_overlap_variables_for): Pass alias_set from fieldoff to
	create_sft.
	* alias.c (get_alias_set): Use alias_set from SFT if set.

	* testsuite/gcc.dg/tree-ssa/alias-14.c: New test.

Index: tree.h
===================================================================
--- tree.h	(revision 125588)
+++ tree.h	(working copy)
@@ -2504,10 +2504,15 @@ struct tree_struct_field_tag GTY(())
   /* Size of the field.  */
   unsigned HOST_WIDE_INT size;
 
+  /* Alias set for a DECL_NONADDRESSABLE_P field.  Otherwise -1.  */
+  HOST_WIDE_INT alias_set;
 };
 #define SFT_PARENT_VAR(NODE) (STRUCT_FIELD_TAG_CHECK (NODE)->sft.parent_var)
 #define SFT_OFFSET(NODE) (STRUCT_FIELD_TAG_CHECK (NODE)->sft.offset)
 #define SFT_SIZE(NODE) (STRUCT_FIELD_TAG_CHECK (NODE)->sft.size)
+#define SFT_NONADDRESSABLE_P(NODE) \
+  (STRUCT_FIELD_TAG_CHECK (NODE)->sft.alias_set != -1)
+#define SFT_ALIAS_SET(NODE) (STRUCT_FIELD_TAG_CHECK (NODE)->sft.alias_set)
 
 /* Memory Partition Tags (MPTs) group memory symbols under one
    common name for the purposes of placing memory PHI nodes.  */
Index: tree-ssa-alias.c
===================================================================
--- tree-ssa-alias.c	(revision 125588)
+++ tree-ssa-alias.c	(working copy)
@@ -3637,11 +3637,13 @@ get_or_create_used_part_for (size_t uid)
 
 
 /* Create and return a structure sub-variable for field type FIELD at
-   offset OFFSET, with size SIZE, of variable VAR.  */
+   offset OFFSET, with size SIZE, of variable VAR.  If ALIAS_SET not
+   -1 this field is non-addressable and we should use this alias set
+   with this field.  */
 
 static tree
 create_sft (tree var, tree field, unsigned HOST_WIDE_INT offset,
-	    unsigned HOST_WIDE_INT size)
+	    unsigned HOST_WIDE_INT size, HOST_WIDE_INT alias_set)
 {
   tree subvar = create_tag_raw (STRUCT_FIELD_TAG, field, "SFT");
 
@@ -3660,6 +3662,7 @@ create_sft (tree var, tree field, unsign
   SFT_PARENT_VAR (subvar) = var;
   SFT_OFFSET (subvar) = offset;
   SFT_SIZE (subvar) = size;
+  SFT_ALIAS_SET (subvar) = alias_set;
   return subvar;
 }
 
@@ -3679,7 +3682,8 @@ create_overlap_variables_for (tree var)
       || up->write_only)
     return;
 
-  push_fields_onto_fieldstack (TREE_TYPE (var), &fieldstack, 0, NULL);
+  push_fields_onto_fieldstack (TREE_TYPE (var), &fieldstack, 0, NULL,
+			       TREE_TYPE (var));
   if (VEC_length (fieldoff_s, fieldstack) != 0)
     {
       subvar_t *subvars;
@@ -3771,7 +3775,8 @@ create_overlap_variables_for (tree var)
 	    continue;
 	  sv = GGC_NEW (struct subvar);
 	  sv->next = *subvars;
-	  sv->var = create_sft (var, fo->type, fo->offset, fosize);
+	  sv->var =
+	    create_sft (var, fo->type, fo->offset, fosize, fo->alias_set);
 
 	  if (dump_file)
 	    {
Index: alias.c
===================================================================
--- alias.c	(revision 125588)
+++ alias.c	(working copy)
@@ -585,6 +585,13 @@ get_alias_set (tree t)
 	    return 0;
 	}
 
+      /* For non-addressable fields we return the alias set of the
+	 outermost object that could have its address taken.  If this
+	 is an SFT use the precomputed value.  */
+      if (TREE_CODE (t) == STRUCT_FIELD_TAG
+	  && SFT_NONADDRESSABLE_P (t))
+	return SFT_ALIAS_SET (t);
+
       /* Otherwise, pick up the outermost object that we could have a pointer
 	 to, processing conversions as above.  */
       while (component_uses_parent_alias_set (t))
Index: tree-flow.h
===================================================================
--- tree-flow.h	(revision 125588)
+++ tree-flow.h	(working copy)
@@ -1156,13 +1156,14 @@ struct fieldoff
   tree size;
   tree decl;
   HOST_WIDE_INT offset;  
+  HOST_WIDE_INT alias_set;
 };
 typedef struct fieldoff fieldoff_s;
 
 DEF_VEC_O(fieldoff_s);
 DEF_VEC_ALLOC_O(fieldoff_s,heap);
 int push_fields_onto_fieldstack (tree, VEC(fieldoff_s,heap) **,
-				 HOST_WIDE_INT, bool *);
+				 HOST_WIDE_INT, bool *, tree);
 void sort_fieldstack (VEC(fieldoff_s,heap) *);
 
 void init_alias_heapvars (void);
Index: tree-ssa-structalias.c
===================================================================
--- tree-ssa-structalias.c	(revision 125588)
+++ tree-ssa-structalias.c	(working copy)
@@ -3672,11 +3672,13 @@ sort_fieldstack (VEC(fieldoff_s,heap) *f
    than just the immediately containing structure.  Returns the number
    of fields pushed.
    HAS_UNION is set to true if we find a union type as a field of
-   TYPE.  */
+   TYPE.  ADDRESSABLE_TYPE is the type of the outermost object that could have
+   its address taken.  */
 
 int
 push_fields_onto_fieldstack (tree type, VEC(fieldoff_s,heap) **fieldstack,
-			     HOST_WIDE_INT offset, bool *has_union)
+			     HOST_WIDE_INT offset, bool *has_union,
+			     tree addressable_type)
 {
   tree field;
   int count = 0;
@@ -3689,12 +3691,14 @@ push_fields_onto_fieldstack (tree type, 
       real_part->size = TYPE_SIZE (TREE_TYPE (type));
       real_part->offset = offset;
       real_part->decl = NULL_TREE;
+      real_part->alias_set = -1;
 
       img_part = VEC_safe_push (fieldoff_s, heap, *fieldstack, NULL);
       img_part->type = TREE_TYPE (type);
       img_part->size = TYPE_SIZE (TREE_TYPE (type));
       img_part->offset = offset + TREE_INT_CST_LOW (TYPE_SIZE (TREE_TYPE (type)));
       img_part->decl = NULL_TREE;
+      img_part->alias_set = -1;
 
       return 2;
     }
@@ -3732,7 +3736,8 @@ push_fields_onto_fieldstack (tree type, 
 	    push = true;
 	  else if (!(pushed = push_fields_onto_fieldstack
 		     (TREE_TYPE (type), fieldstack,
-		      offset + i * TREE_INT_CST_LOW (elsz), has_union)))
+		      offset + i * TREE_INT_CST_LOW (elsz), has_union,
+		      TREE_TYPE (type))))
 	    /* Empty structures may have actual size, like in C++. So
 	       see if we didn't push any subfields and the size is
 	       nonzero, push the field onto the stack */
@@ -3747,6 +3752,7 @@ push_fields_onto_fieldstack (tree type, 
 	      pair->size = elsz;
 	      pair->decl = NULL_TREE;
 	      pair->offset = offset + i * TREE_INT_CST_LOW (elsz);
+	      pair->alias_set = -1;
 	      count++;
 	    }
 	  else
@@ -3771,7 +3777,10 @@ push_fields_onto_fieldstack (tree type, 
 	  push = true;
 	else if (!(pushed = push_fields_onto_fieldstack
 		   (TREE_TYPE (field), fieldstack,
-		    offset + bitpos_of_field (field), has_union))
+		    offset + bitpos_of_field (field), has_union,
+		    (DECL_NONADDRESSABLE_P (field)
+		     ? addressable_type
+		     : TREE_TYPE (field))))
 		 && DECL_SIZE (field)
 		 && !integer_zerop (DECL_SIZE (field)))
 	  /* Empty structures may have actual size, like in C++. So
@@ -3788,6 +3797,10 @@ push_fields_onto_fieldstack (tree type, 
 	    pair->size = DECL_SIZE (field);
 	    pair->decl = field;
 	    pair->offset = offset + bitpos_of_field (field);
+	    if (DECL_NONADDRESSABLE_P (field))
+	      pair->alias_set = get_alias_set (addressable_type);
+	    else
+	      pair->alias_set = -1;
 	    count++;
 	  }
 	else
@@ -3986,7 +3999,8 @@ create_variable_info_for (tree decl, con
 	     || TREE_CODE (decltype) == QUAL_UNION_TYPE;
   if (var_can_have_subvars (decl) && use_field_sensitive && !hasunion)
     {
-      push_fields_onto_fieldstack (decltype, &fieldstack, 0, &hasunion);
+      push_fields_onto_fieldstack (decltype, &fieldstack, 0, &hasunion,
+				   decltype);
       if (hasunion)
 	{
 	  VEC_free (fieldoff_s, heap, fieldstack);
Index: testsuite/gcc.dg/tree-ssa/alias-14.c
===================================================================
--- testsuite/gcc.dg/tree-ssa/alias-14.c	(revision 0)
+++ testsuite/gcc.dg/tree-ssa/alias-14.c	(revision 0)
@@ -0,0 +1,20 @@
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+struct s
+{
+  long long a:12;
+  long long b:12;
+  long long c:40;
+};
+
+struct s s, *p = &s;
+
+int
+main ()
+{
+  p->a = 1;
+  s.a = 0;
+  s.b = 0;
+  return p->a + s.b;
+}