[flang] Modifying dummy argument via its global actual argument failed for array case with 2 or more elements

[DanielCChen]

Consider the following code:

module m
    type base
        integer*4 :: id = -1
        contains
        procedure :: print => printBase
        procedure :: increaseID => increaseBaseID
    end type

    type, extends (base) :: child
        character*20 :: name = 'default'
        contains
        procedure :: print => printChild
    end type

    class (base), pointer :: b1_m (:)

    type (child), save, target :: c1_m (1)
    type (child), save, target :: c2_m (2)

    class (base), pointer :: b1_m_s
    type (child), save, target :: c1_m_s

    contains
    elemental subroutine increaseBaseID (b, i1)
        class (base), intent(inout) :: b
        integer*4, intent(in) :: i1

        b%id = b%id + i1
    end subroutine

    subroutine printBase (b)
        class (base), intent(in) :: b

        print *, b%id
    end subroutine

    subroutine printChild (b)
        class (child), intent(in) :: b

        print *, b%id, b%name
    end subroutine

    subroutine test1 (b)
        type (base), target, intent(inout) :: b (:)

        call b1_m%increaseID (10)
        call b(1)%print
    end subroutine

    subroutine test1_s (b)
        type (base), target, intent(inout) :: b

        call b1_m_s%increaseID (10)
        call b%print
    end subroutine
end module


program fArg033a3
use m

    c1_m = (/child (1,'c1_m')/)
    c2_m = (/(child (i,'c1_m'), i=1,2)/)

    b1_m => c2_m              !! ERROR case: array of 2 elements
    call test1 (b1_m)

    b1_m => c1_m              !! WORKING case: array of 1 element
    call test1 (b1_m)

    c1_m_s = child (1,'c1_m') !! WORKING case: scalar
    b1_m_s => c1_m_s
    call test1_s (b1_m_s)
end

Flang failed to update the global variable c2_m that is an array of 2 elements in the ERROR case.
It works fine for the c1_m case that is an array of 1 element.
It works fine for the c1_m_s, the scalar case.

Flang outputs

> a.out
 1
 11
 11

Expected output

> a.out
 11
 11
 11

[llvmbot]

@llvm/issue-subscribers-flang-frontend

Author: Daniel Chen (DanielCChen)

Consider the following code: ``` module m type base integer*4 :: id = -1 contains procedure :: print => printBase procedure :: increaseID => increaseBaseID end type

type, extends (base) :: child
    character*20 :: name = 'default'
    contains
    procedure :: print => printChild
end type

class (base), pointer :: b1_m (:)

type (child), save, target :: c1_m (1)
type (child), save, target :: c2_m (2)

class (base), pointer :: b1_m_s
type (child), save, target :: c1_m_s

contains
elemental subroutine increaseBaseID (b, i1)
    class (base), intent(inout) :: b
    integer*4, intent(in) :: i1

    b%id = b%id + i1
end subroutine

subroutine printBase (b)
    class (base), intent(in) :: b

    print *, b%id
end subroutine

subroutine printChild (b)
    class (child), intent(in) :: b

    print *, b%id, b%name
end subroutine

subroutine test1 (b)
    type (base), target, intent(inout) :: b (:)

    call b1_m%increaseID (10)
    call b(1)%print
end subroutine

subroutine test1_s (b)
    type (base), target, intent(inout) :: b

    call b1_m_s%increaseID (10)
    call b%print
end subroutine

end module

program fArg033a3
use m

c1_m = (/child (1,'c1_m')/)
c2_m = (/(child (i,'c1_m'), i=1,2)/)

b1_m => c2_m              !! ERROR case: array of 2 elements
call test1 (b1_m)

b1_m => c1_m              !! WORKING case: array of 1 element
call test1 (b1_m)

c1_m_s = child (1,'c1_m') !! WORKING case: scalar
b1_m_s => c1_m_s
call test1_s (b1_m_s)

end

Flang failed to update the global variable `c2_m` that is an array of 2 elements in the ERROR case. 
It works fine for the `c1_m` case that is an array of 1 element.
It works fine for the `c1_m_s`, the scalar case.

Flang outputs

> a.out
1
11
11

Expected output

> a.out
11
11
11

</details>

[klausler]

The program's seeming violation of the usual dummy argument aliasing rules is conforming under 15.5.2.14 p1 (3)(b), since the dummy argument of test1 is TARGET, INTENT(IN OUT), and assumed-shape, and the actual argument b1_m is a pointer and thus also a target.

Assigning to Jean for confirmation of this analysis and possible investigation.

[DanielCChen]

My reading of that restriction is that changing the value of the actual argument would be invalid if 15.5.2.14 p1 (3)(b) is NOT true becuase the restriction says:

(3) Action that affects the value of the entity or any subobject of it shall be taken only through the
dummy argument **unless**

(b) the dummy argument is a scalar, **assumed-shape**, or assumed-rank object, and **has the TARGET
attribute but not the INTENT (IN) or CONTIGUOUS attributes**, and the **actual argument
is a target** other than a coindexed object or an array section with a vector subscript,

[klausler]

My reading of that restriction is that changing the value of the actual argument would be invalid if 15.5.2.14 p1 (3)(b) is NOT true becuase the restriction says:

(3) Action that affects the value of the entity or any subobject of it shall be taken only through the
dummy argument **unless**

(b) the dummy argument is a scalar, **assumed-shape**, or assumed-rank object, and **has the TARGET
attribute but not the INTENT (IN) or CONTIGUOUS attributes**, and the **actual argument
is a target** other than a coindexed object or an array section with a vector subscript,

The standard is saying that there are circumstances where a TARGET dummy argument is not the only name for a data object in its scope; it can alias with pointers when the actual argument was a target or pointer, and we're guaranteed to not be in a copy-in situation.

[DanielCChen]

I agree.

I think the pointer actual argument b1_m is pointer associated with assumed-shape target dummy argument b(:), so no copy-in is involved in this case. The code should be valid.

Flang has the expected output when the array size of the actual argument target is 1. Not sure if it is intentional though.

[jeanPerier]

I agree no copy-in/copy-out should be made here. @eugeneepshteyn, this looks like a regression from 4b6a4aa. Can you have a look?

I do not understand why the new MayNeedCopy is returning true for cases where check.HavePolymorphicDifferences()).

I also think the check is doing a bit too much, it is only relevant to call when the actual is a variable and the dummy is not a VALUE argument (I find it weird that it is returning true for the VALUE case and for expression actual. These are different situations for which we do not want to generate the conditional copy logic that copy-in/copy-out does and is purely driven by runtime contiguity aspects ).

[eugeneepshteyn]

I do not understand why the new MayNeedCopy is returning true for cases where check.HavePolymorphicDifferences())

Will fix.

I also think the check is doing a bit too much, it is only relevant to call when the actual is a variable and the dummy is not a VALUE argument (I find it weird that it is returning true for the VALUE case and for expression actual. These are different situations for which we do not want to generate the conditional copy logic that copy-in/copy-out does and is purely driven by runtime contiguity aspects ).

Let's find specific cases where this may be broken and I'll either fix them or add tests to ensure they don't break. MayNeedCopy() does more general check that can be used both by lowering and by other semantic checks. The lowering case is currently only used for arrays only and I can add further restrictions if necessary.