Message ID | 8b3a94a09d35d51da2b0330ecff543d6dbcbe029.camel@med.uni-goettingen.de |
---|---|
State | New |
Headers | show |
Series | [v4] Fix ICE when mixing VLAs and statement expressions [PR91038] | expand |
On 10/17/21 09:52, Uecker, Martin wrote: > > > Here is the 4th version of the patch. I tried to implement > Jason's suggestion and this also fixes the problem. But > I am not sure I understand the condition on > the TREE_SIDE_EFFECTS ... Checking TREE_SIDE_EFFECTS filters out many trivial cases that we don't need to worry about. I think we also want to check variably_modified_type_p, which ought to avoid the OMP problem below. > And there is now another problem: > > c_finish_omp_for in c-family/c-omp.c does not seem > to understand the expressions anymore and I get a test > failure in > > testsuite/c-c++-common/gomp/for-5.c > > where I now get an "invalid increment expression" > instead of the expected error. > (bootstrapping and all other tests work fine) > > > 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. Most of these issues can be addressed by > gimplifying the base expression earlier in gimplify_compound_lval. > > Another issue is fixed by adding SAVE_EXPRs in pointer_int_sum > in the FE to force a correct order of evaluation. This fixes > PR91038 and some of the test cases from PR29970 (structs with > VLA members need further work). > > > 2021-08-01 Martin Uecker <muecker@gwdg.de> > > 2021-08-01 Martin Uecker <muecker@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. > (gimplify_target_expr): Add comment. > * c-family/c- > common.c (pointer_int_sum): Wrap pointer > operand in SAVE_EXPR and also it to the integer > argument. > > gcc/testsuite/ > PR c/91038 > PR c/29970 > * gcc.dg/vla-stexp-3.c: > New test. > * gcc.dg/vla-stexp-4.c: New test. > * gcc.dg/vla-stexp-5.c: New test. > * > gcc.dg/vla-stexp-6.c: New test. > * gcc.dg/vla-stexp-7.c: New test. > * gcc.dg/vla-stexp- > 8.c: New test. > * gcc.dg/vla-stexp-9.c: New test. > > > diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c > index 9d19e352725..522085664f5 100644 > --- a/gcc/c-family/c-common.c > +++ b/gcc/c-family/c-common.c > @@ -3348,6 +3348,16 @@ pointer_int_sum (location_t loc, enum tree_code resultcode, > intop = convert (c_common_type_for_size (TYPE_PRECISION (sizetype), > TYPE_UNSIGNED (sizetype)), intop); > > + /* Wrap the pointer expression in a SAVE_EXPR to make sure it > + * is evaluated first because the size expression may depend on it > + * for VM types. > + */ We usually don't give the trailing */ its own line. > + if (TREE_SIDE_EFFECTS (size_exp)) > + { > + ptrop = build1_loc (loc, SAVE_EXPR, TREE_TYPE (ptrop), ptrop); Why not use the save_expr function? > + intop = build2 (COMPOUND_EXPR, TREE_TYPE (intop), ptrop, intop); > + } > + > /* Replace the integer argument with a suitable product by the object size. > Do this multiplication as signed, then convert to the appropriate type > for the pointer operation and disregard an overflow that occurred only > diff --git a/gcc/gimplify.c b/gcc/gimplify.c > index d8e4b139349..be5b00b6716 100644 > --- a/gcc/gimplify.c > +++ b/gcc/gimplify.c > @@ -2958,7 +2958,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 possible culpit are size expressions for variably modified > + types which are lost in the FE or not gimplified correctly. > + */ As above. > if (VAR_P (decl) > && !DECL_SEEN_IN_BIND_EXPR_P (decl) > && !TREE_STATIC (decl) && !DECL_EXTERNAL (decl) > @@ -3103,16 +3106,22 @@ 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. */ > + The base expression may contain a statement expression that > + has declarations used in size expressions, so has to be > + gimplified before gimplifying the size expressions. > + > + So we do this in three steps. First we deal with variable > + bounds, sizes, and positions, then we gimplify the base, > + then we deal with the annotations for any variables in the > + components and any indices, from left to right. */ > + > for (i = expr_stack.length () - 1; i >= 0; i--) > { > tree t = expr_stack[i]; > > if (TREE_CODE (t) == ARRAY_REF || TREE_CODE (t) == ARRAY_RANGE_REF) > { > - /* Gimplify the low bound and element type size and put them into > + /* Deal with the low bound and element type size and put them into > the ARRAY_REF. If these values are set, they have already been > gimplified. */ > if (TREE_OPERAND (t, 2) == NULL_TREE) > @@ -3121,18 +3130,8 @@ gimplify_compound_lval (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, > if (!is_gimple_min_invariant (low)) > { > TREE_OPERAND (t, 2) = low; > - tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p, > - post_p, is_gimple_reg, > - fb_rvalue); > - ret = MIN (ret, tret); > } > } > - else > - { > - tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p, post_p, > - is_gimple_reg, fb_rvalue); > - ret = MIN (ret, tret); > - } > > if (TREE_OPERAND (t, 3) == NULL_TREE) > { > @@ -3149,18 +3148,8 @@ gimplify_compound_lval (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, > elmt_size, factor); > > TREE_OPERAND (t, 3) = elmt_size; > - tret = gimplify_expr (&TREE_OPERAND (t, 3), pre_p, > - post_p, is_gimple_reg, > - fb_rvalue); > - ret = MIN (ret, tret); > } > } > - else > - { > - tret = gimplify_expr (&TREE_OPERAND (t, 3), pre_p, post_p, > - is_gimple_reg, fb_rvalue); > - ret = MIN (ret, tret); > - } > } > else if (TREE_CODE (t) == COMPONENT_REF) > { > @@ -3180,18 +3169,8 @@ gimplify_compound_lval (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, > offset, factor); > > TREE_OPERAND (t, 2) = offset; > - tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p, > - post_p, is_gimple_reg, > - fb_rvalue); > - ret = MIN (ret, tret); > } > } > - else > - { > - tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p, post_p, > - is_gimple_reg, fb_rvalue); > - ret = MIN (ret, tret); > - } > } > } > > @@ -3202,21 +3181,34 @@ gimplify_compound_lval (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, > 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. */ > + /* Step 3: gimplify size expressions and the indices and operands of > + ARRAY_REF. During this loop we also remove any useless conversions. */ > + > for (; expr_stack.length () > 0; ) > { > tree t = expr_stack.pop (); > > if (TREE_CODE (t) == ARRAY_REF || TREE_CODE (t) == ARRAY_RANGE_REF) > { > + /* Gimplify the low bound and element type size. */ > + tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p, post_p, > + is_gimple_reg, fb_rvalue); > + ret = MIN (ret, tret); > + > + tret = gimplify_expr (&TREE_OPERAND (t, 3), pre_p, post_p, > + is_gimple_reg, fb_rvalue); > + ret = MIN (ret, tret); > + > /* Gimplify the dimension. */ > - if (!is_gimple_min_invariant (TREE_OPERAND (t, 1))) > - { > - tret = gimplify_expr (&TREE_OPERAND (t, 1), pre_p, post_p, > - is_gimple_val, fb_rvalue); > - ret = MIN (ret, tret); > - } > + tret = gimplify_expr (&TREE_OPERAND (t, 1), pre_p, post_p, > + is_gimple_val, fb_rvalue); > + ret = MIN (ret, tret); > + } > + else if (TREE_CODE (t) == COMPONENT_REF) > + { > + tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p, post_p, > + is_gimple_reg, fb_rvalue); > + ret = MIN (ret, tret); > } > > STRIP_USELESS_TYPE_CONVERSION (TREE_OPERAND (t, 0)); > @@ -6914,6 +6906,8 @@ gimplify_target_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p) > { > if (!TYPE_SIZES_GIMPLIFIED (TREE_TYPE (temp))) > gimplify_type_sizes (TREE_TYPE (temp), pre_p); > + /* FIXME: this is correct only when the size of the type does > + not depend on expressions evaluated in init. */ > gimplify_vla_decl (temp, pre_p); > } > else >
Am Montag, den 18.10.2021, 12:35 -0400 schrieb Jason Merrill: > On 10/17/21 09:52, Uecker, Martin wrote: > > > > Here is the 4th version of the patch. I tried to implement > > Jason's suggestion and this also fixes the problem. But > > I am not sure I understand the condition on > > the TREE_SIDE_EFFECTS ... > > Checking TREE_SIDE_EFFECTS filters out many trivial cases that we don't > need to worry about. Yes, but are we sure there are always side effects in the cases we care about? I assume that the problematic case are only those where the size expression depends on a variable and then there was a write to one.. But I am not sure. > I think we also want to check > variably_modified_type_p, which ought to avoid the OMP problem below. I don't think so because the problem below involves VM types. Martin > > And there is now another problem: > > > > c_finish_omp_for in c-family/c-omp.c does not seem > > to understand the expressions anymore and I get a test > > failure in > > > > testsuite/c-c++-common/gomp/for-5.c > > > > where I now get an "invalid increment expression" > > instead of the expected error. > > (bootstrapping and all other tests work fine) > > > > > > 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. Most of these issues can be addressed by > > gimplifying the base expression earlier in gimplify_compound_lval. > > > > Another issue is fixed by adding SAVE_EXPRs in pointer_int_sum > > in the FE to force a correct order of evaluation. This fixes > > PR91038 and some of the test cases from PR29970 (structs with > > VLA members need further work). > > > > > > 2021-08-01 Martin Uecker <muecker@gwdg.de> > > > > 2021-08-01 Martin Uecker <muecker@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. > > (gimplify_target_expr): Add comment. > > * c-family/c- > > common.c (pointer_int_sum): Wrap pointer > > operand in SAVE_EXPR and also it to the integer > > argument. > > > > gcc/testsuite/ > > PR c/91038 > > PR c/29970 > > * gcc.dg/vla-stexp-3.c: > > New test. > > * gcc.dg/vla-stexp-4.c: New test. > > * gcc.dg/vla-stexp-5.c: New test. > > * > > gcc.dg/vla-stexp-6.c: New test. > > * gcc.dg/vla-stexp-7.c: New test. > > * gcc.dg/vla-stexp- > > 8.c: New test. > > * gcc.dg/vla-stexp-9.c: New test. > > > > > > diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c > > index 9d19e352725..522085664f5 100644 > > --- a/gcc/c-family/c-common.c > > +++ b/gcc/c-family/c-common.c > > @@ -3348,6 +3348,16 @@ pointer_int_sum (location_t loc, enum tree_code resultcode, > > intop = convert (c_common_type_for_size (TYPE_PRECISION (sizetype), > > TYPE_UNSIGNED (sizetype)), intop); > > > > + /* Wrap the pointer expression in a SAVE_EXPR to make sure it > > + * is evaluated first because the size expression may depend on it > > + * for VM types. > > + */ > > We usually don't give the trailing */ its own line. > > > + if (TREE_SIDE_EFFECTS (size_exp)) > > + { > > + ptrop = build1_loc (loc, SAVE_EXPR, TREE_TYPE (ptrop), ptrop); > > Why not use the save_expr function? > > > + intop = build2 (COMPOUND_EXPR, TREE_TYPE (intop), ptrop, intop); > > + } > > + > > /* Replace the integer argument with a suitable product by the object size. > > Do this multiplication as signed, then convert to the appropriate type > > for the pointer operation and disregard an overflow that occurred only > > diff --git a/gcc/gimplify.c b/gcc/gimplify.c > > index d8e4b139349..be5b00b6716 100644 > > --- a/gcc/gimplify.c > > +++ b/gcc/gimplify.c > > @@ -2958,7 +2958,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 possible culpit are size expressions for variably modified > > + types which are lost in the FE or not gimplified correctly. > > + */ > > As above. > > > if (VAR_P (decl) > > && !DECL_SEEN_IN_BIND_EXPR_P (decl) > > && !TREE_STATIC (decl) && !DECL_EXTERNAL (decl) > > @@ -3103,16 +3106,22 @@ 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. */ > > + The base expression may contain a statement expression that > > + has declarations used in size expressions, so has to be > > + gimplified before gimplifying the size expressions. > > + > > + So we do this in three steps. First we deal with variable > > + bounds, sizes, and positions, then we gimplify the base, > > + then we deal with the annotations for any variables in the > > + components and any indices, from left to right. */ > > + > > for (i = expr_stack.length () - 1; i >= 0; i--) > > { > > tree t = expr_stack[i]; > > > > if (TREE_CODE (t) == ARRAY_REF || TREE_CODE (t) == ARRAY_RANGE_REF) > > { > > - /* Gimplify the low bound and element type size and put them into > > + /* Deal with the low bound and element type size and put them into > > the ARRAY_REF. If these values are set, they have already been > > gimplified. */ > > if (TREE_OPERAND (t, 2) == NULL_TREE) > > @@ -3121,18 +3130,8 @@ gimplify_compound_lval (tree *expr_p, gimple_seq *pre_p, gimple_seq > > *post_p, > > if (!is_gimple_min_invariant (low)) > > { > > TREE_OPERAND (t, 2) = low; > > - tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p, > > - post_p, is_gimple_reg, > > - fb_rvalue); > > - ret = MIN (ret, tret); > > } > > } > > - else > > - { > > - tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p, post_p, > > - is_gimple_reg, fb_rvalue); > > - ret = MIN (ret, tret); > > - } > > > > if (TREE_OPERAND (t, 3) == NULL_TREE) > > { > > @@ -3149,18 +3148,8 @@ gimplify_compound_lval (tree *expr_p, gimple_seq *pre_p, gimple_seq > > *post_p, > > elmt_size, factor); > > > > TREE_OPERAND (t, 3) = elmt_size; > > - tret = gimplify_expr (&TREE_OPERAND (t, 3), pre_p, > > - post_p, is_gimple_reg, > > - fb_rvalue); > > - ret = MIN (ret, tret); > > } > > } > > - else > > - { > > - tret = gimplify_expr (&TREE_OPERAND (t, 3), pre_p, post_p, > > - is_gimple_reg, fb_rvalue); > > - ret = MIN (ret, tret); > > - } > > } > > else if (TREE_CODE (t) == COMPONENT_REF) > > { > > @@ -3180,18 +3169,8 @@ gimplify_compound_lval (tree *expr_p, gimple_seq *pre_p, gimple_seq > > *post_p, > > offset, factor); > > > > TREE_OPERAND (t, 2) = offset; > > - tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p, > > - post_p, is_gimple_reg, > > - fb_rvalue); > > - ret = MIN (ret, tret); > > } > > } > > - else > > - { > > - tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p, post_p, > > - is_gimple_reg, fb_rvalue); > > - ret = MIN (ret, tret); > > - } > > } > > } > > > > @@ -3202,21 +3181,34 @@ gimplify_compound_lval (tree *expr_p, gimple_seq *pre_p, gimple_seq > > *post_p, > > 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. */ > > + /* Step 3: gimplify size expressions and the indices and operands of > > + ARRAY_REF. During this loop we also remove any useless conversions. */ > > + > > for (; expr_stack.length () > 0; ) > > { > > tree t = expr_stack.pop (); > > > > if (TREE_CODE (t) == ARRAY_REF || TREE_CODE (t) == ARRAY_RANGE_REF) > > { > > + /* Gimplify the low bound and element type size. */ > > + tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p, post_p, > > + is_gimple_reg, fb_rvalue); > > + ret = MIN (ret, tret); > > + > > + tret = gimplify_expr (&TREE_OPERAND (t, 3), pre_p, post_p, > > + is_gimple_reg, fb_rvalue); > > + ret = MIN (ret, tret); > > + > > /* Gimplify the dimension. */ > > - if (!is_gimple_min_invariant (TREE_OPERAND (t, 1))) > > - { > > - tret = gimplify_expr (&TREE_OPERAND (t, 1), pre_p, post_p, > > - is_gimple_val, fb_rvalue); > > - ret = MIN (ret, tret); > > - } > > + tret = gimplify_expr (&TREE_OPERAND (t, 1), pre_p, post_p, > > + is_gimple_val, fb_rvalue); > > + ret = MIN (ret, tret); > > + } > > + else if (TREE_CODE (t) == COMPONENT_REF) > > + { > > + tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p, post_p, > > + is_gimple_reg, fb_rvalue); > > + ret = MIN (ret, tret); > > } > > > > STRIP_USELESS_TYPE_CONVERSION (TREE_OPERAND (t, 0)); > > @@ -6914,6 +6906,8 @@ gimplify_target_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p) > > { > > if (!TYPE_SIZES_GIMPLIFIED (TREE_TYPE (temp))) > > gimplify_type_sizes (TREE_TYPE (temp), pre_p); > > + /* FIXME: this is correct only when the size of the type does > > + not depend on expressions evaluated in init. */ > > gimplify_vla_decl (temp, pre_p); > > } > > else > >
On 10/20/21 01:58, Uecker, Martin wrote: > Am Montag, den 18.10.2021, 12:35 -0400 schrieb Jason Merrill: >> On 10/17/21 09:52, Uecker, Martin wrote: >>> >>> Here is the 4th version of the patch. I tried to implement >>> Jason's suggestion and this also fixes the problem. But >>> I am not sure I understand the condition on >>> the TREE_SIDE_EFFECTS ... >> >> Checking TREE_SIDE_EFFECTS filters out many trivial cases that we don't >> need to worry about. > > Yes, but are we sure there are always side effects in the > cases we care about? I assume that the problematic case > are only those where the size expression depends on a > variable and then there was a write to one.. But I am not > sure. The problematic case should always involve a SAVE_EXPR or TARGET_EXPR, which has TREE_SIDE_EFFECTS. >> I think we also want to check >> variably_modified_type_p, which ought to avoid the OMP problem below. > > I don't think so because the problem below involves VM types. Ah, true. >>> And there is now another problem: >>> >>> c_finish_omp_for in c-family/c-omp.c does not seem >>> to understand the expressions anymore and I get a test >>> failure in >>> >>> testsuite/c-c++-common/gomp/for-5.c >>> >>> where I now get an "invalid increment expression" >>> instead of the expected error. >>> (bootstrapping and all other tests work fine) Ah, yes, because the variable size is still wrapped in a SAVE_EXPR even if the inner expression is just a variable reference. I think we only want to do this transformation if size_exp involves a BIND_EXPR. And probably do it sooner, changing size_exp rather than intop. Jason
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index 9d19e352725..522085664f5 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -3348,6 +3348,16 @@ pointer_int_sum (location_t loc, enum tree_code resultcode, intop = convert (c_common_type_for_size (TYPE_PRECISION (sizetype), TYPE_UNSIGNED (sizetype)), intop); + /* Wrap the pointer expression in a SAVE_EXPR to make sure it + * is evaluated first because the size expression may depend on it + * for VM types. + */ + if (TREE_SIDE_EFFECTS (size_exp)) + { + ptrop = build1_loc (loc, SAVE_EXPR, TREE_TYPE (ptrop), ptrop); + intop = build2 (COMPOUND_EXPR, TREE_TYPE (intop), ptrop, intop); + } + /* Replace the integer argument with a suitable product by the object size. Do this multiplication as signed, then convert to the appropriate type for the pointer operation and disregard an overflow that occurred only diff --git a/gcc/gimplify.c b/gcc/gimplify.c index d8e4b139349..be5b00b6716 100644 --- a/gcc/gimplify.c +++ b/gcc/gimplify.c @@ -2958,7 +2958,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 possible 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) @@ -3103,16 +3106,22 @@ 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. */ + The base expression may contain a statement expression that + has declarations used in size expressions, so has to be + gimplified before gimplifying the size expressions. + + So we do this in three steps. First we deal with variable + bounds, sizes, and positions, then we gimplify the base, + then we deal with the annotations for any variables in the + components and any indices, from left to right. */ + for (i = expr_stack.length () - 1; i >= 0; i--) { tree t = expr_stack[i]; if (TREE_CODE (t) == ARRAY_REF || TREE_CODE (t) == ARRAY_RANGE_REF) { - /* Gimplify the low bound and element type size and put them into + /* Deal with the low bound and element type size and put them into the ARRAY_REF. If these values are set, they have already been gimplified. */ if (TREE_OPERAND (t, 2) == NULL_TREE) @@ -3121,18 +3130,8 @@ gimplify_compound_lval (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, if (!is_gimple_min_invariant (low)) { TREE_OPERAND (t, 2) = low; - tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p, - post_p, is_gimple_reg, - fb_rvalue); - ret = MIN (ret, tret); } } - else - { - tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p, post_p, - is_gimple_reg, fb_rvalue); - ret = MIN (ret, tret); - } if (TREE_OPERAND (t, 3) == NULL_TREE) { @@ -3149,18 +3148,8 @@ gimplify_compound_lval (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, elmt_size, factor); TREE_OPERAND (t, 3) = elmt_size; - tret = gimplify_expr (&TREE_OPERAND (t, 3), pre_p, - post_p, is_gimple_reg, - fb_rvalue); - ret = MIN (ret, tret); } } - else - { - tret = gimplify_expr (&TREE_OPERAND (t, 3), pre_p, post_p, - is_gimple_reg, fb_rvalue); - ret = MIN (ret, tret); - } } else if (TREE_CODE (t) == COMPONENT_REF) { @@ -3180,18 +3169,8 @@ gimplify_compound_lval (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, offset, factor); TREE_OPERAND (t, 2) = offset; - tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p, - post_p, is_gimple_reg, - fb_rvalue); - ret = MIN (ret, tret); } } - else - { - tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p, post_p, - is_gimple_reg, fb_rvalue); - ret = MIN (ret, tret); - } } } @@ -3202,21 +3181,34 @@ gimplify_compound_lval (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, 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. */ + /* Step 3: gimplify size expressions and the indices and operands of + ARRAY_REF. During this loop we also remove any useless conversions. */ + for (; expr_stack.length () > 0; ) { tree t = expr_stack.pop (); if (TREE_CODE (t) == ARRAY_REF || TREE_CODE (t) == ARRAY_RANGE_REF) { + /* Gimplify the low bound and element type size. */ + tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p, post_p, + is_gimple_reg, fb_rvalue); + ret = MIN (ret, tret); + + tret = gimplify_expr (&TREE_OPERAND (t, 3), pre_p, post_p, + is_gimple_reg, fb_rvalue); + ret = MIN (ret, tret); + /* Gimplify the dimension. */ - if (!is_gimple_min_invariant (TREE_OPERAND (t, 1))) - { - tret = gimplify_expr (&TREE_OPERAND (t, 1), pre_p, post_p, - is_gimple_val, fb_rvalue); - ret = MIN (ret, tret); - } + tret = gimplify_expr (&TREE_OPERAND (t, 1), pre_p, post_p, + is_gimple_val, fb_rvalue); + ret = MIN (ret, tret); + } + else if (TREE_CODE (t) == COMPONENT_REF) + { + tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p, post_p, + is_gimple_reg, fb_rvalue); + ret = MIN (ret, tret); } STRIP_USELESS_TYPE_CONVERSION (TREE_OPERAND (t, 0)); @@ -6914,6 +6906,8 @@ gimplify_target_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p) { if (!TYPE_SIZES_GIMPLIFIED (TREE_TYPE (temp))) gimplify_type_sizes (TREE_TYPE (temp), pre_p); + /* FIXME: this is correct only when the size of the type does + not depend on expressions evaluated in init. */ gimplify_vla_decl (temp, pre_p); } else