diff mbox series

Fortran: fix invalid rank error in ASSOCIATED when rank is remapped [PR77652]

Message ID trinity-b07ff04e-affb-445c-b27a-14692575dbe0-1658434335166@3c-app-gmx-bs48
State New
Headers show
Series Fortran: fix invalid rank error in ASSOCIATED when rank is remapped [PR77652] | expand

Commit Message

Harald Anlauf July 21, 2022, 8:12 p.m. UTC
Dear all,

the rank check for ASSOCIATED (POINTER, TARGET) did not allow all
rank combinations that were allowed in pointer assignment for
newer versions of the Fortran standard (F2008+).  Fix the logic.

Regtested on x86_64-pc-linux-gnu.  OK for mainline?

Thanks,
Harald

Comments

Mikael Morin July 25, 2022, 10:43 a.m. UTC | #1
Le 21/07/2022 à 22:12, Harald Anlauf via Fortran a écrit :
> Dear all,
> 
> the rank check for ASSOCIATED (POINTER, TARGET) did not allow all
> rank combinations that were allowed in pointer assignment for
> newer versions of the Fortran standard (F2008+).  Fix the logic.
> 
So, if I understand correctly the (fixed) logic, it is:
  f2008+    => no check
  f2003     => check target’s rank different from 1
  up to f95 => check pointer’s rank equals target’s


I think one check is missing, that is when pointer is scalar and the 
target is non-scalar (either rank 1 or not).  This case should also be 
rejected for f2003+, not just up to f95.

Mikael
Harald Anlauf July 25, 2022, 4:01 p.m. UTC | #2
Hi Mikael, all,

a discussion in the Intel compiler forum suggests that the F2018
standard prohibits such use of the ASSOCIATED intrinsic.

https://community.intel.com/t5/Intel-Fortran-Compiler/Intel-rejects-ASSOCIATED-pointer-target-for-non-equal-ranks/m-p/1402799/highlight/true#M162159

As a consequence, the PR is likely invalid, as is the patch.
Withdrawing.

Sorry for the noise!

Harald


> Gesendet: Montag, 25. Juli 2022 um 12:43 Uhr
> Von: "Mikael Morin" <morin-mikael@orange.fr>
> An: "Harald Anlauf" <anlauf@gmx.de>, "fortran" <fortran@gcc.gnu.org>, "gcc-patches" <gcc-patches@gcc.gnu.org>
> Betreff: Re: [PATCH] Fortran: fix invalid rank error in ASSOCIATED when rank is remapped [PR77652]
>
> Le 21/07/2022 à 22:12, Harald Anlauf via Fortran a écrit :
> > Dear all,
> > 
> > the rank check for ASSOCIATED (POINTER, TARGET) did not allow all
> > rank combinations that were allowed in pointer assignment for
> > newer versions of the Fortran standard (F2008+).  Fix the logic.
> > 
> So, if I understand correctly the (fixed) logic, it is:
>   f2008+    => no check
>   f2003     => check target’s rank different from 1
>   up to f95 => check pointer’s rank equals target’s
> 
> 
> I think one check is missing, that is when pointer is scalar and the 
> target is non-scalar (either rank 1 or not).  This case should also be 
> rejected for f2003+, not just up to f95.
> 
> Mikael
>
Mikael Morin July 25, 2022, 6:08 p.m. UTC | #3
Le 25/07/2022 à 18:01, Harald Anlauf a écrit :
> Hi Mikael, all,
> 
> a discussion in the Intel compiler forum suggests that the F2018
> standard prohibits such use of the ASSOCIATED intrinsic.
> 
> https://community.intel.com/t5/Intel-Fortran-Compiler/Intel-rejects-ASSOCIATED-pointer-target-for-non-equal-ranks/m-p/1402799/highlight/true#M162159
> 

I disagree with the conclusion.  Quoting Steve Lionel’s post:
> What you're missing is this:
> 
> TARGET (optional) shall be allowable as the data-target or proc-target in a pointer assignment statement (10.2.2) in which POINTER is data-pointer-object or proc-pointer-object.
> 
> We then go to 10.2.2 which says (emphasis mine):
> 
> C1019 (R1033) If bounds-remapping-list is not specified, the ranks of data-pointer-object and data-target shall be the same.
> 
> So... not valid Fortran 2018.

except, that there is also this:
> C1018 (R1033) If bounds-remapping-list is specified, the number of bounds-remappings shall equal the rank of data-pointer-object.
which practically imposes no conformance rule between 
data-pointer-object and data-target.

Note that in the syntax definition, bounds-remapping-list is not part of 
data-pointer-object.  In other words, by collating a 
bounds-remapping-list next to POINTER, one can construct an allowable 
pointer assignment from TARGET to POINTER, which satisfies the 
requirement, even if TARGET and POINTER don’t have the same rank.
Harald Anlauf July 25, 2022, 8:18 p.m. UTC | #4
Hi Mikael,

> > https://community.intel.com/t5/Intel-Fortran-Compiler/Intel-rejects-ASSOCIATED-pointer-target-for-non-equal-ranks/m-p/1402799/highlight/true#M162159
> > 
> 
> I disagree with the conclusion.  Quoting Steve Lionel’s post:
> > What you're missing is this:
> > 
> > TARGET (optional) shall be allowable as the data-target or proc-target in a pointer assignment statement (10.2.2) in which POINTER is data-pointer-object or proc-pointer-object.
> > 
> > We then go to 10.2.2 which says (emphasis mine):
> > 
> > C1019 (R1033) If bounds-remapping-list is not specified, the ranks of data-pointer-object and data-target shall be the same.
> > 
> > So... not valid Fortran 2018.
> 
> except, that there is also this:
> > C1018 (R1033) If bounds-remapping-list is specified, the number of bounds-remappings shall equal the rank of data-pointer-object.
> which practically imposes no conformance rule between 
> data-pointer-object and data-target.

this is also why I initially thought that rank remapping is fine.

> Note that in the syntax definition, bounds-remapping-list is not part of 
> data-pointer-object.  In other words, by collating a 
> bounds-remapping-list next to POINTER, one can construct an allowable 
> pointer assignment from TARGET to POINTER, which satisfies the 
> requirement, even if TARGET and POINTER don’t have the same rank.

I fully agree with you here.

My current state of - sort-of - knowledge:

- Crayftn 14.0 allows for rank remapping, accepts code the way you describe,
  including assumed-rank for the POINTER argument.

- Nvidia 22.5 allows for rank remapping, but does not handle assumed-rank.

- NAG 7.1 is said to reject non-equal rank.  NAG 7.0 does not accept it.

- Intel rejects non-equal rank.  Steve Lionel even thinks that assumed-rank
  should not be allowed here.  I believe he is wrong here.

I would normally trust NAG more than Intel and Cray.  If somebody else convinces
me to accept that NAG has it wrong this time, I would be happy to proceed.

Apart from the above discussion about what the compiler should accept,
the library side of gfortran seems to be fine...  :-)

Harald
Mikael Morin July 26, 2022, 7:25 p.m. UTC | #5
Le 25/07/2022 à 22:18, Harald Anlauf a écrit :
> I would normally trust NAG more than Intel and Cray.  
… and yourself, it seems.  Too bad.

> If somebody else convinces me to accept that NAG has it wrong this time, I would be happy to proceed.
It won’t convince you about NAG, but here are two reasons to proceed:
  - Consensus among the maintainers is sufficient; it’s the case here.
  - If uncertain, let’s be rather too permissive than too strict; it’s 
fine as long as the runtime answer is right.
diff mbox series

Patch

From 338b43aefece04435d32f961c33d217aaa511095 Mon Sep 17 00:00:00 2001
From: Harald Anlauf <anlauf@gmx.de>
Date: Thu, 21 Jul 2022 22:02:58 +0200
Subject: [PATCH] Fortran: fix invalid rank error in ASSOCIATED when rank is
 remapped [PR77652]

gcc/fortran/ChangeLog:

	PR fortran/77652
	* check.cc (gfc_check_associated): Make the rank check of POINTER
	vs. TARGET match the selected Fortran standard.

gcc/testsuite/ChangeLog:

	PR fortran/77652
	* gfortran.dg/associated_target_9a.f90: New test.
	* gfortran.dg/associated_target_9b.f90: New test.
---
 gcc/fortran/check.cc                          | 16 +++++++++--
 .../gfortran.dg/associated_target_9a.f90      | 27 +++++++++++++++++++
 .../gfortran.dg/associated_target_9b.f90      | 15 +++++++++++
 3 files changed, 56 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/associated_target_9a.f90
 create mode 100644 gcc/testsuite/gfortran.dg/associated_target_9b.f90

diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc
index 91d87a1b2c1..6d3a4701950 100644
--- a/gcc/fortran/check.cc
+++ b/gcc/fortran/check.cc
@@ -1502,8 +1502,20 @@  gfc_check_associated (gfc_expr *pointer, gfc_expr *target)
     t = false;
   /* F2018 C838 explicitly allows an assumed-rank variable as the first
      argument of intrinsic inquiry functions.  */
-  if (pointer->rank != -1 && !rank_check (target, 0, pointer->rank))
-    t = false;
+  if (pointer->rank != -1 && pointer->rank != target->rank)
+    {
+      if (target->rank != 1)
+	{
+	  if (!gfc_notify_std (GFC_STD_F2008, "Rank remapping target is not "
+			       "rank 1 at %L", &target->where))
+	    t = false;
+	}
+      else if ((gfc_option.allow_std & GFC_STD_F2003) == 0)
+	{
+	  if (!rank_check (target, 0, pointer->rank))
+	    t = false;
+	}
+    }
   if (target->rank > 0 && target->ref)
     {
       for (i = 0; i < target->rank; i++)
diff --git a/gcc/testsuite/gfortran.dg/associated_target_9a.f90 b/gcc/testsuite/gfortran.dg/associated_target_9a.f90
new file mode 100644
index 00000000000..708645d5bcb
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/associated_target_9a.f90
@@ -0,0 +1,27 @@ 
+! { dg-do run }
+! { dg-options "-std=f2018" }
+! PR fortran/77652 - Invalid rank error in ASSOCIATED when rank is remapped
+! Contributed by Paul Thomas
+
+program p
+  real, dimension(100),  target  :: array
+  real, dimension(:,:),  pointer :: matrix
+  real, dimension(20,5), target  :: array2
+  real, dimension(:),    pointer :: matrix2
+  matrix(1:20,1:5) => array
+  matrix2(1:100)   => array2
+  !
+  ! F2018:16.9.16, ASSOCIATED (POINTER [, TARGET])
+  ! Case(v): If TARGET is present and is an array target, the result is
+  ! true if and only if POINTER is associated with a target that has
+  ! the same shape as TARGET, ...
+  if (associated (matrix, array )) stop 1
+  if (associated (matrix2,array2)) stop 2
+  call check (matrix2, array2)
+contains
+  subroutine check (ptr, tgt)
+    real, pointer :: ptr(..)
+    real, target  :: tgt(:,:)
+    if (associated (ptr, tgt)) stop 3
+  end subroutine check
+end
diff --git a/gcc/testsuite/gfortran.dg/associated_target_9b.f90 b/gcc/testsuite/gfortran.dg/associated_target_9b.f90
new file mode 100644
index 00000000000..ca62ab155c0
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/associated_target_9b.f90
@@ -0,0 +1,15 @@ 
+! { dg-do compile }
+! { dg-options "-std=f2003" }
+! PR fortran/77652 - Invalid rank error in ASSOCIATED when rank is remapped
+! Contributed by Paul Thomas
+
+subroutine s
+  real, dimension(100),  target  :: array
+  real, dimension(:,:),  pointer :: matrix
+  real, dimension(20,5), target  :: array2
+  real, dimension(:),    pointer :: matrix2
+! matrix(1:20,1:5) => array
+! matrix2(1:100)   => array2
+  print *, associated (matrix, array ) ! Technically legal F2003
+  print *, associated (matrix2,array2) ! { dg-error "is not rank 1" }
+end
--
2.35.3