diff mbox series

[Fortran] Fix setting of array lower bound for named arrays

Message ID c8d8cac8-d066-cb1f-d1cc-c932f3edb89f@codesourcery.com
State Committed
Commit 6262e3a22b3d86afc116480bc59a7bb30b0cfd40
Headers show
Series [Fortran] Fix setting of array lower bound for named arrays | expand

Commit Message

Chung-Lin Tang Nov. 29, 2021, 2:25 p.m. UTC
This patch by Tobias, fixes a case of setting array low-bounds, found
for particular uses of SOURCE=/MOLD=.

For example:
program A_M
   implicit none
   real, dimension (:), allocatable :: A, B
   allocate (A(0:5))
   call Init (A)
contains
   subroutine Init ( A )
     real, dimension ( 0 : ), intent ( in ) :: A
     integer, dimension ( 1 ) :: lb_B

     allocate (B, mold = A)
     ...
     lb_B = lbound (B, dim=1)   ! Error: lb_B assigned 1, instead of 0 like lower-bound of A.

Referencing the Fortran standard:

"16.9.109 LBOUND (ARRAY [, DIM, KIND])"
states:
"If DIM is present, ARRAY is a whole array, and either ARRAY is
  an assumed-size array of rank DIM or dimension DIM of ARRAY has
  nonzero extent, the result has a value equal to the lower bound
  for subscript DIM of ARRAY. Otherwise, if DIM is present, the
  result value is 1."

And on what is a "whole array":

"9.5.2 Whole arrays"
"A whole array is a named array or a structure component ..."

The attached patch adjusts the relevant part in gfc_trans_allocate() to only set
e3_has_nodescriptor only for non-named arrays.

Tobias has tested this once, and I've tested this patch as well on our complete set of
testsuites (which usually serves for OpenMP related stuff). Everything appears well with no regressions.

Is this okay for trunk?

Thanks,
Chung-Lin

2021-11-29  Tobias Burnus  <tobias@codesourcery.com>

gcc/fortran/ChangeLog:

	* trans-stmt.c (gfc_trans_allocate): Set e3_has_nodescriptor to true
	only for non-named arrays.

gcc/testsuite/ChangeLog:

	* gfortran.dg/allocate_with_source_26.f90: Adjust testcase.
	* gfortran.dg/allocate_with_mold_4.f90: New testcase.

Comments

Harald Anlauf Nov. 29, 2021, 8:21 p.m. UTC | #1
Hi Chung-Lin,

Am 29.11.21 um 15:25 schrieb Chung-Lin Tang:
> This patch by Tobias, fixes a case of setting array low-bounds, found
> for particular uses of SOURCE=/MOLD=.
>
> For example:
> program A_M
>    implicit none
>    real, dimension (:), allocatable :: A, B
>    allocate (A(0:5))
>    call Init (A)
> contains
>    subroutine Init ( A )
>      real, dimension ( 0 : ), intent ( in ) :: A
>      integer, dimension ( 1 ) :: lb_B
>
>      allocate (B, mold = A)
>      ...
>      lb_B = lbound (B, dim=1)   ! Error: lb_B assigned 1, instead of 0
> like lower-bound of A.
>
> Referencing the Fortran standard:
>
> "16.9.109 LBOUND (ARRAY [, DIM, KIND])"
> states:
> "If DIM is present, ARRAY is a whole array, and either ARRAY is
>   an assumed-size array of rank DIM or dimension DIM of ARRAY has
>   nonzero extent, the result has a value equal to the lower bound
>   for subscript DIM of ARRAY. Otherwise, if DIM is present, the
>   result value is 1."
>
> And on what is a "whole array":
>
> "9.5.2 Whole arrays"
> "A whole array is a named array or a structure component ..."
>
> The attached patch adjusts the relevant part in gfc_trans_allocate() to
> only set
> e3_has_nodescriptor only for non-named arrays.
>
> Tobias has tested this once, and I've tested this patch as well on our
> complete set of
> testsuites (which usually serves for OpenMP related stuff). Everything
> appears well with no regressions.
>
> Is this okay for trunk?

I think you need to check the following:

  allocate(c, source=h(3))
  write(*,*) lbound(c,1), ubound(c,1) ! prints 1 3

...

  pure function h(i) result(r)
   integer, value, intent(in) :: i
   integer, allocatable :: r(:)
   allocate(r(3:5))
   r = [1,2,3]
  end function h

This used to print 3 5, which is also what e.g. NAG, Nvidia, flang do.
Intel prints 1 3, so it agrees with you.

The Fortran standard has:

9.7.1.2  Execution of an ALLOCATE statement

(6) When an ALLOCATE statement is executed for an array with no
     allocate-shape-spec-list, the bounds of source-expr determine the
     bounds of the array. Subsequent changes to the bounds of source-expr
     do not affect the array bounds.

Please re-check with Tobias.

Thanks,
Harald

> Thanks,
> Chung-Lin
>
> 2021-11-29  Tobias Burnus  <tobias@codesourcery.com>
>
> gcc/fortran/ChangeLog:
>
>      * trans-stmt.c (gfc_trans_allocate): Set e3_has_nodescriptor to true
>      only for non-named arrays.
>
> gcc/testsuite/ChangeLog:
>
>      * gfortran.dg/allocate_with_source_26.f90: Adjust testcase.
>      * gfortran.dg/allocate_with_mold_4.f90: New testcase.
Tobias Burnus Nov. 29, 2021, 8:56 p.m. UTC | #2
Hi Harald, hi Chung-Lin,

On 29.11.21 21:21, Harald Anlauf wrote:
> I think you need to check the following:
>
>  allocate(c, source=h(3))
>  write(*,*) lbound(c,1), ubound(c,1) ! prints 1 3
> ...
>  pure function h(i) result(r)
>   integer, value, intent(in) :: i
>   integer, allocatable :: r(:)
>   allocate(r(3:5))
>   r = [1,2,3]
>  end function h
>
> This used to print 3 5, which is also what e.g. NAG, Nvidia, flang do.
> Intel prints 1 3, so it agrees with you.
Hmm, usually NAG is right ...
> The Fortran standard has:
> 9.7.1.2  Execution of an ALLOCATE statement
>
> (6) When an ALLOCATE statement is executed for an array with no
>     allocate-shape-spec-list, the bounds of source-expr determine the
>     bounds of the array. Subsequent changes to the bounds of source-expr
>     do not affect the array bounds.

The problem is that the standard does not really state what the bounds
are :-(

Usually, it ends up referring to LBOUND (with wordings like "each lower
bound equal to the corresponding element of LBOUND (expr)") – albeit not
in this case.

Assuming that holds, the question is what's lbound(h(3))? gfortran gives
[1] for that one. What does NAG yield?

For lbound(h(3),dim=1), the standard has:

"If DIM is present, ARRAY is a whole array, and either ARRAY is an
assumed-size array of rank DIM or dimension DIM of ARRAY has nonzero
extent, the result has a value equal to the lower bound for subscript
DIM of ARRAY. Otherwise, if DIM is present, the result value is 1."

Thus, the question is whether "h(3)" is a 'whole array' or not. That reads:

"A whole array is a named array or a structure component whose final
part-ref is an array component name; no subscript list is appended."

I think in "h(3)" there is not really a named array – thus I read it as
if the "Otherwise ... result value is 1" applies.

Tobias

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
Harald Anlauf Nov. 29, 2021, 9:11 p.m. UTC | #3
Hi Tobias, all,

Am 29.11.21 um 21:56 schrieb Tobias Burnus:
> The problem is that the standard does not really state what the bounds
> are :-(

I sort of expected that comment...

> Usually, it ends up referring to LBOUND (with wordings like "each lower
> bound equal to the corresponding element of LBOUND (expr)") – albeit not
> in this case.
>
> Assuming that holds, the question is what's lbound(h(3))? gfortran gives
> [1] for that one. What does NAG yield?
>
> For lbound(h(3),dim=1), the standard has:
>
> "If DIM is present, ARRAY is a whole array, and either ARRAY is an
> assumed-size array of rank DIM or dimension DIM of ARRAY has nonzero
> extent, the result has a value equal to the lower bound for subscript
> DIM of ARRAY. Otherwise, if DIM is present, the result value is 1."
>
> Thus, the question is whether "h(3)" is a 'whole array' or not. That reads:

F2018: 9.5.2 Whole arrays

> "A whole array is a named array or a structure component whose final
> part-ref is an array component name; no subscript list is appended."
>
> I think in "h(3)" there is not really a named array – thus I read it as
> if the "Otherwise ... result value is 1" applies.

If you read on in the standard:

"The appearance of a whole array variable in an executable construct
specifies all the elements of the array ..."

which might make you/makes me think that the sentence before that one
could need an official interpretation...

I've submitted a reduced example to the Intel Fortran Forum:

https://community.intel.com/t5/Intel-Fortran-Compiler/Allocate-with-SOURCE-and-bounds/m-p/1339992#M158535

There are good chances that Steve Lionel reads and comments on it.

You're invited to add to that thread!

Harald

> Tobias
>
> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201,
> 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer:
> Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München;
> Registergericht München, HRB 106955
>
Tobias Burnus Nov. 30, 2021, 5:24 p.m. UTC | #4
On 29.11.21 22:11, Harald Anlauf wrote:

>> "A whole array is a named array or a structure component whose final
>> part-ref is an array component name; no subscript list is appended."
>>
>> I think in "h(3)" there is not really a named array – thus I read it as
>> if the "Otherwise ... result value is 1" applies.
>
> If you read on in the standard:
>
> "The appearance of a whole array variable in an executable construct
> specifies all the elements of the array ..."
>
> which might make you/makes me think that the sentence before that one
> could need an official interpretation...

I am not sure whether I understand what part of the spec you wonder
about. (I mean besides that 'variable' can also mean referencing a
data-pointer-returning function.)

Question: What do NAG/flang/... report for lbound(h(3)) - also [3] – or
[1] as gfortran?

> I've submitted a reduced example to the Intel Fortran Forum:
> https://community.intel.com/t5/Intel-Fortran-Compiler/Allocate-with-SOURCE-and-bounds/m-p/1339992#M158535
>
>
> There are good chances that Steve Lionel reads and comments on it.

So far only "FortranFan" has replied – and he comes to the same
conclusion as my reading, albeit without referring to the standard.

Tobias

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
Harald Anlauf Nov. 30, 2021, 7:54 p.m. UTC | #5
Hi Tobias,

Am 30.11.21 um 18:24 schrieb Tobias Burnus:
> On 29.11.21 22:11, Harald Anlauf wrote:
>
>>> "A whole array is a named array or a structure component whose final
>>> part-ref is an array component name; no subscript list is appended."
>>>
>>> I think in "h(3)" there is not really a named array – thus I read it as
>>> if the "Otherwise ... result value is 1" applies.
>>
>> If you read on in the standard:
>>
>> "The appearance of a whole array variable in an executable construct
>> specifies all the elements of the array ..."
>>
>> which might make you/makes me think that the sentence before that one
>> could need an official interpretation...
>
> I am not sure whether I understand what part of the spec you wonder
> about. (I mean besides that 'variable' can also mean referencing a
> data-pointer-returning function.)

strictly speaking you're now talking about the text for LBOUND,
and your quote is not from the standard section about the ALLOCATE
statement.  And there are several places in the standard document
where there is an explicit reference to LBOUND when talking about
what the bounds should be.  This is why I am unhappy with the text
about ALLOCATE, not about LBOUND.

> Question: What do NAG/flang/... report for lbound(h(3)) - also [3] – or
> [1] as gfortran?
>
>> I've submitted a reduced example to the Intel Fortran Forum:
>> https://community.intel.com/t5/Intel-Fortran-Compiler/Allocate-with-SOURCE-and-bounds/m-p/1339992#M158535
>>
>>
>>
>> There are good chances that Steve Lionel reads and comments on it.
>
> So far only "FortranFan" has replied – and he comes to the same
> conclusion as my reading, albeit without referring to the standard.

You seem to be quite convinced with your interpretation,
while I am simply confused.

So go ahead and apply to mainline.  Let's see if we learn more.
I do hope I will.

Harald

> Tobias
>
> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201,
> 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer:
> Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München;
> Registergericht München, HRB 106955
>
Toon Moene Nov. 30, 2021, 8:01 p.m. UTC | #6
On 11/30/21 8:54 PM, Harald Anlauf via Fortran wrote:

> Hi Tobias,

> You seem to be quite convinced with your interpretation,
> while I am simply confused.

If both compiler developers are confused, and actual compiler 
implementations differ in their outcomes of the test case, IMNSHO it is 
time to ask the Fortran Standardization Committee for an interpretation 
(of the standard's text).

Kind regards,
diff mbox series

Patch

diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c
index bdf7957..982e1e0 100644
--- a/gcc/fortran/trans-stmt.c
+++ b/gcc/fortran/trans-stmt.c
@@ -6660,16 +6660,13 @@  gfc_trans_allocate (gfc_code * code)
       else
 	e3rhs = gfc_copy_expr (code->expr3);
 
-      // We need to propagate the bounds of the expr3 for source=/mold=;
-      // however, for nondescriptor arrays, we use internally a lower bound
-      // of zero instead of one, which needs to be corrected for the allocate obj
-      if (e3_is == E3_DESC)
-	{
-	  symbol_attribute attr = gfc_expr_attr (code->expr3);
-	  if (code->expr3->expr_type == EXPR_ARRAY ||
-	      (!attr.allocatable && !attr.pointer))
-	    e3_has_nodescriptor = true;
-	}
+      // We need to propagate the bounds of the expr3 for source=/mold=.
+      // However, for non-named arrays, the lbound has to be 1 and neither the
+      // bound used inside the called function even when returning an
+      // allocatable/pointer nor the zero used internally.
+      if (e3_is == E3_DESC
+	  && code->expr3->expr_type != EXPR_VARIABLE)
+	e3_has_nodescriptor = true;
     }
 
   /* Loop over all objects to allocate.  */
diff --git a/gcc/testsuite/gfortran.dg/allocate_with_mold_4.f90 b/gcc/testsuite/gfortran.dg/allocate_with_mold_4.f90
new file mode 100644
index 0000000..d545fe1
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/allocate_with_mold_4.f90
@@ -0,0 +1,24 @@ 
+program A_M
+  implicit none
+  real, parameter :: C(5:10) = 5.0
+  real, dimension (:), allocatable :: A, B
+  allocate (A(6))
+  call Init (A)
+contains
+  subroutine Init ( A )
+    real, dimension ( -1 : ), intent ( in ) :: A
+    integer, dimension ( 1 ) :: lb_B
+
+    allocate (B, mold = A)
+    if (any (lbound (B) /= lbound (A))) stop 1
+    if (any (ubound (B) /= ubound (A))) stop 2
+    if (any (shape (B) /= shape (A))) stop 3
+    if (size (B) /= size (A)) stop 4
+    deallocate (B)
+    allocate (B, mold = C)
+    if (any (lbound (B) /= lbound (C))) stop 5
+    if (any (ubound (B) /= ubound (C))) stop 6
+    if (any (shape (B) /= shape (C))) stop 7
+    if (size (B) /= size (C)) stop 8
+end
+end 
diff --git a/gcc/testsuite/gfortran.dg/allocate_with_source_26.f90 b/gcc/testsuite/gfortran.dg/allocate_with_source_26.f90
index 28f24fc..323c8a3 100644
--- a/gcc/testsuite/gfortran.dg/allocate_with_source_26.f90
+++ b/gcc/testsuite/gfortran.dg/allocate_with_source_26.f90
@@ -34,23 +34,23 @@  program p
  if (lbound(p1, 1) /= 3 .or. ubound(p1, 1) /= 4 &
      .or. lbound(p2, 1) /= 3 .or. ubound(p2, 1) /= 4 &
      .or. lbound(p3, 1) /= 1 .or. ubound(p3, 1) /= 2 &
-     .or. lbound(p4, 1) /= 7 .or. ubound(p4, 1) /= 8 &
+     .or. lbound(p4, 1) /= 1 .or. ubound(p4, 1) /= 2 &
      .or. p1(3)%i /= 43 .or. p1(4)%i /= 56 &
      .or. p2(3)%i /= 43 .or. p2(4)%i /= 56 &
      .or. p3(1)%i /= 43 .or. p3(2)%i /= 56 &
-     .or. p4(7)%i /= 11 .or. p4(8)%i /= 12) then
+     .or. p4(1)%i /= 11 .or. p4(2)%i /= 12) then
    call abort()
  endif
 
  !write(*,*) lbound(a,1), ubound(a,1) ! prints 1 3
  !write(*,*) lbound(b,1), ubound(b,1) ! prints 1 3
- !write(*,*) lbound(c,1), ubound(c,1) ! prints 3 5
+ !write(*,*) lbound(c,1), ubound(c,1) ! prints 1 3
  !write(*,*) lbound(d,1), ubound(d,1) ! prints 1 5
  !write(*,*) lbound(e,1), ubound(e,1) ! prints 1 6
 
  if (lbound(a,1) /= 1 .or. ubound(a,1) /= 3 &
      .or. lbound(b,1) /= 1 .or. ubound(b,1) /= 3 &
-     .or. lbound(c,1) /= 3 .or. ubound(c,1) /= 5 &
+     .or. lbound(c,1) /= 1 .or. ubound(c,1) /= 3 & 
      .or. lbound(d,1) /= 1 .or. ubound(d,1) /= 5 &
      .or. lbound(e,1) /= 1 .or. ubound(e,1) /= 6) then
    call abort()