* [patch] Fix middle-end/93961
@ 2020-03-11 7:49 Eric Botcazou
2020-03-11 8:30 ` Richard Biener
0 siblings, 1 reply; 3+ messages in thread
From: Eric Botcazou @ 2020-03-11 7:49 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1996 bytes --]
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.
--
Eric Botcazou
[-- Attachment #2: pr93961.diff --]
[-- Type: text/x-patch, Size: 924 bytes --]
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;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch] Fix middle-end/93961
2020-03-11 7:49 [patch] Fix middle-end/93961 Eric Botcazou
@ 2020-03-11 8:30 ` Richard Biener
2020-03-11 10:26 ` Eric Botcazou
0 siblings, 1 reply; 3+ messages in thread
From: Richard Biener @ 2020-03-11 8:30 UTC (permalink / raw)
To: Eric Botcazou; +Cc: GCC Patches
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch] Fix middle-end/93961
2020-03-11 8:30 ` Richard Biener
@ 2020-03-11 10:26 ` Eric Botcazou
0 siblings, 0 replies; 3+ messages in thread
From: Eric Botcazou @ 2020-03-11 10:26 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
> 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. */
--
Eric Botcazou
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-03-11 10:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-11 7:49 [patch] Fix middle-end/93961 Eric Botcazou
2020-03-11 8:30 ` Richard Biener
2020-03-11 10:26 ` Eric Botcazou
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).