[patch] PR32901: handle assignment from constructors better

[patch] PR32901: handle assignment from constructors better

[Aldy Hernandez]

Hi folks.

The problem in the PR is that a constructor with multiple bitfield
constants gets put into static storage, instead of being calculated at
compile time.

	struct foo {
	        unsigned a1: 1;
	        unsigned a2: 3;
	        unsigned : 4;
	};

	extern struct foo thefoo;

	void setup_foo(void) {
	        const struct foo init = {
			.a1 = 1,
                	.a2 = 5,
        	};
        	thefoo = init;
	}

In the snippet above, we put the constant 11 in static storage, then
initialize "thefoo", by reading from memory.

In my fix below, when initializing a variable from a const variable that
was initialized from a constant constructor, we pull the constant
constructor to the RHS of the assignment.  In the above example, we will
end up with:

	thefoo.0 = {};
	thefoo.0.a1 = 1;
	thefoo.0.a2 = 5;

Which unfortunately, will pass pretty much unmodified through the tree
optimizations, but combine saves the day and calculates the 11 constant
and make the replacement.

As Jakub points out in the PR, we should ideally (also) have a tree pass
that optimizes consecutive constant stores into aggregates, by
calculating the constant-- but we don't.

This patch fixes the problem in the PR.

Bootstrapped finished.  Tests are almost done.

OK pending tests?

	* gimplify.c (gimplify_modify_expr_rhs): Handle the case when we
	are assigning from a constant constructor.
	Fix wrapping in function comment.

Index: gimplify.c
===================================================================
--- gimplify.c	(revision 130773)
+++ gimplify.c	(working copy)
@@ -3470,8 +3470,9 @@ fold_indirect_ref_rhs (tree t)
   return NULL_TREE;
 }
 
-/* Subroutine of gimplify_modify_expr to do simplifications of MODIFY_EXPRs
-   based on the code of the RHS.  We loop for as long as something changes.  */
+/* Subroutine of gimplify_modify_expr to do simplifications of
+   MODIFY_EXPRs based on the code of the RHS.  We loop for as long as
+   something changes.  */
 
 static enum gimplify_status
 gimplify_modify_expr_rhs (tree *expr_p, tree *from_p, tree *to_p, tree *pre_p,
@@ -3482,6 +3483,19 @@ gimplify_modify_expr_rhs (tree *expr_p, 
   while (ret != GS_UNHANDLED)
     switch (TREE_CODE (*from_p))
       {
+      case VAR_DECL:
+	/* If we're assigning from a constant constructor, move the
+	   constructor expression to the RHS of the MODIFY_EXPR.
+	*/
+	if (DECL_INITIAL (*from_p)
+	    && TYPE_READONLY (TREE_TYPE (*from_p))
+	    && TREE_CODE (DECL_INITIAL (*from_p)) == CONSTRUCTOR)
+	  {
+	    *from_p = DECL_INITIAL (*from_p);
+	    ret = GS_OK;
+	  }
+	ret = GS_UNHANDLED;
+	break;
       case INDIRECT_REF:
 	{
 	  /* If we have code like 

Re: [patch] PR32901: handle assignment from constructors better

[Aldy Hernandez]

Hi folks!

I forgot to include the testsuite change.  Appended below.

And btw, tests finished.  No regressions.

Thoughts, approvals?

        * gimplify.c (gimplify_modify_expr_rhs): Handle the case when we
        are assigning from a constant constructor.
        Fix wrapping in function comment.

--- gimplify.c	(revision 130776)
+++ gimplify.c	(local)
@@ -3470,8 +3470,9 @@ fold_indirect_ref_rhs (tree t)
   return NULL_TREE;
 }
 
-/* Subroutine of gimplify_modify_expr to do simplifications of MODIFY_EXPRs
-   based on the code of the RHS.  We loop for as long as something changes.  */
+/* Subroutine of gimplify_modify_expr to do simplifications of
+   MODIFY_EXPRs based on the code of the RHS.  We loop for as long as
+   something changes.  */
 
 static enum gimplify_status
 gimplify_modify_expr_rhs (tree *expr_p, tree *from_p, tree *to_p, tree *pre_p,
@@ -3482,6 +3483,19 @@ gimplify_modify_expr_rhs (tree *expr_p, 
   while (ret != GS_UNHANDLED)
     switch (TREE_CODE (*from_p))
       {
+      case VAR_DECL:
+	/* If we're assigning from a constant constructor, move the
+	   constructor expression to the RHS of the MODIFY_EXPR.
+	*/
+	if (DECL_INITIAL (*from_p)
+	    && TYPE_READONLY (TREE_TYPE (*from_p))
+	    && TREE_CODE (DECL_INITIAL (*from_p)) == CONSTRUCTOR)
+	  {
+	    *from_p = DECL_INITIAL (*from_p);
+	    ret = GS_OK;
+	  }
+	ret = GS_UNHANDLED;
+	break;
       case INDIRECT_REF:
 	{
 	  /* If we have code like 
--- testsuite/gcc.dg/tree-ssa/pr32901.c	(revision 130776)
+++ testsuite/gcc.dg/tree-ssa/pr32901.c	(local)
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-gimple" } */
+
+struct foo {
+        unsigned a1: 1;
+        unsigned a2: 3;
+        unsigned : 4;
+};
+
+extern struct foo thefoo;
+
+void setup_foo(void)
+{
+        const struct foo init = {
+                .a1 = 1,
+                .a2 = 5,
+        };
+        thefoo = init;
+}
+
+/* { dg-final { scan-tree-dump-times "thefoo.0 = \{\}" 1 "gimple"} } */
+/* { dg-final { scan-tree-dump-times "thefoo.0.a1 = 1" 1 "gimple"} } */
+/* { dg-final { scan-tree-dump-times "thefoo.0.a2 = 5" 1 "gimple"} } */
+/* { dg-final { cleanup-tree-dump "gimple" } } */

Re: [patch] PR32901: handle assignment from constructors better

[Diego Novillo]

On 12/11/07 2:22 PM, Aldy Hernandez wrote:

> 	* gimplify.c (gimplify_modify_expr_rhs): Handle the case when we
> 	are assigning from a constant constructor.
> 	Fix wrapping in function comment.

OK, with:

> +      case VAR_DECL:
> +	/* If we're assigning from a constant constructor, move the
> +	   constructor expression to the RHS of the MODIFY_EXPR.
> +	*/

Closing comment on previous line.

You also need to add the PR reference to the CL entry.


Diego.

Re: [patch] PR32901: handle assignment from constructors better

[Diego Novillo]

On 12/11/07 6:15 PM, Aldy Hernandez wrote:

>         * gimplify.c (gimplify_modify_expr_rhs): Handle the case when we
>         are assigning from a constant constructor.
>         Fix wrapping in function comment.

Yes.  With ChangeLog entry for the test.


Diego.

Re: [patch] PR32901: handle assignment from constructors better

[Jakub Jelinek]

On Tue, Dec 11, 2007 at 08:36:49PM -0500, Diego Novillo wrote:
> On 12/11/07 6:15 PM, Aldy Hernandez wrote:
> 
> >        * gimplify.c (gimplify_modify_expr_rhs): Handle the case when we
> >        are assigning from a constant constructor.
> >        Fix wrapping in function comment.
> 
> Yes.  With ChangeLog entry for the test.

Sorry to chime in, but I'm afraid this needs some tuning.
Consider e.g.

struct foo {
  unsigned long int a1, a2, a3, a4, a5, a6, a7, a8, a9, a10;
  unsigned long int b1, b2, b3, b4, b5, b6, b7, b8, b9, b10;
};

extern struct foo v1, v2, v3, v4;

void setup_foo(void)
{
  const struct foo init = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20 };

  v1 = init;
  v2 = init;
  v3 = init;
  v4 = init;
}

Before this patch we got (*optimized dump):
setup_foo ()
{
  static const struct foo init = {.a1=1, .a2=2, .a3=3, .a4=4, .a5=5, .a6=6, .a7=7, .a8=8, .a9=9, .a10=10, .b1=11, .b2=12, .b3=13, .b4=14, .b5=15, .b6=16, .b7=17, .b8=18, .b9=19, .b10=20};
 
<bb 2>:
  v1 = init;
  v2 = init;
  v3 = init;
  v4 = init;
  return;

}

now we get:

setup_foo ()
{
  static const struct foo C.3 = {.a1=1, .a2=2, .a3=3, .a4=4, .a5=5, .a6=6, .a7=7, .a8=8, .a9=9, .a10=10, .b1=11, .b2=12, .b3=13, .b4=14, .b5=15, .b6=16, .b7=17, .b8=18, .b9=19, .b10=20};
  static const struct foo C.2 = {.a1=1, .a2=2, .a3=3, .a4=4, .a5=5, .a6=6, .a7=7, .a8=8, .a9=9, .a10=10, .b1=11, .b2=12, .b3=13, .b4=14, .b5=15, .b6=16, .b7=17, .b8=18, .b9=19, .b10=20};
  static const struct foo C.1 = {.a1=1, .a2=2, .a3=3, .a4=4, .a5=5, .a6=6, .a7=7, .a8=8, .a9=9, .a10=10, .b1=11, .b2=12, .b3=13, .b4=14, .b5=15, .b6=16, .b7=17, .b8=18, .b9=19, .b10=20};
  static const struct foo C.0 = {.a1=1, .a2=2, .a3=3, .a4=4, .a5=5, .a6=6, .a7=7, .a8=8, .a9=9, .a10=10, .b1=11, .b2=12, .b3=13, .b4=14, .b5=15, .b6=16, .b7=17, .b8=18, .b9=19, .b10=20};

<bb 2>:
  v1 = C.0;
  v2 = C.1;
  v3 = C.2;
  v4 = C.3;
  return;

}

There are several things which should be done IMHO:
1) this new gimplify_modify_expr_rhs optimization should do some checks
   that are done in gimplify_init_constructor, and not do this
   optimization if that would create a new C.* static constant.
   See that if (size > 0 && !can_move_by_pieces (size, align)) in
   gimplify_init_constructor.
2) independently from this optimization, we should set some flag
   on these create_tmp_var_name ("C") statics (not those when
   a const var with initializer is promoted to the static) and if
   -fmerge-constants (default at -O2) put them into mergeable sections,
   or at least remove duplicates within each TU.  As these C.* statics
   are only ever used by compiler generated code to initialize some vars,
   it really can be shared if the content is the same.
   2) should help if we have source like:
struct foo {
  unsigned long int a1, a2, a3, a4, a5, a6, a7, a8, a9, a10;
  unsigned long int b1, b2, b3, b4, b5, b6, b7, b8, b9, b10;
};

extern struct foo v1, v2, v3, v4;

void setup_foo(void)
{
  v1 = (const struct foo) { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20 };
  v2 = (const struct foo) { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20 };
  v3 = (const struct foo) { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20 };
  v4 = (const struct foo) { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20 };
}

	Jakub

Re: [patch] PR32901: handle assignment from constructors better

[Mark Mitchell]

Diego Novillo wrote:
> On 12/11/07 6:15 PM, Aldy Hernandez wrote:
> 
>>         * gimplify.c (gimplify_modify_expr_rhs): Handle the case when we
>>         are assigning from a constant constructor.
>>         Fix wrapping in function comment.
> 
> Yes.  With ChangeLog entry for the test.

This patch is not correct as-is: it does not take into account things
that are both "const" and "volatile".  In particular:

+	if (DECL_INITIAL (*from_p)
+	    && TYPE_READONLY (TREE_TYPE (*from_p))
+	    && TREE_CODE (DECL_INITIAL (*from_p)) == CONSTRUCTOR)

needs a !TYPE_VOLATILE check.  Aldy, please update that accordingly.

(For reference, "const volatile struct S s = { ... };" means that the
variable initially has the value specified, that the program is not
allowed to change it, but that it might magically change due to magic
hardware or whatever.)

This isn't a very common thing to do, but we need to be paranoid...

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: [patch] PR32901: handle assignment from constructors better

[Aldy Hernandez]

> This patch is not correct as-is: it does not take into account things
> that are both "const" and "volatile".  In particular:

I'll address your corrections and Jakub's before end of day tomorrow.

Thanks for the reviews guys.

Aldy

Re: [patch] PR32901: handle assignment from constructors better

[Richard Guenther]

On Dec 12, 2007 10:56 PM, Aldy Hernandez <aldyh@redhat.com> wrote:
> > This patch is not correct as-is: it does not take into account things
> > that are both "const" and "volatile".  In particular:
>
> I'll address your corrections and Jakub's before end of day tomorrow.
>
> Thanks for the reviews guys.

For reference, this also causes PR34448.

Richard.

Re: [patch] PR32901: handle assignment...(and PR34465 and PR34448)

[Aldy Hernandez]

Hi folks.

This just ain't my week.  I seem to be causing more PRs than I'm
closing.

This patch addresses Mark's concern regarding volatiles.  It also fixes
34448 by not allowing the optimization to run if
gimplify_init_constructor will dump the constructor to memory.  And
finally, it fixes the tree sharing problem in PR34465.

The patch bootstraps.  I'm still running tests, but I'm fixing an ICE it
has encountered, so I'm not ready to commit.  I wanted to get feedback
while tests run, and also let folks know I haven't dropped the ball on
this, I'm fixing everything :).

Also... in a subsequent patch we should really look into merging
consecutive initializations of aggregate fields-- sometime before
expand.  Jakub has suggested TER.  I volunteer to do the work once I'm
done with this.

How does this look guys?

Aldy

	PR tree-optimization/34448
	PR tree-optimization/34465
	* gimplify.c (gimplify_init_constructor): Add new parameter
	notify_temp_creation.  Use it.
	(gimplify_modify_expr_rhs): Take volatiles into account when
	optimizing constructors.
	Do not optimize constructors if gimplify_init_constructor will dump to
	memory.
	* gcc.dg/tree-ssa/pr32901.c: Tests const volatiles.
	* gcc.c-torture/compile/pr34448.c: New.

--- gimplify.c	(revision 130934)
+++ gimplify.c	(local)
@@ -3119,11 +3119,18 @@ gimplify_init_ctor_eval (tree object, VE
 
    Note that we still need to clear any elements that don't have explicit
    initializers, so if not all elements are initialized we keep the
-   original MODIFY_EXPR, we just remove all of the constructor elements.  */
+   original MODIFY_EXPR, we just remove all of the constructor elements.
+
+   If NOTIFY_TEMP_CREATION is true, do not gimplify, just return
+   GS_ERROR if we would have to create a temporary when gimplifying
+   this constructor.  Otherwise, return GS_OK.
+
+   If NOTIFY_TEMP_CREATION is false, just do the gimplification.  */
 
 static enum gimplify_status
 gimplify_init_constructor (tree *expr_p, tree *pre_p,
-			   tree *post_p, bool want_value)
+			   tree *post_p, bool want_value,
+			   bool notify_temp_creation)
 {
   tree object;
   tree ctor = GENERIC_TREE_OPERAND (*expr_p, 1);
@@ -3134,10 +3141,13 @@ gimplify_init_constructor (tree *expr_p,
   if (TREE_CODE (ctor) != CONSTRUCTOR)
     return GS_UNHANDLED;
 
-  ret = gimplify_expr (&GENERIC_TREE_OPERAND (*expr_p, 0), pre_p, post_p,
-		       is_gimple_lvalue, fb_lvalue);
-  if (ret == GS_ERROR)
-    return ret;
+  if (!notify_temp_creation)
+    {
+      ret = gimplify_expr (&GENERIC_TREE_OPERAND (*expr_p, 0), pre_p, post_p,
+			   is_gimple_lvalue, fb_lvalue);
+      if (ret == GS_ERROR)
+	return ret;
+    }
   object = GENERIC_TREE_OPERAND (*expr_p, 0);
 
   elts = CONSTRUCTOR_ELTS (ctor);
@@ -3159,7 +3169,11 @@ gimplify_init_constructor (tree *expr_p,
 	   individual elements.  The exception is that a CONSTRUCTOR node
 	   with no elements indicates zero-initialization of the whole.  */
 	if (VEC_empty (constructor_elt, elts))
-	  break;
+	  {
+	    if (notify_temp_creation)
+	      return GS_OK;
+	    break;
+	  }
 
 	/* Fetch information about the constructor to direct later processing.
 	   We might want to make static versions of it in various cases, and
@@ -3175,6 +3189,8 @@ gimplify_init_constructor (tree *expr_p,
 	    && TREE_READONLY (object)
 	    && TREE_CODE (object) == VAR_DECL)
 	  {
+	    if (notify_temp_creation)
+	      return GS_ERROR;
 	    DECL_INITIAL (object) = ctor;
 	    TREE_STATIC (object) = 1;
 	    if (!DECL_NAME (object))
@@ -3251,7 +3267,12 @@ gimplify_init_constructor (tree *expr_p,
 
 	    if (size > 0 && !can_move_by_pieces (size, align))
 	      {
-		tree new = create_tmp_var_raw (type, "C");
+		tree new;
+
+		if (notify_temp_creation)
+		  return GS_ERROR;
+
+		new = create_tmp_var_raw (type, "C");
 
 		gimple_add_tmp_var (new);
 		TREE_STATIC (new) = 1;
@@ -3273,6 +3294,9 @@ gimplify_init_constructor (tree *expr_p,
 	      }
 	  }
 
+	if (notify_temp_creation)
+	  return GS_OK;
+
 	/* If there are nonzero elements, pre-evaluate to capture elements
 	   overlapping with the lhs into temporaries.  We must do this before
 	   clearing to fetch the values before they are zeroed-out.  */
@@ -3312,6 +3336,9 @@ gimplify_init_constructor (tree *expr_p,
       {
 	tree r, i;
 
+	if (notify_temp_creation)
+	  return GS_OK;
+
 	/* Extract the real and imaginary parts out of the ctor.  */
 	gcc_assert (VEC_length (constructor_elt, elts) == 2);
 	r = VEC_index (constructor_elt, elts, 0)->value;
@@ -3348,6 +3375,9 @@ gimplify_init_constructor (tree *expr_p,
 	unsigned HOST_WIDE_INT ix;
 	constructor_elt *ce;
 
+	if (notify_temp_creation)
+	  return GS_OK;
+
 	/* Go ahead and simplify constant constructors to VECTOR_CST.  */
 	if (TREE_CONSTANT (ctor))
 	  {
@@ -3488,10 +3518,27 @@ gimplify_modify_expr_rhs (tree *expr_p, 
 	   constructor expression to the RHS of the MODIFY_EXPR.  */
 	if (DECL_INITIAL (*from_p)
 	    && TYPE_READONLY (TREE_TYPE (*from_p))
+	    && !TREE_THIS_VOLATILE (*from_p)
 	    && TREE_CODE (DECL_INITIAL (*from_p)) == CONSTRUCTOR)
 	  {
-	    *from_p = DECL_INITIAL (*from_p);
-	    ret = GS_OK;
+	    tree old_from = *from_p;
+
+	    /* Move the constructor into the RHS.  */
+	    *from_p = unshare_expr (DECL_INITIAL (*from_p));
+
+	    /* Let's see if gimplify_init_constructor will need to put
+	       it in memory.  If so, revert the change.  */
+	    ret = gimplify_init_constructor (expr_p, NULL, NULL, false, true);
+	    if (ret == GS_ERROR)
+	      {
+		*from_p = old_from;
+		/* Fall through.  */
+	      }
+	    else
+	      {
+		ret = GS_OK;
+		break;
+	      }
 	  }
 	ret = GS_UNHANDLED;
 	break;
@@ -3551,7 +3598,8 @@ gimplify_modify_expr_rhs (tree *expr_p, 
       case CONSTRUCTOR:
 	/* If we're initializing from a CONSTRUCTOR, break this into
 	   individual MODIFY_EXPRs.  */
-	return gimplify_init_constructor (expr_p, pre_p, post_p, want_value);
+	return gimplify_init_constructor (expr_p, pre_p, post_p, want_value,
+					  false);
 
       case COND_EXPR:
 	/* If we're assigning to a non-register type, push the assignment
--- testsuite/gcc.c-torture/compile/pr34448.c	(revision 130934)
+++ testsuite/gcc.c-torture/compile/pr34448.c	(local)
@@ -0,0 +1,34 @@
+/* { dg-do compile } */
+/* { dg-options "-O" } */
+
+typedef struct chunk_t chunk_t;
+struct chunk_t
+{
+  unsigned char *ptr;
+  long unsigned int len;
+};
+extern chunk_t asn1_wrap (chunk_t c, ...);
+typedef struct linked_list_t linked_list_t;
+chunk_t ietfAttr_list_encode (linked_list_t * list);
+extern linked_list_t *groups;
+static unsigned char ASN1_group_oid_str[] = {
+    0x06
+};
+static const chunk_t ASN1_group_oid = {
+  ASN1_group_oid_str, sizeof (ASN1_group_oid_str)
+};
+static chunk_t
+build_attribute_type (const chunk_t type, chunk_t content)
+{
+  return type;
+}
+static chunk_t
+build_attributes (void)
+{
+  return asn1_wrap (build_attribute_type (ASN1_group_oid,
+					  ietfAttr_list_encode (groups)));
+}
+void build_attr_cert (void)
+{
+  asn1_wrap (build_attributes ());
+}
--- testsuite/gcc.dg/tree-ssa/pr32901.c	(revision 130934)
+++ testsuite/gcc.dg/tree-ssa/pr32901.c	(local)
@@ -7,7 +7,7 @@ struct foo {
         unsigned : 4;
 };
 
-extern struct foo thefoo;
+extern struct foo thefoo, theotherfoo;
 
 void setup_foo(void)
 {
@@ -15,10 +15,16 @@ void setup_foo(void)
                 .a1 = 1,
                 .a2 = 5,
         };
+	volatile const struct foo volinit = {
+		.a1 = 0,
+		.a2 = 6
+	};
         thefoo = init;
+	theotherfoo = volinit;
 }
 
-/* { dg-final { scan-tree-dump-times "thefoo.0 = \{\}" 1 "gimple"} } */
-/* { dg-final { scan-tree-dump-times "thefoo.0.a1 = 1" 1 "gimple"} } */
-/* { dg-final { scan-tree-dump-times "thefoo.0.a2 = 5" 1 "gimple"} } */
+/* { dg-final { scan-tree-dump-times "thefoo.* = {}" 1 "gimple"} } */
+/* { dg-final { scan-tree-dump-times "thefoo.* = 1" 1 "gimple"} } */
+/* { dg-final { scan-tree-dump-times "thefoo.* = 5" 1 "gimple"} } */
+/* { dg-final { scan-tree-dump-times "theotherfoo = volinit" 1 "gimple"} } */
 /* { dg-final { cleanup-tree-dump "gimple" } } */

Re: [patch] PR32901: handle assignment...(and PR34465 and PR34448)

[Aldy Hernandez]

> The patch bootstraps.  I'm still running tests, but I'm fixing an ICE it
> has encountered, so I'm not ready to commit.  I wanted to get feedback

Tests are done, and the ICE I thought I had was there before my patch;
so there are no regressions.

The patch is done, and it fixes both 34448 and 34465.

Can someone please review this for mainline?

Thanks.
Aldy

Re: [patch] PR32901: handle assignment...(and PR34465 and PR34448)

[Jakub Jelinek]

On Fri, Dec 14, 2007 at 05:25:27PM -0400, Aldy Hernandez wrote:
> This patch addresses Mark's concern regarding volatiles.  It also fixes
> 34448 by not allowing the optimization to run if
> gimplify_init_constructor will dump the constructor to memory.  And
> finally, it fixes the tree sharing problem in PR34465.

It looks good to me (though you need to find someone^H^H^H^H^H^H^HDiego to
approve it anyway), except that the unshare_expr is IMHO done too early,
wasting GC memory if gimplify_init_constructor returns GS_ERROR.  I believe
gimplify_init_constructor doesn't care if RHS is shared or unshared
and with true as last argument it shouldn't gimplify it either, so
IMHO doing:
  *from_p = DECL_INITIAL (*from_p);
before the gimplify_init_constructor and
  *from_p = unshare_expr (*from_p);
right before ret = GS_OK after it would work too and wouldn't waste memory
if the optimization is not applied.

> -	    *from_p = DECL_INITIAL (*from_p);
> -	    ret = GS_OK;
> +	    tree old_from = *from_p;
> +
> +	    /* Move the constructor into the RHS.  */
> +	    *from_p = unshare_expr (DECL_INITIAL (*from_p));
> +
> +	    /* Let's see if gimplify_init_constructor will need to put
> +	       it in memory.  If so, revert the change.  */
> +	    ret = gimplify_init_constructor (expr_p, NULL, NULL, false, true);
> +	    if (ret == GS_ERROR)
> +	      {
> +		*from_p = old_from;
> +		/* Fall through.  */
> +	      }
> +	    else
> +	      {
> +		ret = GS_OK;
> +		break;
> +	      }
>  	  }
>  	ret = GS_UNHANDLED;
>  	break;

	Jakub

Re: [patch] PR32901: handle assignment...(and PR34465 and PR34448)

[Aldy Hernandez]

> It looks good to me (though you need to find someone^H^H^H^H^H^H^HDiego to
> approve it anyway), except that the unshare_expr is IMHO done too early,
> wasting GC memory if gimplify_init_constructor returns GS_ERROR.  I believe

Good catch!  Fixed.

Diego, how does the patch look?

Aldy

Re: [patch] PR32901: handle assignment...(and PR34465 and PR34448)

[Diego Novillo]

On 12/14/07 16:25, Aldy Hernandez wrote:

> Also... in a subsequent patch we should really look into merging
> consecutive initializations of aggregate fields-- sometime before
> expand.  Jakub has suggested TER.  I volunteer to do the work once I'm
> done with this.

Any reason why you couldn't do this directly in the gimplifier?  Instead 
of emitting the separate field assignments, just emit a single 
CONSTRUCTOR assignment.  You already have the DECL_INITIAL constructor 
after all.  I haven't thought it through, so the suggestion may be nonsense.


> @@ -3119,11 +3119,18 @@ gimplify_init_ctor_eval (tree object, VE
>  
>     Note that we still need to clear any elements that don't have explicit
>     initializers, so if not all elements are initialized we keep the
> -   original MODIFY_EXPR, we just remove all of the constructor elements.  */
> +   original MODIFY_EXPR, we just remove all of the constructor elements.
> +
> +   If NOTIFY_TEMP_CREATION is true, do not gimplify, just return
> +   GS_ERROR if we would have to create a temporary when gimplifying
> +   this constructor.  Otherwise, return GS_OK.
> +
> +   If NOTIFY_TEMP_CREATION is false, just do the gimplification.  */

Hmm, I don't much like this part.  This function is supposed to just 
gimplify the constructor.  The tests for NOTIFY_TEMP_CREATION are simple 
enough to be hoisted into a single predicate for 
gimplify_modify_expr_rhs to use.

Otherwise, this one function grows in complexity and it already is a bit 
convoluted.

> +	    tree old_from = *from_p;
> +
> +	    /* Move the constructor into the RHS.  */
> +	    *from_p = unshare_expr (DECL_INITIAL (*from_p));

As Jakub suggested, this unsharing is better done after the check.


OK with that refactoring.


Diego.

Re: [patch] PR32901: handle assignment...(and PR34465 and PR34448)

[Jakub Jelinek]

On Wed, Dec 19, 2007 at 06:27:34AM -0500, Diego Novillo wrote:
> On 12/14/07 16:25, Aldy Hernandez wrote:
> 
> >Also... in a subsequent patch we should really look into merging
> >consecutive initializations of aggregate fields-- sometime before
> >expand.  Jakub has suggested TER.  I volunteer to do the work once I'm
> >done with this.
> 
> Any reason why you couldn't do this directly in the gimplifier?  Instead 
> of emitting the separate field assignments, just emit a single 
> CONSTRUCTOR assignment.  You already have the DECL_INITIAL constructor 
> after all.  I haven't thought it through, so the suggestion may be nonsense.

ATM non-empty CONSTRUCTOR on RHS (unless it is a vector CONSTRUCTOR) is
considered invalid GIMPLE.  I guess there are good reasons for that,
otherwise many optimizations would need to be taught how to pick individual
elements from the constructor etc.  Furthermore, I guess we want to optimize
even cases where the CONSTRUCTOR wasn't there in user's code:

typedef struct __attribute__((aligned (4))) { unsigned char a, b, c, d; } T;
typedef struct { unsigned int a : 5, b : 5, c : 5, d : 5, e : 5, f : 5; } S;

static T u, v;
static S s, t;

void
foo (void)
{
  u = (T) { 1, 2, 3, 4};
  v.a = 1;
  v.b = 2;
  v.c = 3;
  v.d = 4;
}
 
void
bar (void)
{
  s = (S) { 1, 2, 3, 4, 5, 6 };
  t.a = 1;
  t.b = 2;
  t.c = 3;
  t.d = 4;
  t.e = 5;
  t.f = 6;
}

IMHO this should be optimized the same for u and v stores, particularly
just store 0x01020304 (resp. 0x04030201 depending on endianity) to u
and v, with the alias set of the whole T for the stores.  The bar assembly
is complete disaster.  For bar 3.4 the store to s is just:
        movl    $206703681, s(%rip)     #, s
while trunk needs 20 times more instructions to do the same.
The reason why I mentioned TER is that such an optimization would create
invalid GIMPLE (so should be done as late as possible), but to do it
properly we need SSA form, so we can walk the VDEFs to collect the
individual constructor elements.  When expand sees the whole constructor,
it can make better decisions with it, compared to what it can do when it
sees just individual stores.

For bitfields, even if there are just a few consecutive stores (or same
bitwise ops, e.g.
t.a = 1;
t.b = 2;
t.c = 3;
or
t.a |= 4;
t.b |= 4;
t.c |= 4;
from above example) it might be beneficial to aggregate them together, but
not sure how to represent that at the tree level.  But that can be done
separately.

> >@@ -3119,11 +3119,18 @@ gimplify_init_ctor_eval (tree object, VE
> > 
> >    Note that we still need to clear any elements that don't have explicit
> >    initializers, so if not all elements are initialized we keep the
> >-   original MODIFY_EXPR, we just remove all of the constructor elements.  
> >*/
> >+   original MODIFY_EXPR, we just remove all of the constructor elements.
> >+
> >+   If NOTIFY_TEMP_CREATION is true, do not gimplify, just return
> >+   GS_ERROR if we would have to create a temporary when gimplifying
> >+   this constructor.  Otherwise, return GS_OK.
> >+
> >+   If NOTIFY_TEMP_CREATION is false, just do the gimplification.  */
> 
> Hmm, I don't much like this part.  This function is supposed to just 
> gimplify the constructor.  The tests for NOTIFY_TEMP_CREATION are simple 
> enough to be hoisted into a single predicate for 
> gimplify_modify_expr_rhs to use.
> 
> Otherwise, this one function grows in complexity and it already is a bit 
> convoluted.

Could just a name change of the function help?  The thing is that
both the predicate and the actual gimplification of the CONSTRUCTOR needs
to compute a lot of stuff (categorize_ctor_elements itself sets 4 different
things, then num_type_elements, further cleared adjustments, size,
alignment), so if we have an predicate and gimplify_init_constructor as
separate functions, we'd either need to duplicate a lot of stuff, or e.g.
have some helper function which precomputes a lot of stuff into some temp
structure, from which the predicate and gimplify_init_constructor then use it.

	Jakub

Re: [patch] PR32901: handle assignment...(and PR34465 and PR34448)

[Aldy Hernandez]

> > Hmm, I don't much like this part.  This function is supposed to just 
> > gimplify the constructor.  The tests for NOTIFY_TEMP_CREATION are simple 
> > enough to be hoisted into a single predicate for 
> > gimplify_modify_expr_rhs to use.
> > 
> > Otherwise, this one function grows in complexity and it already is a bit 
> > convoluted.
> 
> Could just a name change of the function help?  The thing is that
> both the predicate and the actual gimplification of the CONSTRUCTOR needs
> to compute a lot of stuff (categorize_ctor_elements itself sets 4 different
> things, then num_type_elements, further cleared adjustments, size,
> alignment), so if we have an predicate and gimplify_init_constructor as
> separate functions, we'd either need to duplicate a lot of stuff, or e.g.
> have some helper function which precomputes a lot of stuff into some temp
> structure, from which the predicate and gimplify_init_constructor then use it.

Exactly, what Jakub said. :).

I originally tried to factor this code out into a separate function, but
the current solution seemed a lot cleaner.

Aldy

Re: [patch] PR32901: handle assignment...(and PR34465 and PR34448)

[Diego Novillo]

On 12/19/07 07:44, Jakub Jelinek wrote:

> ATM non-empty CONSTRUCTOR on RHS (unless it is a vector CONSTRUCTOR) is
> considered invalid GIMPLE.

I said CONSTRUCTOR, but I really meant BIT_FIELD_REF.  However, I see 
your point and, yes, this is something we should combine at a later 
stage.  On our way out of SSA and GIMPLE looks like a good place.

Now, how much optimization are we losing here in real life?  I doubt 
these initializers are often used in hot paths.  So, I don't think 
there's a lot of urgency in fixing this.  But we should open a PR (if 
one doesn't exist yet).


> have some helper function which precomputes a lot of stuff into some temp
> structure, from which the predicate and gimplify_init_constructor then use it.

Yes, that's what I was thinking about.  This would mean stripping all 
the code out of the 'case RECORD_TYPE' handler.  Since we are pretty 
late in  stage 3, I guess this would be more surgery than we want at 
this point.

Would you guys be willing to re-arrange this for 4.4?  I'd rather do it 
now, but I understand if you want to wait.  In the meantime I guess this 
is OK.


Thanks.  Diego.

Re: [patch] PR32901: handle assignment...(and PR34465 and PR34448)

[Jakub Jelinek]

On Thu, Dec 20, 2007 at 07:44:18AM -0500, Diego Novillo wrote:
> On 12/19/07 07:44, Jakub Jelinek wrote:
> 
> >ATM non-empty CONSTRUCTOR on RHS (unless it is a vector CONSTRUCTOR) is
> >considered invalid GIMPLE.
> 
> I said CONSTRUCTOR, but I really meant BIT_FIELD_REF.  However, I see 
> your point and, yes, this is something we should combine at a later 
> stage.  On our way out of SSA and GIMPLE looks like a good place.

Yeah, as BIT_FIELD_REF is a valid gimple, perhaps just adding some late pass
that would optimize adjacent similar bitfield operations into BIT_FIELD_REF
operations would be worthwhile.  Still combining initializers into
CONSTRUCTORs might be handy.

> Now, how much optimization are we losing here in real life?  I doubt 
> these initializers are often used in hot paths.  So, I don't think 
> there's a lot of urgency in fixing this.  But we should open a PR (if 
> one doesn't exist yet).

If not anything else, it can be sometimes really serious size regression.
Say on that
typedef struct { unsigned int a : 5, b : 5, c : 5, d : 5, e : 5, f : 7; } S;

static S s;

void
bar (void)
{
  S t = { 1, 2, 3, 4, 5, 6 };
  s = t;
}

we are talking about with -Os about 11 .text bytes in 3.4.x and 112 .text bytes
on the trunk.  There are many projects that use bitfields heavily (including
gcc) and the generated code is really abysmal.  Sometimes combiner is able
to reduce some bloat, but often it is not.
We have PR22141 for the non-bitfield initializer stores not being merged
together, I thought we have several PRs about horrible bitfield code, but
can't find them ATM.

	Jakub

Re: [patch] PR32901: handle assignment...(and PR34465 and PR34448)

[Aldy Hernandez]

>> have some helper function which precomputes a lot of stuff into some temp
>> structure, from which the predicate and gimplify_init_constructor then use it.
>
> Yes, that's what I was thinking about.  This would mean stripping all the 
> code out of the 'case RECORD_TYPE' handler.  Since we are pretty late in  
> stage 3, I guess this would be more surgery than we want at this point.
>
> Would you guys be willing to re-arrange this for 4.4?  I'd rather do it 
> now, but I understand if you want to wait.  In the meantime I guess this is 
> OK.

After all your off-list oppression and bantering, I cave.  Besides, I
really don't want to drag this into 4.4, because I'll forget and/or
cease to care.

I'm fixing it now.  If it turns out to be uglier than the present code,
I assume we can go with the the present approach.

But of course, everyone's going on holiday today, and I'll be eating
copious amounts of "lechon asado" over the next week.

Expect a patch sometime next week or early the next.

Aldy

Re: [patch] PR32901: handle assignment...(and PR34465 and PR34448)

[Diego Novillo]

On 12/21/07 10:01, Aldy Hernandez wrote:

> After all your off-list oppression and bantering, I cave.

LOL!  You lie like a rug, mon ami ;)

> I'm fixing it now.  If it turns out to be uglier than the present code,
> I assume we can go with the the present approach.

Sure.  If the alternative is uglier, then the patch you posted earlier 
this week is fine.

> But of course, everyone's going on holiday today, and I'll be eating
> copious amounts of "lechon asado" over the next week.

Excellent plan.

> Expect a patch sometime next week or early the next.

Thanks!


Diego.

Re: [patch] PR32901: handle assignment...(and PR34465 and PR34448)

[Aldy Hernandez]

> Sure.  If the alternative is uglier, then the patch you posted earlier this 
> week is fine.
>
>> But of course, everyone's going on holiday today, and I'll be eating
>> copious amounts of "lechon asado" over the next week.
>
> Excellent plan.
>
>> Expect a patch sometime next week or early the next.
>
> Thanks!

Hi Diego.  Hi folks.

I have a patch which is mostly done, but I've run into a few snafus
which have me wondering whether all this refactoring is worth the
trouble.

I am calculating all the ancillary data that both
gimplify_init_constructor and the new predicate calculating temporary
creation, will use.  It's being stored in a structure, as suggested,
but there are two problems.

First, we need to calculate the LHS of the initialization ("object" in
gimplify_init_constructor) in order to generate the rest of the info we
need.  To calculate this, we must gimplify the LHS, thus introducing
side-effects while generating the data.

Second, to determine whether we will create a temporary in the second
instance (see below), we need to calculate "size", which has the
side-effect of changing the type of ctor.

            if (size < 0)
              {
                size = int_size_in_bytes (TREE_TYPE (object));
                if (size >= 0)
                  TREE_TYPE (ctor) = type = TREE_TYPE (object);
  SIDE EFFECT HERE^^^^^^^^^^^^^^^^^^^^^
              }

            /* Find the maximum alignment we can assume for the object.  */
            /* ??? Make use of DECL_OFFSET_ALIGN.  */
            if (DECL_P (object))
              align = DECL_ALIGN (object);
            else
              align = TYPE_ALIGN (type);

            if (size > 0 && !can_move_by_pieces (size, align))
  WE NEED SIZE HERE^^^^^^^^^
              {
                tree new = create_tmp_var_raw (type, "C");

Can someone see a clean way of approaching this, or should we just go
back to my original patch?

Aldy

Re: [patch] PR32901: handle assignment...(and PR34465 and PR34448)

[Richard Guenther]

On Jan 3, 2008 10:35 AM, Aldy Hernandez <aldyh@redhat.com> wrote:
> > Sure.  If the alternative is uglier, then the patch you posted earlier this
> > week is fine.
> >
> >> But of course, everyone's going on holiday today, and I'll be eating
> >> copious amounts of "lechon asado" over the next week.
> >
> > Excellent plan.
> >
> >> Expect a patch sometime next week or early the next.
> >
> > Thanks!
>
> Hi Diego.  Hi folks.
>
> I have a patch which is mostly done, but I've run into a few snafus
> which have me wondering whether all this refactoring is worth the
> trouble.
>
> I am calculating all the ancillary data that both
> gimplify_init_constructor and the new predicate calculating temporary
> creation, will use.  It's being stored in a structure, as suggested,
> but there are two problems.
>
> First, we need to calculate the LHS of the initialization ("object" in
> gimplify_init_constructor) in order to generate the rest of the info we
> need.  To calculate this, we must gimplify the LHS, thus introducing
> side-effects while generating the data.
>
> Second, to determine whether we will create a temporary in the second
> instance (see below), we need to calculate "size", which has the
> side-effect of changing the type of ctor.
>
>             if (size < 0)
>               {
>                 size = int_size_in_bytes (TREE_TYPE (object));
>                 if (size >= 0)
>                   TREE_TYPE (ctor) = type = TREE_TYPE (object);
>   SIDE EFFECT HERE^^^^^^^^^^^^^^^^^^^^^
>               }
>
>             /* Find the maximum alignment we can assume for the object.  */
>             /* ??? Make use of DECL_OFFSET_ALIGN.  */
>             if (DECL_P (object))
>               align = DECL_ALIGN (object);
>             else
>               align = TYPE_ALIGN (type);
>
>             if (size > 0 && !can_move_by_pieces (size, align))
>   WE NEED SIZE HERE^^^^^^^^^
>               {
>                 tree new = create_tmp_var_raw (type, "C");
>
> Can someone see a clean way of approaching this, or should we just go
> back to my original patch?

I appreciate the efforts to fix all this, but at some point we might consider
reverting the patch that caused this fallout, which is I believe the fix
for PR32901.

Richard.

Re: [patch] PR32901: handle assignment...(and PR34465 and PR34448)

[Aldy Hernandez]

> I appreciate the efforts to fix all this, but at some point we might consider
> reverting the patch that caused this fallout, which is I believe the fix
> for PR32901.

Tell you what... I'm going to commit the patch below which fixes all the
problems, and was approved by Diego.  If Diego still wants the
refactoring, I am willing to iterate with him.

Committed to mainline.

Aldy

	PR tree-optimization/34448
	PR tree-optimization/34465
	* gimplify.c (gimplify_init_constructor): Add new parameter
	notify_temp_creation.  Use it.
	(gimplify_modify_expr_rhs): Take volatiles into account when
	optimizing constructors.
	Do not optimize constructors if gimplify_init_constructor will dump to
	memory.
	* gcc.dg/tree-ssa/pr32901.c: Tests const volatiles.
	* gcc.c-torture/compile/pr34448.c: New.

--- gimplify.c	(revision 130934)
+++ gimplify.c	(local)
@@ -3119,11 +3119,18 @@ gimplify_init_ctor_eval (tree object, VE
 
    Note that we still need to clear any elements that don't have explicit
    initializers, so if not all elements are initialized we keep the
-   original MODIFY_EXPR, we just remove all of the constructor elements.  */
+   original MODIFY_EXPR, we just remove all of the constructor elements.
+
+   If NOTIFY_TEMP_CREATION is true, do not gimplify, just return
+   GS_ERROR if we would have to create a temporary when gimplifying
+   this constructor.  Otherwise, return GS_OK.
+
+   If NOTIFY_TEMP_CREATION is false, just do the gimplification.  */
 
 static enum gimplify_status
 gimplify_init_constructor (tree *expr_p, tree *pre_p,
-			   tree *post_p, bool want_value)
+			   tree *post_p, bool want_value,
+			   bool notify_temp_creation)
 {
   tree object;
   tree ctor = GENERIC_TREE_OPERAND (*expr_p, 1);
@@ -3134,10 +3141,13 @@ gimplify_init_constructor (tree *expr_p,
   if (TREE_CODE (ctor) != CONSTRUCTOR)
     return GS_UNHANDLED;
 
-  ret = gimplify_expr (&GENERIC_TREE_OPERAND (*expr_p, 0), pre_p, post_p,
-		       is_gimple_lvalue, fb_lvalue);
-  if (ret == GS_ERROR)
-    return ret;
+  if (!notify_temp_creation)
+    {
+      ret = gimplify_expr (&GENERIC_TREE_OPERAND (*expr_p, 0), pre_p, post_p,
+			   is_gimple_lvalue, fb_lvalue);
+      if (ret == GS_ERROR)
+	return ret;
+    }
   object = GENERIC_TREE_OPERAND (*expr_p, 0);
 
   elts = CONSTRUCTOR_ELTS (ctor);
@@ -3159,7 +3169,11 @@ gimplify_init_constructor (tree *expr_p,
 	   individual elements.  The exception is that a CONSTRUCTOR node
 	   with no elements indicates zero-initialization of the whole.  */
 	if (VEC_empty (constructor_elt, elts))
-	  break;
+	  {
+	    if (notify_temp_creation)
+	      return GS_OK;
+	    break;
+	  }
 
 	/* Fetch information about the constructor to direct later processing.
 	   We might want to make static versions of it in various cases, and
@@ -3175,6 +3189,8 @@ gimplify_init_constructor (tree *expr_p,
 	    && TREE_READONLY (object)
 	    && TREE_CODE (object) == VAR_DECL)
 	  {
+	    if (notify_temp_creation)
+	      return GS_ERROR;
 	    DECL_INITIAL (object) = ctor;
 	    TREE_STATIC (object) = 1;
 	    if (!DECL_NAME (object))
@@ -3251,7 +3267,12 @@ gimplify_init_constructor (tree *expr_p,
 
 	    if (size > 0 && !can_move_by_pieces (size, align))
 	      {
-		tree new = create_tmp_var_raw (type, "C");
+		tree new;
+
+		if (notify_temp_creation)
+		  return GS_ERROR;
+
+		new = create_tmp_var_raw (type, "C");
 
 		gimple_add_tmp_var (new);
 		TREE_STATIC (new) = 1;
@@ -3273,6 +3294,9 @@ gimplify_init_constructor (tree *expr_p,
 	      }
 	  }
 
+	if (notify_temp_creation)
+	  return GS_OK;
+
 	/* If there are nonzero elements, pre-evaluate to capture elements
 	   overlapping with the lhs into temporaries.  We must do this before
 	   clearing to fetch the values before they are zeroed-out.  */
@@ -3312,6 +3336,9 @@ gimplify_init_constructor (tree *expr_p,
       {
 	tree r, i;
 
+	if (notify_temp_creation)
+	  return GS_OK;
+
 	/* Extract the real and imaginary parts out of the ctor.  */
 	gcc_assert (VEC_length (constructor_elt, elts) == 2);
 	r = VEC_index (constructor_elt, elts, 0)->value;
@@ -3348,6 +3375,9 @@ gimplify_init_constructor (tree *expr_p,
 	unsigned HOST_WIDE_INT ix;
 	constructor_elt *ce;
 
+	if (notify_temp_creation)
+	  return GS_OK;
+
 	/* Go ahead and simplify constant constructors to VECTOR_CST.  */
 	if (TREE_CONSTANT (ctor))
 	  {
@@ -3488,10 +3518,27 @@ gimplify_modify_expr_rhs (tree *expr_p, 
 	   constructor expression to the RHS of the MODIFY_EXPR.  */
 	if (DECL_INITIAL (*from_p)
 	    && TYPE_READONLY (TREE_TYPE (*from_p))
+	    && !TREE_THIS_VOLATILE (*from_p)
 	    && TREE_CODE (DECL_INITIAL (*from_p)) == CONSTRUCTOR)
 	  {
-	    *from_p = DECL_INITIAL (*from_p);
-	    ret = GS_OK;
+	    tree old_from = *from_p;
+
+	    /* Move the constructor into the RHS.  */
+	    *from_p = unshare_expr (DECL_INITIAL (*from_p));
+
+	    /* Let's see if gimplify_init_constructor will need to put
+	       it in memory.  If so, revert the change.  */
+	    ret = gimplify_init_constructor (expr_p, NULL, NULL, false, true);
+	    if (ret == GS_ERROR)
+	      {
+		*from_p = old_from;
+		/* Fall through.  */
+	      }
+	    else
+	      {
+		ret = GS_OK;
+		break;
+	      }
 	  }
 	ret = GS_UNHANDLED;
 	break;
@@ -3551,7 +3598,8 @@ gimplify_modify_expr_rhs (tree *expr_p, 
       case CONSTRUCTOR:
 	/* If we're initializing from a CONSTRUCTOR, break this into
 	   individual MODIFY_EXPRs.  */
-	return gimplify_init_constructor (expr_p, pre_p, post_p, want_value);
+	return gimplify_init_constructor (expr_p, pre_p, post_p, want_value,
+					  false);
 
       case COND_EXPR:
 	/* If we're assigning to a non-register type, push the assignment
--- testsuite/gcc.c-torture/compile/pr34448.c	(revision 130934)
+++ testsuite/gcc.c-torture/compile/pr34448.c	(local)
@@ -0,0 +1,34 @@
+/* { dg-do compile } */
+/* { dg-options "-O" } */
+
+typedef struct chunk_t chunk_t;
+struct chunk_t
+{
+  unsigned char *ptr;
+  long unsigned int len;
+};
+extern chunk_t asn1_wrap (chunk_t c, ...);
+typedef struct linked_list_t linked_list_t;
+chunk_t ietfAttr_list_encode (linked_list_t * list);
+extern linked_list_t *groups;
+static unsigned char ASN1_group_oid_str[] = {
+    0x06
+};
+static const chunk_t ASN1_group_oid = {
+  ASN1_group_oid_str, sizeof (ASN1_group_oid_str)
+};
+static chunk_t
+build_attribute_type (const chunk_t type, chunk_t content)
+{
+  return type;
+}
+static chunk_t
+build_attributes (void)
+{
+  return asn1_wrap (build_attribute_type (ASN1_group_oid,
+					  ietfAttr_list_encode (groups)));
+}
+void build_attr_cert (void)
+{
+  asn1_wrap (build_attributes ());
+}
--- testsuite/gcc.dg/tree-ssa/pr32901.c	(revision 130934)
+++ testsuite/gcc.dg/tree-ssa/pr32901.c	(local)
@@ -7,7 +7,7 @@ struct foo {
         unsigned : 4;
 };
 
-extern struct foo thefoo;
+extern struct foo thefoo, theotherfoo;
 
 void setup_foo(void)
 {
@@ -15,10 +15,16 @@ void setup_foo(void)
                 .a1 = 1,
                 .a2 = 5,
         };
+	volatile const struct foo volinit = {
+		.a1 = 0,
+		.a2 = 6
+	};
         thefoo = init;
+	theotherfoo = volinit;
 }
 
-/* { dg-final { scan-tree-dump-times "thefoo.0 = \{\}" 1 "gimple"} } */
-/* { dg-final { scan-tree-dump-times "thefoo.0.a1 = 1" 1 "gimple"} } */
-/* { dg-final { scan-tree-dump-times "thefoo.0.a2 = 5" 1 "gimple"} } */
+/* { dg-final { scan-tree-dump-times "thefoo.* = {}" 1 "gimple"} } */
+/* { dg-final { scan-tree-dump-times "thefoo.* = 1" 1 "gimple"} } */
+/* { dg-final { scan-tree-dump-times "thefoo.* = 5" 1 "gimple"} } */
+/* { dg-final { scan-tree-dump-times "theotherfoo = volinit" 1 "gimple"} } */
 /* { dg-final { cleanup-tree-dump "gimple" } } */

Re: [patch] PR32901: handle assignment...(and PR34465 and PR34448)

[Diego Novillo]

On 1/4/08 9:52 AM, Aldy Hernandez wrote:

> Tell you what... I'm going to commit the patch below which fixes all the
> problems, and was approved by Diego.  If Diego still wants the
> refactoring, I am willing to iterate with him.

Sounds good to me.  Thanks for working on this.


Diego.