Re: [Patch] Fortran: Avoid SAVE_EXPR for deferred-len char types

[Tobias Burnus <tobias@codesourcery.com>]

Hi Richard, hi all,

On 20.02.23 13:46, Richard Biener wrote:
> +      /* TODO: A more middle-end friendly alternative would be to use NULL_TREE
> +        as upper bound and store the value, e.g. as GFC_DECL_STRING_LEN.
> +        Caveat: this requires some cleanup throughout the code to consistently
> +        use some wrapper function.  */
> +      gcc_assert (TREE_CODE (TYPE_SIZE_UNIT (type)) == SAVE_EXPR);
> +      tree tmp = TREE_TYPE (TYPE_SIZE (eltype));
>
> ...
>
> you are probably breaking type sharing here.  You could use
> build_array_type_1 and pass false for 'shared' to get around that.  Note
> that there's also canonical type building done in case 'eltype' is not
> canonical itself.

My feeling is that this is already somewhat broken. Currently, there
is one type per decl as each has its own artificial length variable.
I have no idea how this will be handled in the ME in terms of alias
analysis. And whether shared=false makes sense here and what effect
is has. (Probably yes.)

In principle,
   integer(kind=8) .str., .str2;
   character(kind=1)[1:.str] * str;
   character(kind=1)[1:.str2] * str2;
have the same type and iff .str == .str at runtime, they can alias.
Example:
   str2 = str;
   .str2 = .str;

I have no idea how the type analysis currently works (with or without SAVE_EXPR)
nor what effect shared=false has in this case.

> The solution to the actual problem is a hack - you are relying on
> re-evaluation of TYPE_SIZE, and for that, only from within accesses
> from inside the frontend?

I think this mostly helps with access inside the FE of the type 'size =
TYPE_SIZE_UNIT(type)', which is used surprisingly often and is often
directly evaluated (i.e. assigned to a temporary).

> Since gimplification will produce the result into a single temporary again, re-storing the "breakage".
> So, does it_really_  fix things?

It does seem to fix cases which do  'size = TYPE_SIZE_UNIT (type);' in
the front end and then uses this size expression. Thus, there are fixed.
However, there are many cases where things go wrong - with and without
the patch. I keep discovering more and more :-(

* * *

I still think that the proper way is to have NULL_TREE as upper value
would be better in several ways, except that there is (too) much code
which relies on TYPE_UNIT_SIZE to work. (There are 117 occurrences).
Additionally, there is more code doing assumptions in this area.

Thus, the question is whether it makes sense as hackish partial solution
or whether it should remain in the current broken stage until it is
fixed properly.

Tobias,

who would like to have more time for fixing such issues.

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955