Re: [patch, fortran] Implement maxval for characters

[James Greenhalgh <james dot greenhalgh at arm dot com>]

On Wed, Dec 06, 2017 at 11:38:21AM +0000, Christophe Lyon wrote:
> Hi,
> 
> 
> On 28 November 2017 at 19:40, Thomas Koenig <tkoenig@netcologne.de> wrote:
> > Hello world,
> >
> > the attached patch implements maxval for characters, an F2003 feature
> > that we were missing up to now.
> >
> > Regression-tested on x86_64-pc-linux-gnu.
> >
> > OK for trunk?
> >
> > Regards
> >
> >         Thomas
> >
> > 2017-11-28  Thomas Koenig  <tkoenig@gcc.gnu.org>
> >
> >         PR fortran/36313
> >         * check.c (gfc_check_minval_maxval): Use
> >         int_orLreal_or_char_check_f2003 for array argument.
> >         * iresolve.c (gfc_resolve_maxval): Insert number in
> >         function name for character arguments.
> >         (gfc_resolve_minval): Likewise.
> >         * trans-intrinsic.c (gfc_conv_intrinsic_minmaxloc):
> >         Fix comment.
> >         (gfc_conv_intrinsic_minmaxval): Resort arguments and call library
> >         function if dealing with a character function.
> >
> > 2017-11-28  Thomas Koenig  <tkoenig@gcc.gnu.org>
> >
> >         PR fortran/36313
> >         * Makefile.am: Add new files for character-valued
> >         maxval and minval.
> >         * Makefile.in: Regenerated.
> >         * gfortran.map: Add new functions.
> >         * m4/iforeach-s2.m4: New file.
> >         * m4/ifunction-s2.m4: New file.
> >         * m4/iparm.m4: Add intitval for minval and maxval.
> >         * m4/maxval0s.m4: New file.
> >         * m4/maxval1s.m4: New file.
> >         * m4/minval0s.m4: New file.
> >         * m4/minval1s.m4: New file.
> >         * generated/maxval0_s1.c: New file.
> >         * generated/maxval0_s4.c: New file.
> >         * generated/maxval1_s1.c: New file.
> >         * generated/maxval1_s4.c: New file.
> >         * generated/minval0_s1.c: New file.
> >         * generated/minval0_s4.c: New file.
> >         * generated/minval1_s1.c: New file.
> >         * generated/minval1_s4.c: New file.
> >
> > 2017-11-28  Thomas Koenig  <tkoenig@gcc.gnu.org>
> >
> >         PR fortran/36313
> >         * gfortran.dg/maxval_char_1.f90: New test.
> >         * gfortran.dg/maxval_char_2.f90: New test.
> >         * gfortran.dg/maxval_char_3.f90: New test.
> >         * gfortran.dg/maxval_char_4.f90: New test.
> >         * gfortran.dg/minval_char_1.f90: New test.
> >         * gfortran.dg/minval_char_2.f90: New test.
> >         * gfortran.dg/minval_char_3.f90: New test.
> >         * gfortran.dg/minval_char_4.f90: New test.
> 
> Hi,
> In my testing I'm seeing random results with at least some of these new tests
> (maxval_char_1, maxval_char_2, minval_char_2 at least).
> I'm cross-testing on arm targets using qemu.
> 
> Sorry, I don't really read fortran, so a first obvious question: is
> there anything
> undefined/random/race condition in these tests?
> My logs only show that the program aborted, so it doesn't seem the process
> was killed by a timeout or similar.

I'm also seeing these tests fail sporadically on x86_64-none-linux-gnu and
aarch64-none-linux-gnu.

I also struggle with reading fortran, one abort which triggers for me in
maxval_char_1.f90 is

  if (res /= maxval(b, mask)) call abort

I think we're getting in to trouble when the mask comes out as all False,
that seems to give a different behaviour between this call:

  write (unit=res,fmt='(I5.5)') maxval(v,mask)

After which res = "*****"

And this call:

  maxval(b, mask)

Which returns the empty string. Thus, res != maxval(b, mask) and we fail the
test.

I presume something similar is happening in the other tests from this patch.

Thanks,
James