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 <ja...@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 <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
