Re: Need a hint or more likely help

[Steve Kargl <sgk@troutmask.apl.washington.edu>]

On Sun, Feb 11, 2024 at 11:56:52AM +0100, Mikael Morin wrote:
> Hello,
> 
> Le 11/02/2024 à 03:00, Steve Kargl a écrit :
> > All, consider this simple code:
> > 
> >    module foo
> >      contains
> >        subroutine bar
> >          character(len=:), allocatable :: s(:)
> >          call bah(s)
> >       end subroutine bar
> >    end module foo
> > 
> > If one compiles with -fdump-tree-original, one see (with some pruning)
> > 
> >    void bar ()
> >    {
> >      integer(kind=8) .s;
> >      struct array01_character(kind=1) s;
> > 
> > The above two lines seem to be ok.
> > 
> >      bitsizetype D.4319;
> >      sizetype D.4320;
> > 
> >      try
> >        {
> >          D.4319 = (bitsizetype) (sizetype) NON_LVALUE_EXPR <.s> * 8;
> >          D.4320 = (sizetype) NON_LVALUE_EXPR <.s>;
> >          s.data = 0B;
> >          s.dtype = {.elem_len=(unsigned long) .s, .version=0, .rank=1, .type=6};
> >          bah ((character(kind=1)[0:][1:.s] * restrict) s.data, .s);
> >      }
> > 
> > This is bad. .s is undefined.  I've trace this to trans-array.cc:11531
> > 
> >    if (sym->ts.type == BT_CHARACTER
> >        && !INTEGER_CST_P (sym->ts.u.cl->backend_decl))
> >      {
> >        gfc_conv_string_length (sym->ts.u.cl, NULL, &init);
> >        gfc_trans_vla_type_sizes (sym, &init);
> > 
> > The problem here is that sym->ts.u.cl->length == NULL.  If I change
> > the conditional to
> > 
> >    if (sym->ts.type == BT_CHARACTER
> >        && sym->ts.u.cl->length
> >        && !INTEGER_CST_P (sym->ts.u.cl->backend_decl))
> > 
> No, I think sym->ts.u.cl->length == NULL is correct for deferred length.
> It is set only when there is an expression defining the length:
>     character(len=size(somevar)) :: s(:)

Yes, you are correct.  I spent some time reading gfc_conv_string_length()
and does appear to address the NULL case.

> > then the option -fdump-tree-original produces
> > 
> >    void bar ()
> >    {
> >      integer(kind=8) .s;
> >      struct array01_character(kind=1) s;
> > 
> >      try
> >        {
> >          s.data = 0B;
> >          s.dtype = {.elem_len=(unsigned long) .s, .version=0, .rank=1, .type=6};
> >          bah ((character(kind=1)[0:][1:.s] * restrict) s.data, .s);
> >        }
> > 
> > which looks good except I don't know what the reference to .s here
> > means.  Is this correct or should we set .s to zero by artificially
> > setting sym->ts.u.cl->length to, say, zero length?
> > 
> Setting the variable .s to zero should be correct in any case, but it's not
> the same as setting the expression.
> I think you can use
>     if (sym->ts.deferred)
>       gfc_add_modify (
>         &init,
>         sym->ts.u.cl->backend_decl,
>         build_zero_cst (TREE_TYPE (sym->ts.u.cl->backen_decl))
>       );
> to set .s to zero.
> 
> Actually, setting to any value should be correct (including -1), as the
> value should not be used as long as the data pointer is NULL.
> 
> What is not clear is whether the values of D.4319 and D.4320  need update
> when memory is allocated for s.  I think those variables are referenced in
> the type of s (but it's not visible in the dump), and may need update when
> the value of .s changes.
> 
> I hope it helps.

Indeed, the above appears to be the solution.  I looked at 
the first 9 dumps from -fdump-tree-all, and by time we get
to a.f90.022t.ssa the temp variables D.4310 and D.4320 seem
to be optimized out/reaped.  Thanks for the help.  I tend
to forget how the trans-* files work, and sometimes need
a reminder.

-- 
Steve