Re: [Patch, fortran] PR85836: Implement the F2018 reduce intrinsic

5 Apr 2025

      Hi Christophe,
I have posted what I think is a fix for this problem with comment 14 of
PR119460. I would be grateful if you or one of your colleagues would give
it a spin at your earliest convenience.
It will be submitted before Monday morning.
Best regards
Paul
On Thu, 3 Apr 2025 at 17:32, Christophe Lyon christophe.lyon@linaro.org
wrote:
...
On Thu, 3 Apr 2025 at 17:55, Paul Richard Thomas
paul.richard.thomas@gmail.com wrote:
...
Hi Christophe,
As far as I can tell, the problem with reduce_1.f90 is restricted to one
call to the scalar valued version of the new intrinsic function. When the
result is array valued, all seems to be well.
...
I would be grateful if you would apply the attached patch and let me
know if the problem goes away for you. In either case, I will seek Jakub
Jelnik's help because I have completely run out of ideas.
...
Hi!
Yes with your patch the test passes on arm-unknown-linux-gnueabihf.
...
Je vous en remercie d'avance.
You're welcome!
Thanks,
Christophe
...
Paul
On Wed, 2 Apr 2025 at 10:12, Christophe Lyon christophe.lyon@linaro.org
wrote:
...
...
Hi!
On Wed, 19 Mar 2025 at 19:22, Paul Richard Thomas
paul.richard.thomas@gmail.com wrote:
...
Hi Andre,
Thanks for the review - I'll act on the points that you raised.
The Linaro people reported a failure in reduce_1.f90 execution, which
I believe is due to incorrect casting of 'dim' and a wrong specification of
its kind. I am waiting to hear back from them as to whether or not I have
fixed the failure.
...
...
...
Sorry I notice this message just today, so it's a bit outdated...
I've looked at bugzilla, so I've noticed that the are proper bug
reports about this now (and I've just checked, the problem is still
present on arm).
When you say you are "waiting to hear back from them as to whether or
not I have fixed the failure", did you contact us directly? (I'm
trying to understand if we missed your message, or how we could
improve communication).
Thanks,
Christophe
...
Cheers
Paul
On Wed, 19 Mar 2025 at 12:39, Andre Vehreschild vehre@gmx.de wrote:
...
Hi Paul,
I took a look at your patch and think I found some improvements
needed. In
...
...
...
...
+bool
+gfc_check_reduce (gfc_expr *array, gfc_expr *operation, gfc_expr
*dim,
...
...
...
...

            gfc_expr *mask, gfc_expr *identity, gfc_expr

*ordered)
...
...
...
...
+{
...

if (formal->sym->attr.allocatable || formal->sym->attr.allocatable
 || formal->sym->attr.pointer || formal->sym->attr.pointer

 || formal->sym->attr.optional || formal->sym->attr.optional

 || formal->sym->ts.type == BT_CLASS || formal->sym->ts.type

== BT_CLASS)
...
...
...
...

{
 gfc_error ("Each argument of OPERATION at %L shall be a

scalar, "
...
...
...
...

           "non-allocatable, non-pointer, non-polymorphic and "

           "nonoptional", &operation->where);

 return false;

}

The if is only looking at the first formal argument. The right-hand
sides
...
...
...
...
of the || miss a ->next-> to look at the second formal argument,
right?
...
...
...
...
May be you also want to extend the tests!?
Without having looked at it, but can't you extract the whole block of

if (array->ts.type == BT_CHARACTER)
{
 unsigned long actual_size, formal_size1, formal_size2,

result_size;
...
...
...
...
...

    return false;

  }

}

and share it with the checks for co_reduce? I figure way to many DRY
principle
...
...
...
...
violations are in gfortran. So when we can start this, why not do
it? And a
...
...
...
...
call to a routine, like check_char_arg_conformance() speaks way
better, then
...
...
...
...
having to read all that code ;-)
In gfc_resolve_reduce() identity and ordered are marked as UNUSED.
Should these
...
...
...
...
not a least be resolved?
Please run contrib/check_GNU_style on your patch. It reports several
issues
...
...
...
...
(haven't look into their validity).
In the Changelog:

  (gfc_check_rename): Add prototype for intrinsic with 6

arguments.
...
...
...
...

  * gfortran.h: Add prototype for intrinsic with 6 arguments.

s/discription/description/
I also encountered that nit with the executable stack when working in
OpenCoarrays, but haven't had time (or desire) to look into it. I
will put
...
...
...
...
myself into CC of the pr Jerry mentioned.
Besides the mentions above, this looks good to me.
Thanks for the patch and
Regards,
        Andre
On Sun, 16 Mar 2025 17:26:55 +0000
Paul Richard Thomas paul.richard.thomas@gmail.com wrote:
...
Hi All,
This version of the REDUCE intrinsic patch has evolved somewhat
since the
...
...
...
...
...
posting on 2nd March. The most important changes are to the wrapper
function and the addition of two testsuite entries.
The wrapper function now effects:
    subroutine wrapper (a, b, c)
         type_of_ARRAY, intent(inout) :: a, c
         type_of_ARRAY, intent(inout), optional :: b
         if (present (b)) then
            c = OPERATION (a,b )
         else
            c = a
         end if
    end subroutine
The reason for wrapping OPERATION in a subroutine is to allow
pointer
...
...
...
...
...
arithmetic to be used throughout in the library function. The only
thing
...
...
...
...
...
that needs to be known about the type and kind of ARRAY is the
element
...
...
...
...
...
size. The second branch in the wrapper allows deep copies to be
done in the
...
...
...
...
...
library function, such that derived types with allocatable
components do
...
...
...
...
...
not leak memory. This is needed at the final step of the algorithm
to copy
...
...
...
...
...
the result from each iteration to the result and then nullify it.
This is undoubtedly a bit heavy going for intrinsic types and so,
one day
...
...
...
...
...
soon I will possibly do a bit of M4ery. That said, the present
version
...
...
...
...
...
works for all types of ARRAY and I worry a bit about how much this
intrinsic will be used. Thoughts?
A slight niggle is the linker error that comes up if compiled
without any
...
...
...
...
...
optimization:
/usr/bin/ld: warning: /tmp/cc9cx9Rw.o: requires executable stack
(because
...
...
...
...
...
the .note.GNU-stack section is executable)
I think that this is unlikely to present a security issue,
however, since
...
...
...
...
...
it disappears at -O1, I went through each of the options triggered
by -O1
...
...
...
...
...
but couldn't make it go away. Does anybody know why this is?
Regtests OK with FC41/x86_64 - OK for mainline?
Regards
Paul
--
Andre Vehreschild * Email: vehre ad gmx dot de

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

Re: [Patch, fortran] PR85836: Implement the F2018 reduce intrinsic