| Message ID | 2013248.zzpuD0pr01@polaris |
|---|---|
| State | New |
| Headers | show |
| Series | Fix middle-end/93961 | expand |
On Wed, Mar 11, 2020 at 8:56 AM Eric Botcazou <ebotcazou@adacore.com> wrote: > > Hi, > > this is a GIMPLE verification failure in LTO mode on Ada code: > > /vol/gcc/src/hg/master/local/gcc/testsuite/gnat.dg/lto24_pkg1.ads:9:8: error: > type mismatch in 'component_ref' > lto24_pkg1__rec___b___XVN > > lto24_pkg1__rec___b___XVN > > The __XVN fields are the fields with QUAL_UNION_TYPE in Ada, which is the only > language using this type. The issue is that tree_is_indexable doesn't return > the same result for a FIELD_DECL with QUAL_UNION_TYPE and the QUAL_UNION_TYPE, > resulting in two instances of the QUAL_UNION_TYPE in the bytecode. The result > for the type is the correct one (false, since it is variably modified) while > the result for the field is falsely true because: > > else if (TREE_CODE (t) == FIELD_DECL > && lto_variably_modified_type_p (DECL_CONTEXT (t))) > return false; > > is not satisfied. The reason for this is that the DECL_QUALIFIER of fields of > a QUAL_UNION_TYPE depends on a discriminant in Ada, which means that the size > of the type does too (CONTAINS_PLACEHOLDER_P), which in turn means that it is > reset to a mere PLACEHOLDER_EXPR by free_lang_data, which finally means that > the size of DECL_CONTEXT is too, so RETURN_TRUE_IF_VAR is false. > > In other words, the CONTAINS_PLACEHOLDER_P property of the DECL_QUALIFIER of > fields of a QUAL_UNION_TYPE hides the variably_modified_type_p property of > these fields, if you look from the outside. > > This was clearly overlooked (by me) when the handling of PLACEHOLDER_EXPR was > added to LTO a decade ago, but I think that it's now too late to fundamentally > change how this is done, so I propose the attached fix. > > Tested on SPARC/Solaris and x86-64/Linux, OK for the mainline? It's not a > regression, but the fix is a no-op except for Ada and we have been using it > internally for some time without any issue so far. OK. Note I wondered for some time whether we can pre-compute (and LTO stream) whether a type is variably modified during type layout? Thanks, Richard. > > 2020-03-11 Eric Botcazou <ebotcazou@adacore.com> > > PR middle-end/93961 > * tree.c (variably_modified_type_p) <RECORD_TYPE>: Recurse into fields > whose type is a qualified union. > > -- > Eric Botcazou
> OK. Note I wondered for some time whether we can pre-compute (and LTO > stream) whether a type is variably modified during type layout? During type layout seems a little early, the end of gimplification would seem more natural since we have a workaround in RETURN_TRUE_IF_VAR: /* Test if T is either variable (if FN is zero) or an expression containing a variable in FN. If TYPE isn't gimplified, return true also if gimplify_one_sizepos would gimplify the expression into a local variable. */
diff --git a/gcc/tree.c b/gcc/tree.c index 66d52c71c99..905563fa4be 100644 --- a/gcc/tree.c +++ b/gcc/tree.c @@ -9206,8 +9206,18 @@ variably_modified_type_p (tree type, tree fn) RETURN_TRUE_IF_VAR (DECL_SIZE (t)); RETURN_TRUE_IF_VAR (DECL_SIZE_UNIT (t)); + /* If the type is a qualified union, then the DECL_QUALIFIER + of fields can also be an expression containing a variable. */ if (TREE_CODE (type) == QUAL_UNION_TYPE) RETURN_TRUE_IF_VAR (DECL_QUALIFIER (t)); + + /* If the field is a qualified union, then it's only a container + for what's inside so we look into it. That's necessary in LTO + mode because the sizes of the field tested above have been set + to PLACEHOLDER_EXPRs by free_lang_data. */ + if (TREE_CODE (TREE_TYPE (t)) == QUAL_UNION_TYPE + && variably_modified_type_p (TREE_TYPE (t), fn)) + return true; } break;
Hi, this is a GIMPLE verification failure in LTO mode on Ada code: /vol/gcc/src/hg/master/local/gcc/testsuite/gnat.dg/lto24_pkg1.ads:9:8: error: type mismatch in 'component_ref' lto24_pkg1__rec___b___XVN lto24_pkg1__rec___b___XVN The __XVN fields are the fields with QUAL_UNION_TYPE in Ada, which is the only language using this type. The issue is that tree_is_indexable doesn't return the same result for a FIELD_DECL with QUAL_UNION_TYPE and the QUAL_UNION_TYPE, resulting in two instances of the QUAL_UNION_TYPE in the bytecode. The result for the type is the correct one (false, since it is variably modified) while the result for the field is falsely true because: else if (TREE_CODE (t) == FIELD_DECL && lto_variably_modified_type_p (DECL_CONTEXT (t))) return false; is not satisfied. The reason for this is that the DECL_QUALIFIER of fields of a QUAL_UNION_TYPE depends on a discriminant in Ada, which means that the size of the type does too (CONTAINS_PLACEHOLDER_P), which in turn means that it is reset to a mere PLACEHOLDER_EXPR by free_lang_data, which finally means that the size of DECL_CONTEXT is too, so RETURN_TRUE_IF_VAR is false. In other words, the CONTAINS_PLACEHOLDER_P property of the DECL_QUALIFIER of fields of a QUAL_UNION_TYPE hides the variably_modified_type_p property of these fields, if you look from the outside. This was clearly overlooked (by me) when the handling of PLACEHOLDER_EXPR was added to LTO a decade ago, but I think that it's now too late to fundamentally change how this is done, so I propose the attached fix. Tested on SPARC/Solaris and x86-64/Linux, OK for the mainline? It's not a regression, but the fix is a no-op except for Ada and we have been using it internally for some time without any issue so far. 2020-03-11 Eric Botcazou <ebotcazou@adacore.com> PR middle-end/93961 * tree.c (variably_modified_type_p) <RECORD_TYPE>: Recurse into fields whose type is a qualified union.