Am Montag, den 16.08.2021, 00:30 -0400 schrieb Jason Merrill: > On 8/1/21 1:36 PM, Uecker, Martin wrote: > > > > Here is an attempt to fix some old and annoying bugs related > > to VLAs and statement expressions. In particulary, this seems > > to fix the issues with variably-modified types which are > > returned from statement expressions (which works on clang), > > but there are still bugs remaining related to structs > > with VLA members (which seems to be a FE bug). > > > > Of course, I might be doing something stupid... > > > > The patch survives bootstrapping and regresstion testing > > on x86_64. > > Including Ada?
It broke PLACEHOLDER_EXPRs as you pointed out below. Please take a look at the new version: https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577402.html Martin > > Fix ICE when mixing VLAs and statement expressions [PR91038] > > > > When returning VM-types from statement expressions, this can > > lead to an ICE when declarations from the statement expression > > are referred to later. Some of these issues can be addressed by > > gimplifying the base expression earlier in gimplify_compound_lval. > > This fixes PR91038 and some of the test cases from PR29970 > > (structs with VLA members need further work). > > > > > > 2021-08-01 Martin Uecker <muec...@gwdg.de> > > > > gcc/ > > PR c/91038 > > PR c/29970 > > * gimplify.c (gimplify_var_or_parm_decl): Update comment. > > (gimplify_compound_lval): Gimplify base expression first. > > > > gcc/testsuite/ > > PR c/91038 > > PR c/29970 > > * gcc.dg/vla-stexp-01.c: New test. > > * gcc.dg/vla-stexp-02.c: New test. > > * gcc.dg/vla-stexp-03.c: New test. > > * gcc.dg/vla-stexp-04.c: New test. > > > > > > diff --git a/gcc/gimplify.c b/gcc/gimplify.c > > index 21ff32ee4aa..885d5f73585 100644 > > --- a/gcc/gimplify.c > > +++ b/gcc/gimplify.c > > @@ -2839,7 +2839,10 @@ gimplify_var_or_parm_decl (tree *expr_p) > > declaration, for which we've already issued an error. It would > > be really nice if the front end wouldn't leak these at all. > > Currently the only known culprit is C++ destructors, as seen > > - in g++.old-deja/g++.jason/binding.C. */ > > + in g++.old-deja/g++.jason/binding.C. > > + Another culpit are size expressions for variably modified > > + types which are lost in the FE or not gimplified correctly. > > + */ > > if (VAR_P (decl) > > && !DECL_SEEN_IN_BIND_EXPR_P (decl) > > && !TREE_STATIC (decl) && !DECL_EXTERNAL (decl) > > @@ -2984,9 +2987,23 @@ gimplify_compound_lval (tree *expr_p, gimple_seq > > *pre_p, gimple_seq > > *post_p, > > expression until we deal with any variable bounds, sizes, or > > positions in order to deal with PLACEHOLDER_EXPRs. > > > > - So we do this in three steps. First we deal with the annotations > > - for any variables in the components, then we gimplify the base, > > - then we gimplify any indices, from left to right. */ > > + So we do this in three steps. First we gimplify the base, > > + then we deal with the annotations for any variables in the > > + components, then we gimplify any indices, from left to right. > > + > > + The base expression may contain a statement expression that > > + has declarations used in size expressions, so has to be > > + gimplified first. */ > > The previous paragraph says, > > > But we can't gimplify the > > inner > > expression until we deal with any variable bounds, sizes, > > or > > positions in order to deal with PLACEHOLDER_EXPRs. > > so I would expect your change to break examples that the current code > was designed to handle. The change to delay gimplifying the inner > expression was in r0-59131 (SVN r83474), by Richard Kenner. But there > aren't any testcases in that commit. Richard, any insight? Can you > review this patch? > > + /* Step 1 is to gimplify the base expression. Make sure lvalue is set > > + so as to match the min_lval predicate. Failure to do so may result > > + in the creation of large aggregate temporaries. */ > > + tret = gimplify_expr (p, pre_p, post_p, is_gimple_min_lval, > > + fallback | fb_lvalue); > > + > > + ret = MIN (ret, tret); > > + > > + > > for (i = expr_stack.length () - 1; i >= 0; i--) > > { > > tree t = expr_stack[i]; > > @@ -3076,12 +3093,6 @@ gimplify_compound_lval (tree *expr_p, gimple_seq > > *pre_p, gimple_seq > > *post_p, > > } > > } > > > > - /* Step 2 is to gimplify the base expression. Make sure lvalue is set > > - so as to match the min_lval predicate. Failure to do so may result > > - in the creation of large aggregate temporaries. */ > > - tret = gimplify_expr (p, pre_p, post_p, is_gimple_min_lval, > > - fallback | fb_lvalue); > > - ret = MIN (ret, tret); > > > > /* And finally, the indices and operands of ARRAY_REF. During this > > loop we also remove any useless conversions. */ > > diff --git a/gcc/testsuite/gcc.dg/vla-stexpr-01.c > > b/gcc/testsuite/gcc.dg/vla-stexpr-01.c > > new file mode 100644 > > index 00000000000..27e1817eb63 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/vla-stexpr-01.c > > @@ -0,0 +1,94 @@ > > +/* PR29970, PR91038 */ > > +/* { dg-do run } */ > > +/* { dg-options "-O0" } */ > > + > > +int foo3b(void) // should not return 0 > > +{ > > + int n = 0; > > + return sizeof *({ n = 10; int x[n]; x; &x; }); > > +} > > + > > +int foo4(void) // should not ICE > > +{ > > + return (*({ > > + int n = 20; > > + char (*x)[n][n] = __builtin_malloc(n * n); > > + (*x)[12][1] = 1; > > + x; > > + }))[12][1]; > > +} > > + > > +int foo5(void) // should return 1, returns 0 > > +{ > > + int n = 0; > > + return (*({ > > + n = 20; > > + char (*x)[n][n] = __builtin_malloc(n * n); > > + (*x)[12][1] = 1; > > + (*x)[0][1] = 0; > > + x; > > + }))[12][1]; > > +} > > + > > +int foo5c(void) // should return 400 > > +{ > > + int n = 0; > > + return sizeof(*({ > > + n = 20; > > + char (*x)[n][n] = __builtin_malloc(n * n); > > + (*x)[12][1] = 1; > > + (*x)[0][1] = 0; > > + x; > > + })); > > +} > > + > > +int foo5b(void) // should return 1, returns 0 > > +{ > > + int n = 0; > > + return (*({ > > + int n = 20; > > + char (*x)[n][n] = __builtin_malloc(n * n); > > + (*x)[12][1] = 1; > > + (*x)[0][1] = 0; > > + x; > > + }))[12][1]; > > +} > > + > > +int foo5a(void) // should return 1, returns 0 > > +{ > > + return (*({ > > + int n = 20; > > + char (*x)[n][n] = __builtin_malloc(n * n); > > + (*x)[12][1] = 1; > > + (*x)[0][1] = 0; > > + x; > > + }))[12][1]; > > +} > > + > > + > > + > > + > > +int main() > > +{ > > + if (sizeof(int[10]) != foo3b()) > > + __builtin_abort(); > > + > > + if (1 != foo4()) > > + __builtin_abort(); > > + > > + if (400 != foo5c()) > > + __builtin_abort(); > > + > > + if (1 != foo5a()) > > + __builtin_abort(); > > + > > + if (1 != foo5b()) // -O0 > > + __builtin_abort(); > > + > > + if (1 != foo5()) > > + __builtin_abort(); > > + > > + return 0; > > +} > > + > > + > > diff --git a/gcc/testsuite/gcc.dg/vla-stexpr-02.c > > b/gcc/testsuite/gcc.dg/vla-stexpr-02.c > > new file mode 100644 > > index 00000000000..99b4f571e52 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/vla-stexpr-02.c > > @@ -0,0 +1,31 @@ > > +/* PR29970 */ > > +/* { dg-do run } */ > > +/* { dg-options "" } */ > > + > > + > > + > > + > > +int foo2a(void) // should not ICE > > +{ > > + return ({ int n = 20; struct { int x[n];} x; x.x[12] = 1; > > sizeof(x); }); > > +} > > + > > +#if 0 > > +int foo2b(void) // should not ICE > > +{ > > + return sizeof *({ int n = 20; struct { int x[n];} x; x.x[12] = 1; > > &x; }); > > +} > > +#endif > > + > > +int main() > > +{ > > + if (sizeof(struct { int x[20]; }) != foo2a()) > > + __builtin_abort(); > > +#if 0 > > + if (sizeof(struct { int x[20]; }) != foo2b()) > > + __builtin_abort(); > > +#endif > > + return 0; > > +} > > + > > + > > diff --git a/gcc/testsuite/gcc.dg/vla-stexpr-03.c > > b/gcc/testsuite/gcc.dg/vla-stexpr-03.c > > new file mode 100644 > > index 00000000000..61f8986eaa3 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/vla-stexpr-03.c > > @@ -0,0 +1,94 @@ > > +/* PR29970, PR91038 */ > > +/* { dg-do run } */ > > +/* { dg-options "-O2" } */ > > + > > +int foo3b(void) // should not return 0 > > +{ > > + int n = 0; > > + return sizeof *({ n = 10; int x[n]; x; &x; }); > > +} > > + > > +int foo4(void) // should not ICE > > +{ > > + return (*({ > > + int n = 20; > > + char (*x)[n][n] = __builtin_malloc(n * n); > > + (*x)[12][1] = 1; > > + x; > > + }))[12][1]; > > +} > > + > > +int foo5(void) // should return 1, returns 0 > > +{ > > + int n = 0; > > + return (*({ > > + n = 20; > > + char (*x)[n][n] = __builtin_malloc(n * n); > > + (*x)[12][1] = 1; > > + (*x)[0][1] = 0; > > + x; > > + }))[12][1]; > > +} > > + > > +int foo5c(void) // should return 400 > > +{ > > + int n = 0; > > + return sizeof(*({ > > + n = 20; > > + char (*x)[n][n] = __builtin_malloc(n * n); > > + (*x)[12][1] = 1; > > + (*x)[0][1] = 0; > > + x; > > + })); > > +} > > + > > +int foo5b(void) // should return 1, returns 0 > > +{ > > + int n = 0; > > + return (*({ > > + int n = 20; > > + char (*x)[n][n] = __builtin_malloc(n * n); > > + (*x)[12][1] = 1; > > + (*x)[0][1] = 0; > > + x; > > + }))[12][1]; > > +} > > + > > +int foo5a(void) // should return 1, returns 0 > > +{ > > + return (*({ > > + int n = 20; > > + char (*x)[n][n] = __builtin_malloc(n * n); > > + (*x)[12][1] = 1; > > + (*x)[0][1] = 0; > > + x; > > + }))[12][1]; > > +} > > + > > + > > + > > + > > +int main() > > +{ > > + if (sizeof(int[10]) != foo3b()) > > + __builtin_abort(); > > + > > + if (1 != foo4()) > > + __builtin_abort(); > > + > > + if (400 != foo5c()) > > + __builtin_abort(); > > + > > + if (1 != foo5a()) > > + __builtin_abort(); > > + > > + if (1 != foo5b()) // -O0 > > + __builtin_abort(); > > + > > + if (1 != foo5()) > > + __builtin_abort(); > > + > > + return 0; > > +} > > + > > + > > diff --git a/gcc/testsuite/gcc.dg/vla-stexpr-04.c > > b/gcc/testsuite/gcc.dg/vla-stexpr-04.c > > new file mode 100644 > > index 00000000000..ffc8b12fa9b > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/vla-stexpr-04.c > > @@ -0,0 +1,38 @@ > > +/* PR91038 */ > > +/* { dg-do run } */ > > +/* { dg-options "" } */ > > + > > + > > +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) > > + > > +struct lbm { > > + > > + int D; > > + const int* DQ; > > + > > +} D2Q9 = { 2, > > + (const int*)&(const int[9][2]){ > > + { 0, 0 }, > > + { 1, 0 }, { 0, 1 }, { -1, 0 }, { 0, -1 }, > > + { 1, 1 }, { -1, 1 }, { -1, -1 }, { 1, -1 }, > > + } > > +}; > > + > > +void zouhe_left(void) > > +{ > > + __auto_type xx = (*({ int N = 2; struct lbm __x = D2Q9; ((const > > int(*)[9][N])__x.DQ); })); > > + > > + if (1 != xx[1][0]) > > + __builtin_abort(); > > + > > + if (2 != ARRAY_SIZE(xx[1])) > > + __builtin_abort(); > > +} > > + > > +int main() > > +{ > > + zouhe_left(); > > + return 0; > > +} > > + > > + > >
