Re: PR 60414: Patch proposal

Re: PR 60414: Patch proposal

[Dominique d'Humières]

Le 21 juil. 2014 à 15:03, Andre Vehreschild <vehre@gmx.de> a écrit :

> Hi Dominique,
> 
> thank you very much for your comments. I really appreciate them.

;-)

> Unfortunately I am contracted only for a limited number of around 6 bugs. The
> control on which bugs to pick is done by compiling a project and resolving the
> bugs that occur in order of their appearance.

If so, you should resolve (with your "client") the legal issues as soon as possible.

> I have reworked the test:
> 
> - results are now checked automatically against their expected value, and
> - unexpected results lead to an abort.
> 
> I hope this will do. The test for sure can be made more elegant, but my Fortran
> is to limited for that. I consider it pays more to invest my time into resolving
> bugs, then into writing most elegant test. What do you think?

The test is fine with me (I have regtested it). You don’t need the line

+! { dg-final { cleanup-modules "m" } }

This is done automatically by the gfortran testing machinery (I have checked it).

> The comment in interface.c is changed to:
> 
>  /* Only check ranks compatibility, when the actual argument is not a
>     reference of an array (foo(i)). A reference into an array is assumed
>     when actual->ref is non null. */
>  if (actual->ts.type == BT_CLASS && CLASS_DATA (actual)->as
>        && !actual->ref
> 	&& CLASS_DATA (actual)->as->rank == symbol_rank (formal))
>    return 1;

Although English is not my native language, I would prefer

/* Check ranks compatibility only when the actual argument is not a
   a reference to an element of the array, i.e., when actual->ref is null. */

Someone else should give you the formal approval.

Thanks for the patch,

Dominique

> The complete patch with the changes advised is attached in pr60414_3.patch.
> 
> Again, Bootstrapped and regextested on "x86_64-unknown-linux-gnu" w/o any
> regressions.
> 
> Regards,
> 	Andre
> -- 
> Andre Vehreschild * Kreuzherrenstr. 8 * 52062 Aachen
> Tel.: +49 241 9291018 * Email: vehre@gmx.de 
> <pr60414_3.patch>

Re: PR 60414: Patch proposal

[Mikael Morin]

Hello, thanks for your contribution.

here are some comments about the patch:

Le 21/07/2014 15:03, Andre Vehreschild a écrit :
> diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
> index c33936b..cb01a13 100644
> --- a/gcc/fortran/ChangeLog
> +++ b/gcc/fortran/ChangeLog
Changelogs are preferably provided outside of the patch as they almost
always conflict when applying it.

> @@ -1,3 +1,11 @@
> +2014-07-19  Andre Vehreschild <vehre@gmx.de>
> +
> +	PR fortran/60414
> +	* interface.c (compare_parameter): Fix compile bug: During resolution
> +	of generic an array reference in the actual argument was not
> +	respected. Fixed by checking, if the ref member is non-null.
You don't need to explain why (in the Changelog I mean), only _what_ is
changed should be there.

> Testcase
> +	unlimited_polymorphism_18.f90 add.
No need for that here; it's redundant with the testsuite Changelog.

> +
>  2014-06-15  Tobias Burnus  <burnus@net-b.de>
>
>  	* symbol.c (check_conflict): Add codimension conflict with
> diff --git a/gcc/fortran/interface.c b/gcc/fortran/interface.c
> index b210d18..d84c888 100644
> --- a/gcc/fortran/interface.c
> +++ b/gcc/fortran/interface.c
> @@ -2156,7 +2156,11 @@ compare_parameter (gfc_symbol *formal, gfc_expr
> *actual,
>    if (symbol_rank (formal) == actual->rank || symbol_rank (formal) == -1)
>      return 1;
>
> +  /* Only check ranks compatibility, when the actual argument is not a
> +     reference of an array (foo(i)). A reference into an array is assumed
> +     when actual->ref is non null. */
>    if (actual->ts.type == BT_CLASS && CLASS_DATA (actual)->as
> +        && !actual->ref
>  	&& CLASS_DATA (actual)->as->rank == symbol_rank (formal))
>      return 1;
>
I think you have spotted the right place where the bug happens, but the
patch provided is not completely right for the following reason:

actual->ref points to the beginning of the reference chain, so
for example if actual is the gfc_expr struct for "var%comp%vec",
actual->ref points to the gfc_ref struct for "comp".
Furthermore, you have to be aware that even bare array variables
without sub-reference get an implicit full array reference in the AST, so
if actual is the gfc_expr struct for "array_var", actual->ref is
non-null and points (indirectly) to a gfc_array_ref of type AR_FULL.

So if you want to check for the absence of trailing sub-reference, you
have to walk down to the last reference chain.
But then you have to also support the case "var%array_comp%vec(1)" which
is supposed to have the rank of array_comp.
In fact I think the conditional supposed to support the CLASS cases is
completely bogus, and I don't see why the generic conditional just above
it wouldn't also support CLASS cases just fine.
Did you try removing the CLASS conditional entirely?

Mikael

[PATCH, Fortran] PR fortran/60414 fix ICE was: PR 60414: Patch proposal

[Andre Vehreschild]

[-- Attachment #1: Type: text/plain, Size: 5104 bytes --]

Hi,

thanks for all the input. 

The issue to patch is an ICE while compiling a call of a generic method using an
array reference, e.g., this.Check(vec(1)) where Check aggregates an
implementation for an array parameter and one with a scalar parameter.

Based on Mikael's input, I analyzed the code further.
The original lines to consider are:

gcc/fortran/interface.c: 2155++
  /* If the rank is the same or the formal argument has assumed-rank.  */
  if (symbol_rank (formal) == actual->rank || symbol_rank (formal) == -1)
    return 1;

  if (actual->ts.type == BT_CLASS && CLASS_DATA (actual)->as
	&& CLASS_DATA (actual)->as->rank == symbol_rank (formal))
    return 1;

While the first conditional is checking the rank of the formal argument against
the rank *evaluated* for the actual argument, is the second one is checking the
actual argument's "un-refed" rank to the formal ones'. This results in the
later compile steps in an ICE, when the un-generic-ed function with the array
argument is used for the scalar argument.

I can't see what the purpose of the second conditional is (besides producing
the error now). The code is rather old and I think that with introduction of
generics, where the first conditional was introduced, the second one should
have been removed.

This is what the attached patch is for. The patch also adds a testcase for this
ICE.

*** gcc/fortran/Changelog ***
2014-08-06  Andre Vehreschild  <vehre@gmx.de>

        PR fortran/60414
        * interface.c (compare_parameter): Fixing ICE when argument 
	of a generic is a reference into an array.
*** gcc/fortran/Changelog ***

*** gcc/testsuite/Changelog ***
2014-08-06  Andre Vehreschild  <vehre@gmx.de>

        * gfortran.dg/unlimited_polymorphism_18.f90: Check according to
        PR fortran/60414
*** gcc/testsuite/Changelog ***

Bootstrapped and regtested on x86_64-unkown-linux-gnu.

Regards,
	Andre

On Sat, 26 Jul 2014 21:20:42 +0200
Mikael Morin <mikael.morin@sfr.fr> wrote:

> Hello, thanks for your contribution.
> 
> here are some comments about the patch:
> 
> Le 21/07/2014 15:03, Andre Vehreschild a écrit :
> > diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
> > index c33936b..cb01a13 100644
> > --- a/gcc/fortran/ChangeLog
> > +++ b/gcc/fortran/ChangeLog
> Changelogs are preferably provided outside of the patch as they almost
> always conflict when applying it.
> 
> > @@ -1,3 +1,11 @@
> > +2014-07-19  Andre Vehreschild <vehre@gmx.de>
> > +
> > +	PR fortran/60414
> > +	* interface.c (compare_parameter): Fix compile bug: During
> > resolution
> > +	of generic an array reference in the actual argument was not
> > +	respected. Fixed by checking, if the ref member is non-null.
> You don't need to explain why (in the Changelog I mean), only _what_ is
> changed should be there.
> 
> > Testcase
> > +	unlimited_polymorphism_18.f90 add.
> No need for that here; it's redundant with the testsuite Changelog.
> 
> > +
> >  2014-06-15  Tobias Burnus  <burnus@net-b.de>
> >
> >  	* symbol.c (check_conflict): Add codimension conflict with
> > diff --git a/gcc/fortran/interface.c b/gcc/fortran/interface.c
> > index b210d18..d84c888 100644
> > --- a/gcc/fortran/interface.c
> > +++ b/gcc/fortran/interface.c
> > @@ -2156,7 +2156,11 @@ compare_parameter (gfc_symbol *formal, gfc_expr
> > *actual,
> >    if (symbol_rank (formal) == actual->rank || symbol_rank (formal) == -1)
> >      return 1;
> >
> > +  /* Only check ranks compatibility, when the actual argument is not a
> > +     reference of an array (foo(i)). A reference into an array is assumed
> > +     when actual->ref is non null. */
> >    if (actual->ts.type == BT_CLASS && CLASS_DATA (actual)->as
> > +        && !actual->ref
> >  	&& CLASS_DATA (actual)->as->rank == symbol_rank (formal))
> >      return 1;
> >
> I think you have spotted the right place where the bug happens, but the
> patch provided is not completely right for the following reason:
> 
> actual->ref points to the beginning of the reference chain, so
> for example if actual is the gfc_expr struct for "var%comp%vec",
> actual->ref points to the gfc_ref struct for "comp".
> Furthermore, you have to be aware that even bare array variables
> without sub-reference get an implicit full array reference in the AST, so
> if actual is the gfc_expr struct for "array_var", actual->ref is
> non-null and points (indirectly) to a gfc_array_ref of type AR_FULL.
> 
> So if you want to check for the absence of trailing sub-reference, you
> have to walk down to the last reference chain.
> But then you have to also support the case "var%array_comp%vec(1)" which
> is supposed to have the rank of array_comp.
> In fact I think the conditional supposed to support the CLASS cases is
> completely bogus, and I don't see why the generic conditional just above
> it wouldn't also support CLASS cases just fine.
> Did you try removing the CLASS conditional entirely?
> 
> Mikael
> 


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

[-- Attachment #2: pr60414_5.patch --]
[-- Type: text/x-patch, Size: 2553 bytes --]

diff --git a/gcc/fortran/interface.c b/gcc/fortran/interface.c
index b210d18..9eddcdf 100644
--- a/gcc/fortran/interface.c
+++ b/gcc/fortran/interface.c
@@ -2156,10 +2156,6 @@ compare_parameter (gfc_symbol *formal, gfc_expr *actual,
   if (symbol_rank (formal) == actual->rank || symbol_rank (formal) == -1)
     return 1;
 
-  if (actual->ts.type == BT_CLASS && CLASS_DATA (actual)->as
-	&& CLASS_DATA (actual)->as->rank == symbol_rank (formal))
-    return 1;
-
   rank_check = where != NULL && !is_elemental && formal->as
 	       && (formal->as->type == AS_ASSUMED_SHAPE
 		   || formal->as->type == AS_DEFERRED)
diff --git a/gcc/testsuite/gfortran.dg/unlimited_polymorphic_18.f90 b/gcc/testsuite/gfortran.dg/unlimited_polymorphic_18.f90
new file mode 100644
index 0000000..9044199
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/unlimited_polymorphic_18.f90
@@ -0,0 +1,69 @@
+! { dg-do compile }
+! Testing fix for 
+! PR fortran/60414 
+!
+module m
+    implicit none
+    Type T
+        real, public :: expectedScalar;
+    contains
+        procedure :: FCheck
+        procedure :: FCheckArr
+        generic :: Check => FCheck, FCheckArr
+    end Type
+
+contains
+
+    subroutine FCheck(this,X)
+        class(T) this
+        class(*) X
+        real :: r
+        select type (X)
+            type is (real)
+                if ( abs (X - this%expectedScalar) > 0.0001 ) then
+                    call abort()
+                end if
+            class default 
+                call abort ()
+         end select
+    end subroutine FCheck
+
+    subroutine FCheckArr(this,X)
+        class(T) this
+        class(*) X(:)
+        integer i
+        do i = 1,6
+            this%expectedScalar = i - 1.0
+            call this%FCheck(X(i))
+        end do
+    end subroutine FCheckArr
+
+    subroutine CheckTextVector(vec, n, scal)
+        integer, intent(in) :: n
+        class(*), intent(in) :: vec(n)
+        class(*), intent(in) :: scal
+        integer j
+        Type(T) :: Tester
+
+        ! Check full vector
+        call Tester%Check(vec)
+        ! Check a scalar of the same class like the vector
+        Tester%expectedScalar = 5.0
+        call Tester%Check(scal)
+        ! Check an element of the vector, which is a scalar
+        j=3
+        Tester%expectedScalar = 2.0
+        call Tester%Check(vec(j))
+
+    end subroutine CheckTextVector
+
+end module
+
+program test
+   use :: m
+   implicit none
+  
+   real :: vec(1:6) = (/ 0, 1, 2, 3, 4, 5 /)
+   call checktextvector(vec, 6, 5.0)
+end program test 
+

Re: [PATCH, Fortran] PR fortran/60414 fix ICE was: PR 60414: Patch proposal

[Mikael Morin]

Hello,

Le 06/08/2014 21:23, Andre Vehreschild a écrit :
> Hi,
> 
[...]
> 
> *** gcc/fortran/Changelog ***
> 2014-08-06  Andre Vehreschild  <vehre@gmx.de>
> 
>         PR fortran/60414
>         * interface.c (compare_parameter): Fixing ICE when argument 
> 	of a generic is a reference into an array.
> *** gcc/fortran/Changelog ***
The ChangeLog format is good, but the text is not very
helpful/descriptive for someone hunting a bug in compare_parameter in
the future.

You can say (for example):
Remove class argument rank check short circuit.

> 
> *** gcc/testsuite/Changelog ***
> 2014-08-06  Andre Vehreschild  <vehre@gmx.de>
> 
>         * gfortran.dg/unlimited_polymorphism_18.f90: Check according to
>         PR fortran/60414
> *** gcc/testsuite/Changelog ***
You should add PR fortran/60414 before like in the gcc/fortran Changelog,
and then the text just need to say new/new file/new test (see what the
other contributors use in the rest of the file).

> 
> Bootstrapped and regtested on x86_64-unkown-linux-gnu.
> 
The patch looks good to me.
With the ChangeLog fixes above, OK if/when the copyright assignment is
settled.

Thanks
Mikael

Re: [PATCH, Fortran] PR fortran/60414 fix ICE was: PR 60414: Patch proposal

[Dominique Dhumieres]

As Mikael said in https://gcc.gnu.org/ml/fortran/2014-08/msg00047.html

> the testcase should check that the code generated is actually working,
> not just that the ICE disappeared. ...

thus I think the test should be run, i.e., '! { dg-do compile }' should
be replaced with '! { dg-do run }' (I have checked that the test succeeds).

Dominique

Re: [PATCH, Fortran] PR fortran/60414 fix ICE was: PR 60414: Patch proposal

[Mikael Morin]

Le 17/08/2014 14:26, Dominique Dhumieres a écrit :
> As Mikael said in https://gcc.gnu.org/ml/fortran/2014-08/msg00047.html
> 
>> the testcase should check that the code generated is actually working,
>> not just that the ICE disappeared. ...
> 
Well, this is for another patch where deferred character variable are
made acceptable as argument to unlimited polymorphic dummies.
Here the ICE comes (if I remember correctly) from the wrong generic
procedure being picked, so there is not really some new feature enabled
with the patch.

> thus I think the test should be run, i.e., '! { dg-do compile }' should
> be replaced with '! { dg-do run }' (I have checked that the test succeeds).
> 
I don't have a strong opinion for it, but I'm OK with that change.
In fact the initial test was a run one, and it has been changed to
compile.  Andre: why?

Mikael

Re: [PATCH, Fortran] PR fortran/60414 fix ICE was: PR 60414: Patch proposal

[Andre Vehreschild]

Hi,

On Sun, 17 Aug 2014 15:06:02 +0200
Mikael Morin <mikael.morin@sfr.fr> wrote:

> Le 17/08/2014 14:26, Dominique Dhumieres a écrit :
> > As Mikael said in https://gcc.gnu.org/ml/fortran/2014-08/msg00047.html
> > 
> >> the testcase should check that the code generated is actually working,
> >> not just that the ICE disappeared. ...
> > 
> Well, this is for another patch where deferred character variable are
> made acceptable as argument to unlimited polymorphic dummies.
> Here the ICE comes (if I remember correctly) from the wrong generic
> procedure being picked, so there is not really some new feature enabled
> with the patch.

This is correct so far. 

> 
> > thus I think the test should be run, i.e., '! { dg-do compile }' should
> > be replaced with '! { dg-do run }' (I have checked that the test succeeds).
> > 
> I don't have a strong opinion for it, but I'm OK with that change.
> In fact the initial test was a run one, and it has been changed to
> compile.  Andre: why?

I was asked to move to compile only, because a run test takes a lot of time.
I was told that the run test compiles the code multiple times with different
optimization. This issue was deemed to be solely on the compile stage and was
not influenced by optimization. Therefore I agreed to switch to dg-do compile.
That the test is fine for running, too, is merely for my training of how to do
that. My opinion is, that dg-do compile is sufficient to prove, that PR60414 is
resolved, because that is the sole purpose of the patch. I understand
Dominique wanting to have the dg-do run, because the effectiveness of the patch
is only shown on running the test. Is there a compromise of running a test, but
only for one optimization stage? Then may be we can do that.

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

Re: [PATCH, Fortran] PR fortran/60414 fix ICE was: PR 60414: Patch proposal

[Andre Vehreschild]

[-- Attachment #1: Type: text/plain, Size: 2192 bytes --]

Hi, 

this patch is ready for commit now. Please apply. There have been no objections
against doing dg-do compile only, since my last post in August.

- Andre

On Tue, 26 Aug 2014 11:30:12 +0200
Andre Vehreschild <vehre@gmx.de> wrote:

> Hi,
> 
> On Sun, 17 Aug 2014 15:06:02 +0200
> Mikael Morin <mikael.morin@sfr.fr> wrote:
> 
> > Le 17/08/2014 14:26, Dominique Dhumieres a écrit :
> > > As Mikael said in https://gcc.gnu.org/ml/fortran/2014-08/msg00047.html
> > > 
> > >> the testcase should check that the code generated is actually working,
> > >> not just that the ICE disappeared. ...
> > > 
> > Well, this is for another patch where deferred character variable are
> > made acceptable as argument to unlimited polymorphic dummies.
> > Here the ICE comes (if I remember correctly) from the wrong generic
> > procedure being picked, so there is not really some new feature enabled
> > with the patch.
> 
> This is correct so far. 
> 
> > 
> > > thus I think the test should be run, i.e., '! { dg-do compile }' should
> > > be replaced with '! { dg-do run }' (I have checked that the test
> > > succeeds).
> > > 
> > I don't have a strong opinion for it, but I'm OK with that change.
> > In fact the initial test was a run one, and it has been changed to
> > compile.  Andre: why?
> 
> I was asked to move to compile only, because a run test takes a lot of time.
> I was told that the run test compiles the code multiple times with different
> optimization. This issue was deemed to be solely on the compile stage and was
> not influenced by optimization. Therefore I agreed to switch to dg-do compile.
> That the test is fine for running, too, is merely for my training of how to do
> that. My opinion is, that dg-do compile is sufficient to prove, that PR60414
> is resolved, because that is the sole purpose of the patch. I understand
> Dominique wanting to have the dg-do run, because the effectiveness of the
> patch is only shown on running the test. Is there a compromise of running a
> test, but only for one optimization stage? Then may be we can do that.
> 
> - Andre


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

[-- Attachment #2: pr60414_6.clg --]
[-- Type: application/octet-stream, Size: 426 bytes --]

*** gcc/fortran/Changelog ***
2014-12-03  Andre Vehreschild  <vehre@gmx.de>

        PR fortran/60414
        * interface.c (compare_parameter): Remove class argument rank
	check short circuit.

*** gcc/fortran/Changelog ***

*** gcc/testsuite/Changelog ***
2014-12-03  Andre Vehreschild  <vehre@gmx.de>

        PR fortran/60414
        * gfortran.dg/unlimited_polymorphism_18.f90: New test.

*** gcc/testsuite/Changelog ***

[-- Attachment #3: pr60414_6.patch --]
[-- Type: text/x-patch, Size: 2547 bytes --]

diff --git a/gcc/fortran/interface.c b/gcc/fortran/interface.c
index 2429fd2..c8f61e1 100644
--- a/gcc/fortran/interface.c
+++ b/gcc/fortran/interface.c
@@ -2156,10 +2156,6 @@ compare_parameter (gfc_symbol *formal, gfc_expr *actual,
   if (symbol_rank (formal) == actual->rank || symbol_rank (formal) == -1)
     return 1;
 
-  if (actual->ts.type == BT_CLASS && CLASS_DATA (actual)->as
-	&& CLASS_DATA (actual)->as->rank == symbol_rank (formal))
-    return 1;
-
   rank_check = where != NULL && !is_elemental && formal->as
 	       && (formal->as->type == AS_ASSUMED_SHAPE
 		   || formal->as->type == AS_DEFERRED)
diff --git a/gcc/testsuite/gfortran.dg/unlimited_polymorphic_18.f90 b/gcc/testsuite/gfortran.dg/unlimited_polymorphic_18.f90
new file mode 100644
index 0000000..7a0df1a
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/unlimited_polymorphic_18.f90
@@ -0,0 +1,69 @@
+! { dg-do compile }
+! Testing fix for
+! PR fortran/60414
+!
+module m
+    implicit none
+    Type T
+        real, public :: expectedScalar;
+    contains
+        procedure :: FCheck
+        procedure :: FCheckArr
+        generic :: Check => FCheck, FCheckArr
+    end Type
+
+contains
+
+    subroutine FCheck(this,X)
+        class(T) this
+        class(*) X
+        real :: r
+        select type (X)
+            type is (real)
+                if ( abs (X - this%expectedScalar) > 0.0001 ) then
+                    call abort()
+                end if
+            class default
+                call abort ()
+         end select
+    end subroutine FCheck
+
+    subroutine FCheckArr(this,X)
+        class(T) this
+        class(*) X(:)
+        integer i
+        do i = 1,6
+            this%expectedScalar = i - 1.0
+            call this%FCheck(X(i))
+        end do
+    end subroutine FCheckArr
+
+    subroutine CheckTextVector(vec, n, scal)
+        integer, intent(in) :: n
+        class(*), intent(in) :: vec(n)
+        class(*), intent(in) :: scal
+        integer j
+        Type(T) :: Tester
+
+        ! Check full vector
+        call Tester%Check(vec)
+        ! Check a scalar of the same class like the vector
+        Tester%expectedScalar = 5.0
+        call Tester%Check(scal)
+        ! Check an element of the vector, which is a scalar
+        j=3
+        Tester%expectedScalar = 2.0
+        call Tester%Check(vec(j))
+
+    end subroutine CheckTextVector
+
+end module
+
+program test
+   use :: m
+   implicit none
+
+   real :: vec(1:6) = (/ 0, 1, 2, 3, 4, 5 /)
+   call checktextvector(vec, 6, 5.0)
+end program test
+

Re: [PATCH, Fortran] PR fortran/60414 fix ICE was: PR 60414: Patch proposal

[Dominique d'Humières]

> Le 3 déc. 2014 à 18:08, Andre Vehreschild <vehre@gmx.de> a écrit :
> 
> Hi, 
> 
> this patch is ready for commit now. Please apply. There have been no objections
> against doing dg-do compile only, since my last post in August.

Not really true, I do have objections, but I won’t fight for them. I still think the test should be dg-do run.

Dominique

> - Andre
> 
> On Tue, 26 Aug 2014 11:30:12 +0200
> Andre Vehreschild <vehre@gmx.de> wrote:
> 
>> Hi,
>> 
>> On Sun, 17 Aug 2014 15:06:02 +0200
>> Mikael Morin <mikael.morin@sfr.fr> wrote:
>> 
>>> Le 17/08/2014 14:26, Dominique Dhumieres a écrit :
>>>> As Mikael said in https://gcc.gnu.org/ml/fortran/2014-08/msg00047.html
>>>> 
>>>>> the testcase should check that the code generated is actually working,
>>>>> not just that the ICE disappeared. ...
>>>> 
>>> Well, this is for another patch where deferred character variable are
>>> made acceptable as argument to unlimited polymorphic dummies.
>>> Here the ICE comes (if I remember correctly) from the wrong generic
>>> procedure being picked, so there is not really some new feature enabled
>>> with the patch.
>> 
>> This is correct so far. 
>> 
>>> 
>>>> thus I think the test should be run, i.e., '! { dg-do compile }' should
>>>> be replaced with '! { dg-do run }' (I have checked that the test
>>>> succeeds).
>>>> 
>>> I don't have a strong opinion for it, but I'm OK with that change.
>>> In fact the initial test was a run one, and it has been changed to
>>> compile.  Andre: why?
>> 
>> I was asked to move to compile only, because a run test takes a lot of time.
>> I was told that the run test compiles the code multiple times with different
>> optimization. This issue was deemed to be solely on the compile stage and was
>> not influenced by optimization. Therefore I agreed to switch to dg-do compile.
>> That the test is fine for running, too, is merely for my training of how to do
>> that. My opinion is, that dg-do compile is sufficient to prove, that PR60414
>> is resolved, because that is the sole purpose of the patch. I understand
>> Dominique wanting to have the dg-do run, because the effectiveness of the
>> patch is only shown on running the test. Is there a compromise of running a
>> test, but only for one optimization stage? Then may be we can do that.
>> 
>> - Andre
> 
> 
> -- 
> Andre Vehreschild * Email: vehre ad gmx dot de 
> <pr60414_6.clg><pr60414_6.patch>

Re: [PATCH, Fortran] PR fortran/60414 fix ICE was: PR 60414: Patch proposal

[FX]

> this patch is ready for commit now. Please apply. There have been no objections
> against doing dg-do compile only, since my last post in August.

I wasn’t back on active duty in August, so I didn’t follow that. But if the only argument for compile-test over run-test is the runtime, then I don’t think it should matter. First, testsuite is now much more efficiently parallelized than before. Second, testsuite is already so large that one more test isn’t gonna change things much.

FX

Re: [PATCH, Fortran] PR fortran/60414 fix ICE was: PR 60414: Patch proposal

[Dominique Dhumieres]

> this patch is ready for commit now. Please apply. There have been no objections
> against doing dg-do compile only, since my last post in August.

Since I am stubborn, I have made the test 'dg-do run' and committed the patch
as revision r218422.

Thanks for the patch,

Dominique