Re: [PATCH] Fortran: fix associate with assumed-length character array [PR115700]

2024-07-03 Thread Andre Vehreschild
Hi Harald,

that patch looks fine to me. I'd say ok for mainline. Give it some time there
and then backport it.

Thanks for the patch.

Regards,
Andre

On Tue, 2 Jul 2024 21:44:01 +0200
Harald Anlauf  wrote:

> Dear all,
>
> the attached patch addresses an effectively bogus warning about
> uninitialized temporary string lengths of associate selectors.
> The primary reason is that the array descriptor for a character
> array is created before the corresponding string length is set.
> Moving the setting of the string length temporary to the beginning
> of the block solves the issue.
>
> The patch does not solve the case for the target containing
> substring references.  This needs to be addressed separately.
> (So far I could not find a solution that does not regress.)
>
> Regtested on x86_64-pc-linux-gnu.  OK for mainline?
>
> As the PR is marked as a regression, is it also OK for backporting?
>
> Thanks,
> Harald
>


--
Andre Vehreschild * Email: vehre ad gmx dot de


Re: [Fortran, Patch, PR 96992, V3] Fix Class arrays of different ranks are rejected as storage association argument

2024-07-03 Thread Andre Vehreschild
Hi Harald,

I am sorry for the long delay, but fixing the negative stride lead from one
issue to the next. I finally got a version that does not regress. Please have a
look.

This patch has two parts:
1. The runtime library part in pr96992_3p1.patch and
2. the compiler changes in pr96992_3p2.patch.

In my branch also the two patches from Paul for pr59104 and pr102689 are
living, which might lead to small shifts during application of the patches.

NOTE, this patch adds internal packing and unpacking of class arrays similar to
the regular pack and unpack. I think this is necessary, because the regular
un-/pack does not use the vptr's _copy routine for moving data and therefore
may produce bugs.

The un-/pack_class routines are yet only used for converting a derived type
array to a class array. Extending their use when a UN-/PACK() is applied on a
class array is still to be done (as part of another PR).

Regtests fine on x86_64-pc-linux-gnu/ Fedora 39.

Regards,
Andre

PS: @Paul I could figure my test failures with -Ox with x e { 2, 3, s } to be
caused by initialization order. I.e. a member was set only after it was read.

On Wed, 19 Jun 2024 21:17:23 +0200
Harald Anlauf  wrote:

> Hi Andre,
>
> Am 19.06.24 um 09:07 schrieb Andre Vehreschild:
> > Hi Harald,
> >
> > thank you for the investigation and useful tips. I had to figure what went
> > wrong here, but I now figured, that the array needs repacking when a
> > negative stride is used (or at least a call to that routine, which then
> > fixes "stuff"). I have added it, freeing the memory allocated potentially
> > by pack, and also updated the testcase to include the negative stride.
>
> hmmm, the pack does not always get generated:
>
> module foo_mod
>implicit none
>type foo
>   integer :: i
>end type foo
> contains
>subroutine d1(x,n)
>  integer, intent(in) :: n
>  integer :: i
>  class (foo), intent(out) :: x(n)
>  select type(x)
>  class is(foo)
> x(:)%i = (/ (42 + i, i = 1, n ) /)
>  class default
> stop 1
>  end select
>end subroutine d1
>subroutine d2(x,n)
>  integer, intent(in) :: n
>  integer :: i
>  class (foo), intent(in) :: x(n,n,n)
>  select type (x)
>  class is (foo)
> print *,"d2:  ", x%i
> if ( any( x%i /= reshape((/ (42 + i, i = 1, n ** 3 ) /), [n, n,
> n] ))) stop 2
>  class default
> stop 3
>  end select
>end subroutine d2
>
>subroutine d3(x,n)
>  integer, intent(in) :: n
>  integer :: i
>  class (foo), intent(inout) :: x(n)
>  select type (x)
>  class is (foo)
> print *,"d3_1:", x%i
> x%i = -x%i   ! Simply negate elements
> print *,"d3_2:", x%i
>  class default
> stop 33
>  end select
>end subroutine d3
> end module foo_mod
> program main
>use foo_mod
>implicit none
>type (foo), dimension(:), allocatable :: f
>integer :: n, k, m
>n = 2
>allocate (f(n*n*n))
>! Original testcase:
>call d1(f,n*n*n)
>print *, "d1->:", f%i
>call d2(f,n)
>! Ensure that array f is ok:
>print *, "d2->:", f%i
>
>! The following shows that no appropriate internal pack is generated:
>call d1(f,n*n*n)
>print *, "d1->:", f%i
>m = n*n*n
>k = 3
>print *, "->d3:", f(1:m:k)%i
>call d3(f(1:m:k),1+(m-1)/k)
>print *, "d3->:", f(1:m:k)%i
>print *, "full:", f%i
>deallocate (f)
> end program main
>
>
> After the second version of your patch this prints:
>
>   d1->:  43  44  45  46  47
>  48  49  50
>   d2:43  44  45  46  47
>  48  49  50
>   d2->:  43  44  45  46  47
>  48  49  50
>   d1->:  43  44  45  46  47
>  48  49  50
>   ->d3:  43  46  49
>   d3_1:  43  44  45
>   d3_2: -43 -44 -45
>   d3->: -43  46  49
>   full: -43 -44 -45  46  47
>  48  49  50
>
> While the print properly handles f(1:m:k)%i, passing it as
> actual argument to subroutine d3 does not do pack/unpack.
>
> Can you have another look?
>
> Thanks,
> Harald
>
>
> > Regtests fine on x86_64-pc-linux-gnu/Fedora 39. Ok for mainline?
> >
> > Regards,
> > Andre
> >
> > On Sun, 16 Jun 2024 23:27:46 +0200
> > Harald Anlauf  wrote:
> >
> > << snipped for brevity >>>
> > --
> > Andre Vehreschild * Email: vehre ad gmx dot de
>


--
Andre Vehreschild * Email: vehre ad gmx dot de
From d429783a8b5c9dc9b6004ea8bc89247d1da63127 Mon Sep 17 00:00:00 2001
From: Andre Vehreschild 
Date: Fri, 28 Jun 2024 08:31:29 +0200
Subject: [PATCH 1/2] libgfortran: Add internal un/pack_class runtime
 functions.

Packing class arrays was done using t

[OG14] Fortran/OpenMP: Support mapping of DT with allocatable components: disable 'generate_callback_wrapper' for nvptx target (was: [Patch][Stage 1] Fortran/OpenMP: Support mapping of DT with allocat

2024-07-03 Thread Thomas Schwinge
Hi Tobias!

I've compared test results for nvptx target for GCC 14 vs. the new OG14,
and ran into a number of unexpected regressions: thousands of compilation
PASS -> FAIL in the Fortran testsuite.  The few that I looked at were all
like:

ptxas /tmp/ccAMr7D9.o, line 63; error   : Illegal operand type to 
instruction 'st'
ptxas /tmp/ccAMr7D9.o, line 63; error   : Unknown symbol '%stack'
ptxas fatal   : Ptx assembly aborted due to errors
nvptx-as: ptxas returned 255 exit status
compiler exited with status 1

Comparing '-fdump-tree-all' for 'gfortran.dg/pr37287-1.f90' (randomly
picked) for GCC 14 vs. OG14, already in 'pr37287-1.f90.005t.original' we
see:

--- [GCC 14]/pr37287-1.f90.005t.original  2024-07-03 12:45:08.369948469 
+0200
+++ [OG14]/pr37287-1.f90.005t.original   2024-07-03 12:44:57.770072298 
+0200
@@ -1,3 +1,21 @@
+__attribute__((fn spec (". r r r r ")))
+integer(kind=8) __callback___iso_c_binding_C_ptr (integer(kind=8) 
(*) (void *, void * & restrict, integer(kind=2), void (*) (void)) 
cb, void * token, void * this_ptr, integer(kind=2) flag)
+{
+  integer(kind=8) result;
+  void * * scalar;
+
+  result = 0;
+  if (flag == 1)
+{
+  result = cb (token, &this_ptr, 64, 3, 0B);
+  return result;
+}
+  L$1:;
+  scalar = (void * *) this_ptr;
+  return result;
+}
+
+
 __attribute__((fn spec (". . . ")))
 void __copy___iso_c_binding_C_ptr (void * & restrict src, void * & 
restrict dst)
 {

(In addition to the whole function '__callback___iso_c_binding_C_ptr',
also note that the 'L$1:' label and 'scalar' variable are dead here; but
that's likely unrelated to the issue at hand?)

This points to OG14 commit 92c3af3d4f82351c7133b6ee90e213a8a5a485db
"Fortran/OpenMP: Support mapping of DT with allocatable components":

On 2022-03-01T16:34:18+0100, Tobias Burnus  wrote:
> this patch adds support for mapping something like
>type t
>  type(t2), allocatable :: a, b(:)
>  integer, allocatable :: c, c(:)
>end type t
>type(t), allocatable :: var, var2(:,:)
>
>!$omp target enter data map(var, var)
>
> which does a deep walk of the components at runtime.
>
> [...]
>
> Issues: None known, but I am sure with experimenting,
> more can be found - [...]

Due to a number of other commits (at least textually) depending on this
one, this commit isn't easy to revert on OG14.

But: if I disable it for nvptx target as per the attached
"Fortran/OpenMP: Support mapping of DT with allocatable components: disable 
'generate_callback_wrapper' for nvptx target",
then we're back to good -- all GCC 14 vs. OG14 regressions resolved for
nvptx target.

By the way: it's possible that we've had the same misbehavior also on
OG13 and earlier, but just nobody ever tested that for nvptx target.

Note that also outside of OG14 (that is, in GCC 14 as well as GCC trunk),
we have a number of instances of:

ptxas /tmp/ccAMr7D9.o, line 63; error   : Illegal operand type to 
instruction 'st'
ptxas /tmp/ccAMr7D9.o, line 63; error   : Unknown symbol '%stack'

... all over the Fortran test suite (only).  My current theory therefore
is that there is some latent issue, which is just greatly exacerbated by
OG14 commit 92c3af3d4f82351c7133b6ee90e213a8a5a485db
"Fortran/OpenMP: Support mapping of DT with allocatable components" (or
some related change).

This could be the Fortran front end generating incorrect GIMPLE, or the
middle end or (more likely?) nvptx back end not correctly handling
something that only comes into existance via the Fortran front end.

Anyway: until we understand the underlying issue, OK to push the attached
"Fortran/OpenMP: Support mapping of DT with allocatable components: disable 
'generate_callback_wrapper' for nvptx target"
to devel/omp/gcc-14 branch?


Grüße
 Thomas


>From 3fb9e4cabea736ace66ee197be1b13a978af10ac Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Wed, 3 Jul 2024 22:09:39 +0200
Subject: [PATCH] Fortran/OpenMP: Support mapping of DT with allocatable
 components: disable 'generate_callback_wrapper' for nvptx target

This is, obviously, not the final fix for this issue.

	gcc/fortran/
	* class.cc (generate_callback_wrapper) [GCC_NVPTX_H]: Disable.
---
 gcc/fortran/class.cc | 25 +
 1 file changed, 25 insertions(+)

diff --git a/gcc/fortran/class.cc b/gcc/fortran/class.cc
index 15aacd98fd8..2c062204e5a 100644
--- a/gcc/fortran/class.cc
+++ b/gcc/fortran/class.cc
@@ -64,6 +64,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "gfortran.h"
 #include "constructor.h"
 #include "target-memory.h"
+#include "tm.h" //TODO
 
 /* Inserts a derived type component reference in a data reference chain.
 TS: base type of the ref chain so far, in which we will pick the component
@@ -2420,6 +2421,30 @@ generate_callback_wrapper (gfc_symbol *vtab, gfc_symbol *derived,
 			   gfc_namespace *ns, const char *tname,
 			   gfc_compon