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

[Richard Biener <richard.guenther@gmail.com>]

On Mon, Feb 20, 2023 at 12:57 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Mon, Feb 20, 2023 at 12:48:38PM +0100, Tobias Burnus wrote:
> > On 20.02.23 12:15, Jakub Jelinek wrote:
> > > On Mon, Feb 20, 2023 at 12:07:43PM +0100, Tobias Burnus wrote:
> > > > As mentioned in the TODO for 'deferred', I think we really want
> > > > to have NULL as upper value for the domain for the type, but that
> > > > requires literally hundred of changes to the compiler, which
> > > > I do not want to due during Stage 4, but that are eventually
> > > > required.* — In any case, this patch fixes some of the issues
> > > > in the meanwhile.
> > > Yeah, the actual len can be in some type's lang_specific member.
> >
> > Actually, I think it should be bound to the DECL and not to the TYPE,
> > i.e. lang_decl not type_lang.
> >
> > I just see that, the latter already has a 'tree stringlen' (for I/O)
> > which probably could be reused for this purpose.
>
> I'd drop the
>  && TREE_CODE (TYPE_SIZE (type)) == SAVE_EXPR
> and assert == SAVE_EXPR part, with SAVE_EXPRs one never knows if they
> are added around the whole expression or say some subexpression has
> it and then some trivial arithmetics happens on the SAVE_EXPR tree.
>
> > > Anyway, for the patch for now, I'd probably instead of stripping
> > > SAVE_EXPR overwrite the 2 sizes with newly built expressions.
> >
> > What I now did. (Unchanged otherwise, except that I now also mention
> > GFC_DECL_STRING_LEN in the TODO.)
> >
> > OK for mainline?
>
> If Richard doesn't object.

 tree
-gfc_get_character_type_len_for_eltype (tree eltype, tree len)
+gfc_get_character_type_len_for_eltype (tree eltype, tree len, bool deferred)
 {
   tree bounds, type;

   bounds = build_range_type (gfc_charlen_type_node, gfc_index_one_node, len);
   type = build_array_type (eltype, bounds);
   TYPE_STRING_FLAG (type) = 1;
-
+  if (len && deferred && TREE_CODE (TYPE_SIZE (type)) == SAVE_EXPR)
+    {
+      /* 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.

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?  Since gimplification will produce the result
into a single temporary again, re-storing the "breakage".

So, does it _really_ fix things?

Richard.


>
>         Jakub
>