diff mbox

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

Message ID 20141203180824.3f15a063@gmx.de
State New
Headers show

Commit Message

Andre Vehreschild Dec. 3, 2014, 5:08 p.m. UTC
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

Comments

Dominique d'Humières Dec. 3, 2014, 5:12 p.m. UTC | #1
> 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>
FX Coudert Dec. 3, 2014, 9:49 p.m. UTC | #2
> 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
diff mbox

Patch

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
+