Hi Harald and Jerry,

I cannot see why the segfault is occurring of course:
          _gfortran_transfer_character_write (&dt_parm.9, &"line 4:"[1]{lb:
1 sz: 1}, 7);
          {
            struct array01_integer(kind=4) parm.10;
            integer(kind=8) D.4841;
            struct array01_integer(kind=4) parm.11;
            integer(kind=8) D.4848;
            struct array01_integer(kind=4) parm.12;

            D.4841 = (integer(kind=8)) sort_2 ((integer(kind=4)[0:] *)
parm.10.data);  // parm10 not set.

I am going to see if stopping the call to
'add_check_section_in_array_bounds' when an inner loop is evaluating an
argument for a function call in the outer loop does the trick.

That said, while your patch seems to be a bit hacky, it does fix the
problem in a sensible way. I just worry about potential corner cases since
it is the call to gfc_conv_expr_descriptor that causes the problem.

Cheers

Paul


On Wed, 27 Nov 2024 at 21:35, Harald Anlauf <anl...@gmx.de> wrote:

> Am 27.11.24 um 21:56 schrieb Jerry D:
> > On 11/27/24 12:31 PM, Harald Anlauf wrote:
> >> Dear all,
> >>
> >> the attached patch fixes a wrong-code issue with bounds-checking
> >> enabled when doing I/O of an array section and an index is either
> >> an expression or a function result.  The problem does not occur
> >> without bounds-checking.
> >>
> >> When looking at the original testcase, the function occuring in
> >> the affected index was evaluated twice, once with wrong arguments.
> >>
> >> The most simple solution appears to fall back to scalarization
> >> with bounds-checking enabled.  If someone has a quick idea to
> >> handle this better, please speak up!
> >>
> >> Regtested on x86_64-pc-linux-gnu.  OK for mainline?
> >>
> >> This seems to be a 14/15 regression, so a backport is advisable.
> >>
> >> Thanks,
> >> Harald
> >>
> >
> > The patch looks OK to me.
> >
> > I wonder if this fall back to the scalarizer should be done everywhere
> > if a a user has specified bounds checking, what is the point of
> > optimizing array references?
>
> If an array reference is of the type A(:,f()), there is no need to
> do bounds-checking for the first array index (we don't, so OK),
> and we also could pass the array slice to a library function that
> handles the section in one go, without generating a loop with calls.
> Scalarization is then sort of a missed-optimization.
>
> The problem is that the second argument is somehow evaluated twice
> with bounds-checking, but only with the I/O optimization.  I did not
> see such an issue when assigning A(:,f()) to a temporary rank-1 array
> and passing that array to the write().  It did create the right bounds
> check, and called f() correctly just once.
>
> Instead of creating a temporary, just passing to the scalarizer was
> the simpler solution.  Maybe Paul has an idea to solve this in a
> better way.
>
> > If the code works in 13 maybe we need to isolate to what broke it and
> > intervene at that place.
>
> Looking at the tree-dump, no bounds-check was generated in 13.
> I did some work to extend bounds-checking during 14-development,
> and the testcase may have just uncovered a latent issue?
>
> (And we sometimes evaluate functions way too often, see e.g. pr114021,
> so there's no lack of possibly related issues...)
>
> > Also go ahead with back porting if no other ideas pop up.  I just fear
> > we are covering up something else.
>
> I'll wait until tomorrow to see if Paul intervenes.  Otherwise I will
> proceed and push.
>
> Thanks for the review and discussion!
>
> Harald
>
> > Jerry
> >
> >
>
>

Reply via email to