Re: [PATCH] Clear TREE_STATIC on vector ctors if needed (PR middle-end/70633)

[Richard Biener <rguenther at suse dot de>]

On Tue, 12 Apr 2016, Jakub Jelinek wrote:

> Hi!
> 
> As the testcase shows, gimplification of the VECTOR_TYPE CONSTRUCTOR
> elements can turn previously valid initializer_constant_valid_p
> into a temporary (thus no longer initializer_constant_valid_p).
> 
> The following patch just clears TREE_STATIC on the ctor if that happens,
> so that we expand it properly.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> Or should we instead check initializer_constant_valid_p first and not
> gimplify the initializer in that case if the ctor is TREE_STATIC?
> I'm just afraid that the optimization passes would be upset to see
> more complex elements in the initializers, and it is something people don't
> really write in real-world anyway.

I think GIMPLE doesn't really expect TREE_STATIC constructors in the IL
but if expansion is better in this case we can keep it as with your
patch (nothing in GIMPLE optimizations should make a previously
valid TREE_STATIC constructor invalid ...).

In an ideal world all "static" constructors would be forced to
CONST_DECLs (and we would maintain an IPA constant pool in the
cgraph code).

But I wonder why expansion doesn't simply "re-compute" TREE_STATIC
here.  That is, do we rely on TREE_STATIC constructors for correctness
purposes as set by frontends?  Or is this a "optimization" that is now
(historically) done very too much early?  For example 
recompute_constructor_flags doesn't re-compute TREE_STATIC - what
does set it in the first place?  verify_constructor_flags also doesn't
verify it.

Looks like the C/C++ FEs do that.

Patch is ok.

Thanks,
Richard.

> 2016-04-12  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/70633
> 	* gimplify.c (gimplify_init_constructor): Clear TREE_STATIC if
> 	gimplification turns some element into non-constant.
> 
> 	* gcc.c-torture/compile/pr70633.c: New test.
> 
> --- gcc/gimplify.c.jj	2016-04-09 13:21:09.000000000 +0200
> +++ gcc/gimplify.c	2016-04-12 16:50:17.271533881 +0200
> @@ -4164,7 +4164,7 @@ gimplify_init_constructor (tree *expr_p,
>  	  }
>  
>  	/* Vector types use CONSTRUCTOR all the way through gimple
> -	  compilation as a general initializer.  */
> +	   compilation as a general initializer.  */
>  	FOR_EACH_VEC_SAFE_ELT (elts, ix, ce)
>  	  {
>  	    enum gimplify_status tret;
> @@ -4172,6 +4172,10 @@ gimplify_init_constructor (tree *expr_p,
>  				  fb_rvalue);
>  	    if (tret == GS_ERROR)
>  	      ret = GS_ERROR;
> +	    else if (TREE_STATIC (ctor)
> +		     && !initializer_constant_valid_p (ce->value,
> +						       TREE_TYPE (ce->value)))
> +	      TREE_STATIC (ctor) = 0;
>  	  }
>  	if (!is_gimple_reg (TREE_OPERAND (*expr_p, 0)))
>  	  TREE_OPERAND (*expr_p, 1) = get_formal_tmp_var (ctor, pre_p);
> --- gcc/testsuite/gcc.c-torture/compile/pr70633.c.jj	2016-04-12 17:24:40.494491310 +0200
> +++ gcc/testsuite/gcc.c-torture/compile/pr70633.c	2016-04-12 17:24:23.000000000 +0200
> @@ -0,0 +1,12 @@
> +/* PR middle-end/70633 */
> +
> +typedef long V __attribute__((vector_size (4 * sizeof (long))));
> +
> +void foo (V *);
> +
> +void
> +bar (void)
> +{ 
> +  V b = { (long) bar, 0, 0, 0 };
> +  foo (&b);
> +}
> 
> 	Jakub
> 
> 

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