On Tue, 5 Nov 2019, Jakub Jelinek wrote:
> On Tue, Nov 05, 2019 at 09:27:45AM +0100, Richard Biener wrote:
> > > --- gcc/gimplify.c.jj 2019-11-02 10:00:59.595253274 +0100
> > > +++ gcc/gimplify.c 2019-11-05 00:21:01.585958514 +0100
> > > @@ -6235,8 +6235,14 @@ gimplify_asm_expr (tree *expr_p, gimple_
> > > is_inout = false;
> > > }
> > >
> > > - /* If we can't make copies, we can only accept memory. */
> > > - if (TREE_ADDRESSABLE (TREE_TYPE (TREE_VALUE (link))))
> > > + /* If we can't make copies, we can only accept memory.
> > > + Similarly for VLAs. */
> > > + tree outtype = TREE_TYPE (TREE_VALUE (link));
> > > + if (outtype != error_mark_node
> >
> > so for error_mark_node we don't diagnose anything?
>
> Yes, we should have diagnosed it already. The != error_mark_node
> I've added only after seeing tons of ICEs in the testsuite with earlier
> version of the patch.
>
> > > + && (TREE_ADDRESSABLE (outtype)
> > > + || !COMPLETE_TYPE_P (outtype)
> > > + || (!tree_fits_poly_uint64_p (TYPE_SIZE_UNIT (outtype))
> > > + && max_int_size_in_bytes (outtype))))
> >
> > so max_int_size_in_bytes == 0 is OK? I suppose we have a testcase
> > for this?
>
> Actually, I meant max_int_size_in_bytes (outtype) < 0, i.e. something
> on which force_constant_size ICE immediately, sorry for screwing it up in
> the end.
> All these VLAs with max_int_size_in_bytes >= 0 sizes are specific to Ada
> and I have no idea what is and isn't valid there, for C/C++ it should
> always return -1.
>
> > Otherwise looks reasonable to me.
>
> So, is the following ok if it passes bootstrap/regtest, or shall
> I just go for || !tree_fits_poly_uint64_p (TYPE_SIZE_UNIT (outtype))) ?
Hmm, can we actually copy VLAs with max_int_size_in_bytes != -1? I
suppose we can copy even unconstrained VLAs, we just miss a
WITH_SIZE_EXPR here but then it's unlikely we ever fit that into
a non-memory. That's true as well if we just know the max size,
so allowing that seems suspicious at best.
So I'd say OK with just !tree_fits_poly_uint64_p.
Thanks,
Richard.
> 2019-11-05 Jakub Jelinek <ja...@redhat.com>
>
> PR inline-asm/92352
> * gimplify.c (gimplify_asm_expr): Reject VLA in output or input
> operands with non-memory constraints.
>
> * c-c++-common/pr92352.c: New test.
>
> --- gcc/gimplify.c.jj 2019-11-02 10:00:59.595253274 +0100
> +++ gcc/gimplify.c 2019-11-05 00:21:01.585958514 +0100
> @@ -6235,8 +6235,14 @@ gimplify_asm_expr (tree *expr_p, gimple_
> is_inout = false;
> }
>
> - /* If we can't make copies, we can only accept memory. */
> - if (TREE_ADDRESSABLE (TREE_TYPE (TREE_VALUE (link))))
> + /* If we can't make copies, we can only accept memory.
> + Similarly for VLAs. */
> + tree outtype = TREE_TYPE (TREE_VALUE (link));
> + if (outtype != error_mark_node
> + && (TREE_ADDRESSABLE (outtype)
> + || !COMPLETE_TYPE_P (outtype)
> + || (!tree_fits_poly_uint64_p (TYPE_SIZE_UNIT (outtype))
> + && max_int_size_in_bytes (outtype) < 0)))
> {
> if (allows_mem)
> allows_reg = 0;
> @@ -6392,7 +6398,12 @@ gimplify_asm_expr (tree *expr_p, gimple_
> oconstraints, &allows_mem, &allows_reg);
>
> /* If we can't make copies, we can only accept memory. */
> - if (TREE_ADDRESSABLE (TREE_TYPE (TREE_VALUE (link))))
> + tree intype = TREE_TYPE (TREE_VALUE (link));
> + if (intype != error_mark_node
> + && (TREE_ADDRESSABLE (intype)
> + || !COMPLETE_TYPE_P (intype)
> + || (!tree_fits_poly_uint64_p (TYPE_SIZE_UNIT (intype))
> + && max_int_size_in_bytes (intype) < 0)))
> {
> if (allows_mem)
> allows_reg = 0;
> --- gcc/testsuite/c-c++-common/pr92352.c.jj 2019-11-04 14:03:18.725275255
> +0100
> +++ gcc/testsuite/c-c++-common/pr92352.c 2019-11-04 14:02:55.211629675
> +0100
> @@ -0,0 +1,15 @@
> +/* PR inline-asm/92352 */
> +
> +void
> +foo (int x)
> +{
> + int var[x];
> + asm volatile ("" : "+r" (var)); /* { dg-error "impossible constraint in
> 'asm'" } */
> +} /* { dg-error "non-memory output 0 must
> stay in memory" "" { target *-*-* } .-1 } */
> +
> +void
> +bar (int x)
> +{
> + int var[x];
> + asm volatile ("" : "+m" (var));
> +}
>
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
