I'm trying to do an lto build of CP2K, but this fails with: /data03/vondele/clean/cp2k/makefiles/../src/qs_linres_current.F: In function ‘calculate_jrho_resp’: /data03/vondele/clean/cp2k/makefiles/../src/qs_linres_current.F:614:0: error: non-trivial conversion at assignment struct array3_real(kind=8) struct array3_real(kind=8) # .MEM_3681 = VDEF <.MEM_3286> my_rho = D.13693_938->r; /data03/vondele/clean/cp2k/makefiles/../src/qs_linres_current.F:614:0: error: non-trivial conversion at assignment struct array3_real(kind=8) struct array3_real(kind=8) # .MEM_3682 = VDEF <.MEM_3681> my_current = D.13701_946->r; /data03/vondele/clean/cp2k/makefiles/../src/qs_linres_current.F:614:0: error: non-trivial conversion at assignment struct array3_real(kind=8) struct array3_real(kind=8) # .MEM_3750 = VDEF <.MEM_3287> my_gauge = D.14057_1326->r; /data03/vondele/clean/cp2k/makefiles/../src/qs_linres_current.F:614:0: internal compiler error: verify_stmts failed Please submit a full bug report. Not sure how to get a testcase for this one.... It appears to be for example this pointer assignment my_gauge=>rs_gauge(1)%rs(igrid_level)%rs_grid%r where REAL(dp), DIMENSION(:, :, :), POINTER :: my_gauge and r is: TYPE realspace_grid_type REAL(KIND=dp), DIMENSION ( :, :, : ), ALLOCATABLE :: r ! the grid END TYPE realspace_grid_type
Simple testcase (gfortran -flto test.f90): MODULE M1 INTEGER, PARAMETER :: dp=8 TYPE realspace_grid_type REAL(KIND=dp), DIMENSION ( :, :, : ), ALLOCATABLE :: r END TYPE realspace_grid_type END MODULE MODULE M2 USE m1 CONTAINS SUBROUTINE S1(x) TYPE(realspace_grid_type), POINTER :: x REAL(dp), DIMENSION(:, :, :), POINTER :: y y=>x%r y=0 END SUBROUTINE END MODULE USE M2 TYPE(realspace_grid_type), POINTER :: x ALLOCATE(x) ALLOCATE(x%r(10,10,10)) CALL S1(x) write(6,*) x%r END
Actually works with 4.5 but fails with trunk
Confirmed. We have two different array3_real(kind=8) record types that are not considered compatible. One data pointer member is restrict qualified while the other one is not. Why do we have an aggregate assignment of an array descriptor anyway? struct array3_real(kind=8) y; struct realspace_grid_type * D.2157; <bb 2>: y.data = 0B; D.2157_5 = *x_4(D); y = D.2157_5->r; D.2172_6 = y.data; The issue here is of course that LTO re-computes TYPE_CANONICAL and the FE sets it in a way that the above situation is not detected as non-trivial conversion.
If realspace_grid_type%r is declared as pointer instead of allocatable, the testcase can be compiled, and a similar workaround in CP2K allows an LTO build of CP2K to finish (after updating binutils to its CVS version).
(In reply to comment #3) > The issue here is of course that LTO re-computes TYPE_CANONICAL and the FE > sets it in a way that the above situation is not detected as non-trivial > conversion. Does the fix need to be done in the FE, or will this be addressed by changes to LTO? Actually, looks like there might be some vaguely related issue here in the FE, which I'll open in another PR.
(In reply to comment #5) > Actually, looks like there might be some vaguely related issue here in the FE, > which I'll open in another PR. See PR45777
Mine.
Created attachment 22107 [details] patch Bootstrapped, tested and SPEC CPU 2006 tested. I don't like it too much (it's really a FE hack we put in place that haunts us now ...). Deferred installing to stage3 or later (--enable-checking=release will get you an equivalent solution).
The issue is that realspace_grid_type%r is marked as restrict, but y is a POINTER and y => x%r associates y with that allocatable even though it doesn't have TARGET attribute. This points at either a FE bug, an invalid testcase. The FE bug would be that allocatables are not really restrict as they can be aliased. The invalid testcase would be that doing y => x%r isn't valid (but gfortran doesn't diagnose that, so also a FE bug). Can someone clarify why or if it is legal to associate y with x%r? If declaring x with POINTER attribute makes it legal then we need to form a type variant for realspace_grid_type, re-using the original type isn't conformant with middle-end expectations. FE again for now, atm I think this is a possible generic wrong-code problem.
Thus, isn't what the program does equivalent to REAL(dp), DIMENSION(:, :, :), ALLOCATABLE :: z REAL(dp), DIMENSION(:, :, :), POINTER :: y y=>z which is invalid? Valid would only be REAL(dp), DIMENSION(:, :, :), ALLOCATABLE, TARGET :: z REAL(dp), DIMENSION(:, :, :), POINTER :: y y=>z ?
(In reply to comment #10) > Thus, isn't what the program does equivalent to > > REAL(dp), DIMENSION(:, :, :), ALLOCATABLE :: z > REAL(dp), DIMENSION(:, :, :), POINTER :: y > y=>z > > which is invalid? Valid would only be > > REAL(dp), DIMENSION(:, :, :), ALLOCATABLE, TARGET :: z > REAL(dp), DIMENSION(:, :, :), POINTER :: y > y=>z > > ? no, your example here is different, and is not allowed. The original testcase is fine. so y=>a%b%c%d%z is allowed as soon as any of a, b, c, or d or z have the pointer attribute. That z is allocatable doesn't matter.
The following needs to be taken into account when determining the validity: If this use is supposed to be valid (we can associate a pointer with and entity that isn't marked TARGET but merely ALLOCATABLE) then the distinction between TARGET and ALLOCATABLE (the the language designer introduced most probably for a reason) is more or less mood. GemsFDTD and other benchmarks will regress by a large amount. So, this testcase should better be invalid due to the missing TARGET attribute.
> no, your example here is different, and is not allowed. The original > testcase is fine. > > so y=>a%b%c%d%z > > is allowed as soon as any of a, b, c, or d or z have the pointer attribute. > That z is allocatable doesn't matter. Ugh. That might be terrible. Can you point to some language in the standard supporting that (I haven't looked myself and am not too fluent in the fortran standard).
(In reply to comment #13) > Ugh. That might be terrible. Can you point to some language in the standard > supporting that (I haven't looked myself and am not too fluent in the fortran > standard). I'm afraid that finding this in the standard is not quite easy. I guess the idea is that you point to something which is a subobject of variable with pointer attribute. Maybe Tobias can find this? The closest I come is actually in IBM's compiler manual: http://publib.boulder.ibm.com/infocenter/cellcomp/v9v111/index.jsp?topic=/com.ibm.xlf111.cell.doc/xlflr/f90pass.htm target is a variable or expression. If it is a variable, it must have the TARGET attribute (or be a subobject of such an object) or the POINTER attribute. It isn't that terrible, I think. you still can't point to allocatable arrays, if e.g. they are local variables, or just part of derived types that do not have the pointer attribute.
You then need to make sure to create variant types of aggregates with the target attribute applied to all subtypes (thus, the restrict stuff removed) as the middle-end doesn't know about this rule.
I think this implies this bug depends in somehow on PR45777.
(In reply to comment #16) > I think this implies this bug depends in somehow on PR45777. Yes indeed - that bug looks very much related.
(In reply to comment #13) > Ugh. That might be terrible. Can you point to some language in the standard > supporting that (I haven't looked myself and am not too fluent in the fortran > standard). (All relative to Fortran 2008, ftp://ftp.nag.co.uk/sc22wg5/N1801-N1850/N1830.pdf) "5.3.17 TARGET attribute" [...] "If an object has the TARGET attribute, then all of its nonpointer subobjects also have the TARGET attribute." The pointer part probably enters indirectly: Assume in "a%b%p%d", "p" is the pointer than "a%b%p%d" denotes the "d" components of the target to which "a%b%p" points to. Per construction the target of the pointer has the TARGET attribute - and "d" has it via the rule above. Note, however, that the TARGET issue is local. Example: type t integer :: i end type t type(t) :: a integer, pointer :: ptr call sub(a) ptr = 7 ! Invalid, ptr has unknown association ! status as "a" is not a TARGET contains subroutine sub(x) type(a), TARGET :: x ptr => x%i ! OK, x and thus x%i is a TARGET ptr = 5 end subroutine end Reason: "If the dummy argument has the TARGET attribute and the effective argument does not have the TARGET attribute [...], any pointers associated with the dummy argument become undefined when execution of the procedure completes." (Cf. "12.5.2.4 Ordinary dummy variables".) See also: "16.5.2.5 Events that cause the association status of pointers to become undefined". (In reply to comment #15) > You then need to make sure to create variant types of aggregates with the > target attribute applied to all subtypes (thus, the restrict stuff removed) > as the middle-end doesn't know about this rule. Yes, that sounds sensible - even though it will be a lot of work and requires quite some book keeping :-( That's somehow the curse of -fwhole-file: Now the decls are sensible enough that the ME has plenty of opportunities to exploit all the wrong decl the FE generates... (In reply to comment #16) > I think this implies this bug depends in somehow on PR45777. Maybe. One operates on gfc_expr level - the other on tree level; however, both are about the same issue. I assume they independent of each other.
Tobias, thanks for the clean explanation. I overlooked that the target of a pointer has that target attribute (seems logical!). Richard, I tried to get to a testcase for which the ME generates wrong code, but somehow things are always 'right'. I was expecting this to fail (-O3 -fno-inline), since the ME should not now that X aliases with V1%D: MODULE M1 IMPLICIT NONE TYPE T1 REAL, DIMENSION(:), ALLOCATABLE :: D END TYPE T1 CONTAINS SUBROUTINE S1(V1) TYPE(T1), POINTER :: V1 REAL, DIMENSION(:), POINTER :: X INTEGER :: I CALL PA(V1,X) DO i=1,4 V1%D(i)=1 X(i)=2 V1%D(i)=2*V1%D(i) ENDDO END SUBROUTINE S1 SUBROUTINE PA(V1,X) TYPE(T1), POINTER :: V1 REAL, DIMENSION(:), POINTER :: X X=>V1%D END SUBROUTINE END MODULE M1 USE M1 IMPLICIT NONE TYPE(T1), POINTER :: V1 INTEGER :: i ALLOCATE(V1) ALLOCATE(V1%D(4)) V1%D=(/(i,i=1,4)/) CALL S1(V1) IF (ANY(V1%D.NE.4)) STOP "BUG" write(6,*) "ALL IS FINE" END
On Fri, 26 Nov 2010, Joost.VandeVondele at pci dot uzh.ch wrote: > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45586 > > --- Comment #19 from Joost VandeVondele <Joost.VandeVondele at pci dot uzh.ch> 2010-11-26 13:39:00 UTC --- > Tobias, thanks for the clean explanation. I overlooked that the target of a > pointer has that target attribute (seems logical!). > > Richard, I tried to get to a testcase for which the ME generates wrong code, > but somehow things are always 'right'. I was expecting this to fail (-O3 > -fno-inline), since the ME should not now that X aliases with V1%D: > > MODULE M1 > IMPLICIT NONE > TYPE T1 > REAL, DIMENSION(:), ALLOCATABLE :: D > END TYPE T1 > CONTAINS > SUBROUTINE S1(V1) > TYPE(T1), POINTER :: V1 We can't exploit restrict information for pointers, it would need to be a var of type t1 with target attribute - but even then the double indirection (v1 passed by reference, embedded array desc points to data) prevents us from exploiting restrict info. So we might be safe for now (apart from the related ICE seen with lto). > REAL, DIMENSION(:), POINTER :: X > INTEGER :: I > CALL PA(V1,X) > DO i=1,4 > V1%D(i)=1 > X(i)=2 > V1%D(i)=2*V1%D(i) > ENDDO > END SUBROUTINE S1 > SUBROUTINE PA(V1,X) > TYPE(T1), POINTER :: V1 > REAL, DIMENSION(:), POINTER :: X > X=>V1%D > END SUBROUTINE > END MODULE M1 > > USE M1 > IMPLICIT NONE > TYPE(T1), POINTER :: V1 > INTEGER :: i > ALLOCATE(V1) > ALLOCATE(V1%D(4)) > V1%D=(/(i,i=1,4)/) > CALL S1(V1) > IF (ANY(V1%D.NE.4)) STOP "BUG" > write(6,*) "ALL IS FINE" > END > >
(In reply to comment #20) > On Fri, 26 Nov 2010, Joost.VandeVondele at pci dot uzh.ch wrote: > > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45586 > > > > --- Comment #19 from Joost VandeVondele <Joost.VandeVondele at pci dot uzh.ch> 2010-11-26 13:39:00 UTC --- > > Tobias, thanks for the clean explanation. I overlooked that the target of a > > pointer has that target attribute (seems logical!). > > > > Richard, I tried to get to a testcase for which the ME generates wrong code, > > but somehow things are always 'right'. I was expecting this to fail (-O3 > > -fno-inline), since the ME should not now that X aliases with V1%D: > > > > MODULE M1 > > IMPLICIT NONE > > TYPE T1 > > REAL, DIMENSION(:), ALLOCATABLE :: D > > END TYPE T1 > > CONTAINS > > SUBROUTINE S1(V1) > > TYPE(T1), POINTER :: V1 > > We can't exploit restrict information for pointers, it would need to > be a var of type t1 with target attribute - but even then the double > indirection (v1 passed by reference, embedded array desc points to > data) prevents us from exploiting restrict info. > > So we might be safe for now (apart from the related ICE seen with lto). Actually we _can_ see the restrict, but only with -fipa-pta which then also sees the pointer association (even with -fno-inline) which then makes the alias visible again (so we ignore restrict, maybe by pure luck of course): D.1625_4 = *v1_2(D); # PT = nonlocal unit-escaped null { D.1644 D.1645 } (restr) D.1626_5 = D.1625_4->d.data; # PT = nonlocal unit-escaped null { D.1644 D.1645 } (restr) D.1630_10 = x.data; wonders of propagation ;)
Unassign Richard, who stated that he plans to ignore the issue for 4.6. The proper solution seems to be to fix it on the FE side - though I have not not fully re-read this PR to know whether it is possible or whether something in the definition of the Fortran standard prevents it and still requires some middle-end help/solution/hack. A work around is build with release checking (which is the default on the release branches).
We could do the analysis that fixed PR 45777 here, I just don't know where :-) Could somebody point me to the right place (I presume somewhere in trans-*)?
(In reply to comment #23) > We could do the analysis that fixed PR 45777 here, I just don't know > where :-) > > Could somebody point me to the right place (I presume somewhere in trans-*)? $ grep RESTRICT * ChangeLog-2009: instead set DECL_RESTRICTED_P on affected decls. trans-decl.c: DECL_RESTRICTED_P (decl) = 1; trans-types.c: prvoid_type_node = build_qualified_type (pvoid_type_node, TYPE_QUAL_RESTRICT); trans-types.c: TYPE_QUAL_RESTRICT); trans-types.c: type = build_qualified_type (type, TYPE_QUAL_RESTRICT); trans-types.c: arraytype = build_qualified_type (arraytype, TYPE_QUAL_RESTRICT); trans-types.c: type = build_qualified_type (type, TYPE_QUAL_RESTRICT); $ then if you look at those places, they are guarded with a "if (restricted)" condition, so the places you are looking for are those of calls to functions expecting an argument named restricted (gfc_build_array_type, gfc_get_nodesc_array_type). However, it doesn't look that easy. If I understand the thing correctly, in case you have an entity with a pointer or target attribute, you have to build a variant of its base type without the restrict attribute. That seems easy. Less easy is that you have to build a variant type without the restrict attribute for each of it's sub-components too. That is, for !!!!! type foo integer, allocatable :: bar(:) end type foo type(foo) :: x type(foo), pointer :: y call baz (x, y) !!!!! The type of baz's first actual argument (namely &x) is struct foo * restrict but the type of baz's second actual argument (namely &y) is not struct foo *. typeof(&x) == struct foo_restrict { struct integer_array { struct array_bounds { int lbound, ubound, stride; } bounds []; ... integer * restrict data; } bar; } * restrict whereas typeof(&y) == struct foo_norestrict { struct integer_array { struct array_bounds { int lbound, ubound, stride; } bounds []; ... integer * data; } bar; } * Note that the data member has lost the restrict too. Thus, when creating such pointer qualified types, one should take care not to update the sym->backend_decl of the derived type as they are different. That looks like a substantial rework. Or maybe drop restrict all together on the type for correctness. By the way, does the middle-end support creating a variant of a type with a sub-component restrict (un)qualified ?
Maybe it would be better to set the "inherited" pointer and target attributes much earlier, in gfc_variable_attr. With a bit of luck, things would then work automatically.
(In reply to comment #24) > (In reply to comment #23) > > We could do the analysis that fixed PR 45777 here, I just don't know > > where :-) > > > > Could somebody point me to the right place (I presume somewhere in trans-*)? > > $ grep RESTRICT * > ChangeLog-2009: instead set DECL_RESTRICTED_P on affected decls. > trans-decl.c: DECL_RESTRICTED_P (decl) = 1; > trans-types.c: prvoid_type_node = build_qualified_type (pvoid_type_node, > TYPE_QUAL_RESTRICT); > trans-types.c: TYPE_QUAL_RESTRICT); > trans-types.c: type = build_qualified_type (type, TYPE_QUAL_RESTRICT); > trans-types.c: arraytype = build_qualified_type (arraytype, > TYPE_QUAL_RESTRICT); > trans-types.c: type = build_qualified_type (type, TYPE_QUAL_RESTRICT); > $ > > then if you look at those places, they are guarded with a "if (restricted)" > condition, so the places you are looking for are those of calls to functions > expecting an argument named restricted (gfc_build_array_type, > gfc_get_nodesc_array_type). > > However, it doesn't look that easy. > If I understand the thing correctly, in case you have an entity with a pointer > or target attribute, you have to build a variant of its base type without the > restrict attribute. That seems easy. Less easy is that you have to build a > variant type without the restrict attribute for each of it's sub-components > too. > > That is, for > !!!!! > type foo > integer, allocatable :: bar(:) > end type foo > type(foo) :: x > type(foo), pointer :: y > > call baz (x, y) > !!!!! > The type of baz's first actual argument (namely &x) is struct foo * restrict > but the type of baz's second actual argument (namely &y) is not struct foo *. > > typeof(&x) == > struct foo_restrict { > struct integer_array { > struct array_bounds { > int lbound, ubound, stride; > } bounds []; > ... > integer * restrict data; > } bar; > } * restrict > > whereas > > typeof(&y) == > struct foo_norestrict { > struct integer_array { > struct array_bounds { > int lbound, ubound, stride; > } bounds []; > ... > integer * data; > } bar; > } * > > > Note that the data member has lost the restrict too. Thus, when creating such > pointer qualified types, one should take care not to update the > sym->backend_decl of the derived type as they are different. That looks like a > substantial rework. > Or maybe drop restrict all together on the type for correctness. Indeed when we introduced the restrict handling to the Fortran frontend we thought that the above case would not happen, thus that the testcase in question would be not a valid Fortran program. At the moment we probably even could generate a testcase that generates wrong-code (when the ICE silences itself with release-checking). This means that simply trying to make it not ICE is probably wrong and simply hides the real issue (which the ICE shows). It is, btw, a sign of bad Fortran language design and makes the point of the pointer/target attributes moot. What we could do is drop restrict qualifications for all aggregate types. At least such funny games do not seem possible with mere toplevel arrays or scalars, right? > By the way, does the middle-end support creating a variant of a type with a > sub-component restrict (un)qualified ? No. For type variants all the fields are usually shared, you'd have to create a deep copy.
(In reply to comment #25) > Maybe it would be better to set the "inherited" pointer and target > attributes much earlier, in gfc_variable_attr. With a bit of luck, > things would then work automatically. If you mean set the gfc_component.attr.target field, I don't think it's right. As gfc_component structs belong to the type's gfc_symbol, they are shared between non-pointer and pointer variants. (In reply to comment #26) > (In reply to comment #24) > > Or maybe drop restrict all together on the type for correctness. > > Indeed when we introduced the restrict handling to the Fortran frontend > we thought that the above case would not happen, thus that the testcase > in question would be not a valid Fortran program. At the moment > we probably even could generate a testcase that generates wrong-code > (when the ICE silences itself with release-checking). This means that > simply trying to make it not ICE is probably wrong and simply hides > the real issue (which the ICE shows). > > It is, btw, a sign of bad Fortran language design and makes the point > of the pointer/target attributes moot. I think it makes some sense: If there is more than one data path to a struct, there is more than one data path to all of its components. Thus the inherited target attribute. It's just it doesn't fit the C-minded middle-end well. :-( > > What we could do is drop restrict qualifications for all aggregate > types. At least such funny games do not seem possible with mere > toplevel arrays or scalars, right? Well, i think so. > > > By the way, does the middle-end support creating a variant of a type with a > > sub-component restrict (un)qualified ? > > No. For type variants all the fields are usually shared, you'd have to > create a deep copy. Which means they are not type-compatible anymore ? Which means casts everywhere ?
(In reply to comment #26) > It is, btw, a sign of bad Fortran language design and makes the point > of the pointer/target attributes moot. Well, I think it is just different model, which is used by the middle end and by the Fortran language, thus there is no simple one-to-one matching. That a hack was needed to get a better "restricted" support is already a hint that the models are a bit incompatible. Fortran's handling is simple: Every variable does not alias by default - the compiler only has to assume aliasing (of the variable and its components) if the variable is explicitly marked as TARGET in that scope; for sections of code where it does not have a TARGET attribute, the user has to ensure that there is no aliasing. There is only one complication: pointers and pointer components of derived types ("TYPE") can alias. [Cf. comment 18] Seemingly there is an issue of propagating this information properly to the middle end. Thus, for the first test case (comment 1): TYPE realspace_grid_type REAL(KIND=dp), DIMENSION ( :, :, : ), ALLOCATABLE :: r END TYPE realspace_grid_type TYPE(realspace_grid_type), POINTER :: x, y The "POINTER" attribute means that "x" and "y" are pointers; thus, if "x" and "y" are the same, x%r and y%r are the same, if "x" and "y" are different, "x%r" and "y%r" are different arrays. Additionally, the "pointer" attribute means that "x%r" and "y%r" have implicitly the TARGET attribute. Quoting comment 15: > You then need to make sure to create variant types of aggregates with the > target attribute applied to all subtypes (thus, the restrict stuff removed) > as the middle-end doesn't know about this rule. Which seems to be sensible - though it might need a substantial rework of the FE to keep all the time both variants available: From comment 24: > Note that the data member has lost the restrict too. Thus, when creating such > pointer qualified types, one should take care not to update the > sym->backend_decl of the derived type as they are different. That looks > like a substantial rework. One way would be to keep for data types all the time the two versions around: One with restrict and one without restrict; thus, if one does type extension, one has always a fully restrictless data type. Unfortunately, that seems to cause a missed-optimization for type t integer :: a integer, pointer :: b end type t type(t) :: x as "x%a" cannot alias and only "x%b" can - but is seems that one cannot encode this for the ME such that "t" as a whole has to be marked as unrestricted. > Or maybe drop restrict all together on the type for correctness. Well, "restrict" is a very important means to allow for optimization; at least as long as no POINTER is involved (neither the data type nor a component) and also no TARGET, one should really use restricted pointers. For the other cases, one has probably to accept that the ME does not support Fortran's rules :-( I think the easiest would be to create for derived types sym->backend_decl and another backend_decl with "restrict" - and propagate both variants types through, when extending the type or including it into another type. As soon as it is used as POINTER component, one could drop propagating the backend_decl_restricted.
> It is, btw, a sign of bad Fortran language design I don't see that. The frontend merely needs to emit correctly typed expressions. And that the type of 'a%b%ptr%d' depends on the prefix 'a%b%ptr' is no wonder and happens with most languages. > and makes the point > of the pointer/target attributes moot. Not really. The point being that non-pointer objects themself can't refer to any other object, and that non-target objects can't be referred to by anything else than their name or dummies. So, as long as no pointer is involved in an expression all is well. And as soon as a pointer is involved everything works automatically iff the pointer variable has the correct type (namely a type with an implied target attribute everywhere, which in frontend parlance is equivalent to a type without any restrict markers applied). > What we could do is drop restrict qualifications for all aggregate > types. At least such funny games do not seem possible with mere > toplevel arrays or scalars, right? IIRC the spec2k6 cases for which I added the restrict support were using aggregate types :-/
(In reply to comment #28) > One way would be to keep for data types all the time the two versions around: > One with restrict and one without restrict; makes sense > thus, if one does type extension, > one has always a fully restrictless data type. Why? From my point of view type extension is orthogonal to target. If an entity of extended derived type has the target/pointer attribute, one uses the non-restrict variant for its base type, and if it hasn't one uses the restrict variant. Do I miss something? > Unfortunately, that seems to > cause a missed-optimization for > type t > integer :: a > integer, pointer :: b > end type t > type(t) :: x > as "x%a" cannot alias and only "x%b" can - but is seems that one cannot encode > this for the ME such that "t" as a whole has to be marked as unrestricted. How about: struct t_restrict { int a; int * b; } * restrict VS struct t_norestrict { int a; int * b; } * ? > > I think the easiest would be to create for derived types sym->backend_decl and > another backend_decl with "restrict" - and propagate both variants types > through, when extending the type or including it into another type. As soon as > it is used as POINTER component, one could drop propagating the > backend_decl_restricted. Looks like a plan. :-)
On Tue, 18 Jan 2011, mikael at gcc dot gnu.org wrote: > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45586 > > --- Comment #30 from Mikael Morin <mikael at gcc dot gnu.org> 2011-01-18 12:48:41 UTC --- > (In reply to comment #28) > > One way would be to keep for data types all the time the two versions around: > > One with restrict and one without restrict; > makes sense > > > thus, if one does type extension, > > one has always a fully restrictless data type. > Why? From my point of view type extension is orthogonal to target. > If an entity of extended derived type has the target/pointer attribute, one > uses the non-restrict variant for its base type, and if it hasn't one uses the > restrict variant. Do I miss something? > > > Unfortunately, that seems to > > cause a missed-optimization for > > type t > > integer :: a > > integer, pointer :: b > > end type t > > type(t) :: x > > as "x%a" cannot alias and only "x%b" can - but is seems that one cannot encode > > this for the ME such that "t" as a whole has to be marked as unrestricted. > > How about: > > struct t_restrict { > int a; > int * b; > } * restrict > > VS > > struct t_norestrict { > int a; > int * b; > } * > > ? Yes. The restrict qualifier on t_restrict * does not apply to members, so as long as int *b is not restrict qualified the b's alias. > > I think the easiest would be to create for derived types sym->backend_decl and > > another backend_decl with "restrict" - and propagate both variants types > > through, when extending the type or including it into another type. As soon as > > it is used as POINTER component, one could drop propagating the > > backend_decl_restricted. > > Looks like a plan. :-) I think Micha has started working on something. Richard.
Yes, but it's possible I was going up the wrong tree. My idea was to build the no-restrict variants of types on demand, as necessary, basically from gfc_sym_type(), whenever sym->attr.pointer. This nearly worked, as in, it gives the top-level objects ('x' in question of the testcase from comment #2) a correct type (a record of only non-restrict members, recursively). Unfortunately the fortran frontend uses a peculiar way to expand the actual 'x%r' construct. The necessary FIELD_DECL is not pulled out from the type of 'x' (I mean the TREE_TYPE), but rather from the frontend-specific representation of types, in this case from the r-named-component's backend_decl. And that very backend_decl itself doesn't depend on the context, as in, there's always only one for a given type. So, yes, adding another backend_decl for one component which would hold the no-restrict variant would work. But that feels like a gross hack. Everything would work iff 'x' had the proper type from the start, in the frontend representation already. Then its components also would have the proper attributes set (target namely), leading to correctly typed backend_decls automatically. I quickly tried to implement something like that last paragraph, but were unsuccessful so far. My basic idea was to remember if a frontend decl had attr.pointer set, and then set attr.target for all subcomponents (via current_attr or otherwise). I thought this was possible as it seemed to me that the frontend possibly builds types for declarations anew each time if necessary. But I might be wrong.
(In reply to comment #32) > Yes, but it's possible I was going up the wrong tree. My idea was to > build the no-restrict variants of types on demand, as necessary, basically > from gfc_sym_type(), whenever sym->attr.pointer. > > This nearly worked, as in, it gives the top-level objects ('x' in question > of the testcase from comment #2) a correct type (a record of only non-restrict > members, recursively). > > Unfortunately the fortran frontend uses a peculiar way to expand the actual > 'x%r' construct. The necessary FIELD_DECL is not pulled out from the > type of 'x' (I mean the TREE_TYPE), but rather from the frontend-specific > representation of types, in this case from the r-named-component's > backend_decl. And that very backend_decl itself doesn't depend on the > context, as in, there's always only one for a given type. > > So, yes, adding another backend_decl for one component which would hold > the no-restrict variant would work. But that feels like a gross hack. > > Everything would work iff 'x' had the proper type from the start, in > the frontend representation already. Then its components also would have > the proper attributes set (target namely), leading to correctly typed > backend_decls automatically. > > I quickly tried to implement something like that last paragraph, > but were unsuccessful so far. My basic idea was to remember if a frontend > decl had attr.pointer set, and then set attr.target for all subcomponents > (via current_attr or otherwise). I thought this was possible as it seemed > to me that the frontend possibly builds types for declarations anew each > time if necessary. But I might be wrong. As I said in comment #27, gfc_component structs belonging to the type, they are shared between target and non-target variants. Thus one cannot set inherited target attributes on them. Consider this : !!!!!!!!!!!!! type bar integer :: a end type bar type(bar), pointer :: b type(bar) :: c end !!!!!!!!!!!!! If you want to set the target attribute on b%a, you will set it on the `a' component of the `bar' type, and will impact `c' as well.
> As I said in comment #27, gfc_component structs belonging to the type, they > are shared between target and non-target variants. Thus one cannot set > inherited target attributes on them. Yep, that's what I figured eventually :) The question now is if for: -------------------------- type bar integer :: a end type bar type(bar), pointer :: b type(bar) :: c -------------------------- 'b' and 'c' should have the same type. In other words should "type(bar),pointer" be a different type vs "type(bar)" already in the frontend representation for types, or not? If they would be they wouldn't have shared components and we would be free to set attributes as we like. Not knowing much about the several engineering decisions taken while developing the frontend _I_ personally would have done it this way. Alas, no cake for me :) So, some guidance by the frontend maintainers would be welcome which path I should take. Making it more the above suggestion, or really going with the two backend_decls per entity? (what I'm worried about with the two backend_decls is, what in one year we find other reasons why some attributes need to be different from the base case, then we already need four backend_delcs)
Created attachment 23047 [details] possible patch So, this is my current version. I'm creating a different type for top-level symbols that are attr.pointer. Further I'm checking the accessed FIELD_DECLs for compatibility with the parent type. If they are not, I explicitely search the parent type for a same-named FIELD_DECL (which by construction must exist and match). I cache this result in a new field of components. In this way we could theoretically also support a number of other different record types without having to add new backend_decl fields for each combination (we would merely add a linear number of more cache entries, in the hope that the program author is not in fact making use of all such combinations). It fixes the testcase. Haven't tested much further.
(In reply to comment #34) > Yep, that's what I figured eventually :) The question now is if for: > > -------------------------- > type bar > integer :: a > end type bar > type(bar), pointer :: b > type(bar) :: c > -------------------------- > > 'b' and 'c' should have the same type. In other words > should "type(bar),pointer" be a different type vs "type(bar)" already in the > frontend representation for types, or not? If they would be they wouldn't > have shared components and we would be free to set attributes as we like. > > Not knowing much about the several engineering decisions taken while > developing the frontend _I_ personally would have done it this way. Alas, > no cake for me :) Well, the current scheme is in line with how the standard is meant: entities of the same type can have or not the target attribute; in other words attributes are orthogonal to the type. This doesn't fit the middle-end but having it fit the middle-end would lead to the reverse problem : every time we find two objects of different type, we would have to look for variants to check if the objects are indeed of different type or only of different variants (and thus compatible). Personally, I would put the fix in the middle-end (which doesn't mean there is nothing to do on the front-end side). Instead of having the C type-compatibility rules hardcoded in the middle-end, have some relaxed rules: two objects are type-compatible iff all of their sub-components are type compatible. I guess no one wants to see the hack in his own area. > > So, some guidance by the frontend maintainers would be welcome which path > I should take. Making it more the above suggestion, or really going > with the two backend_decls per entity? (what I'm worried about with the > two backend_decls is, what in one year we find other reasons why some > attributes need to be different from the base case, then we already need > four backend_delcs) That's a valid concern. The following attributes are recursive like TARGET: ASYNCHRONOUS, INTENT, PROTECTED, VOLATILE Of those, INTENT and VOLATILE appear in the middle-end type (and TARGET of course). For ASYNCHRONOUS I don't know. (In reply to comment #35) > Created attachment 23047 [details] > possible patch > > So, this is my current version. I'm creating a different type for top-level > symbols that are attr.pointer. Further I'm checking the accessed FIELD_DECLs > for compatibility with the parent type. If they are not, I explicitely > search the parent type for a same-named FIELD_DECL (which by construction > must exist and match). I cache this result in a new field of components. > In this way we could theoretically also support a number of other different > record types without having to add new backend_decl fields for each > combination (we would merely add a linear number of more cache entries, > in the hope that the program author is not in fact making use of all such > combinations). > I'll look at it tomorrow.
On Fri, 21 Jan 2011, mikael at gcc dot gnu.org wrote: > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45586 > > --- Comment #36 from Mikael Morin <mikael at gcc dot gnu.org> 2011-01-21 22:54:53 UTC --- > (In reply to comment #34) > > Yep, that's what I figured eventually :) The question now is if for: > > > > -------------------------- > > type bar > > integer :: a > > end type bar > > type(bar), pointer :: b > > type(bar) :: c > > -------------------------- > > > > 'b' and 'c' should have the same type. In other words > > should "type(bar),pointer" be a different type vs "type(bar)" already in the > > frontend representation for types, or not? If they would be they wouldn't > > have shared components and we would be free to set attributes as we like. > > > > Not knowing much about the several engineering decisions taken while > > developing the frontend _I_ personally would have done it this way. Alas, > > no cake for me :) > > Well, the current scheme is in line with how the standard is meant: entities of > the same type can have or not the target attribute; in other words attributes > are orthogonal to the type. This doesn't fit the middle-end but having it fit > the middle-end would lead to the reverse problem : every time we find two > objects of different type, we would have to look for variants to check if the > objects are indeed of different type or only of different variants (and thus > compatible). > Personally, I would put the fix in the middle-end (which doesn't mean there is > nothing to do on the front-end side). Instead of having the C > type-compatibility rules hardcoded in the middle-end, have some relaxed rules: > two objects are type-compatible iff all of their sub-components are type > compatible. That's what we do ;) And void * restrict is not compatible with void *. So you get the ICE. Richard.
The patch looks good. Somewhat hackish as you acknowledge, but worth submitting anyway. A few minor comments below. > Index: fortran/trans-expr.c > =================================================================== > --- fortran/trans-expr.c (revision 168749) > +++ fortran/trans-expr.c (working copy) > @@ -504,6 +504,19 @@ gfc_conv_component_ref (gfc_se * se, gfc > field = c->backend_decl; > gcc_assert (TREE_CODE (field) == FIELD_DECL); > decl = se->expr; > + if (DECL_CONTEXT (field) != TREE_TYPE (decl)) > + { > + tree f2 = c->backend_decl2; > + if (f2 && DECL_FIELD_CONTEXT (f2) == TREE_TYPE (decl)) > + ; > + else for (f2 = TYPE_FIELDS (TREE_TYPE (decl)); f2; f2 = DECL_CHAIN (f2)) I prefer `if (!cond) ...' instead of `if (cond) ; else ...' > Index: fortran/gfortran.h > =================================================================== > --- fortran/gfortran.h (revision 168749) > +++ fortran/gfortran.h (working copy) > @@ -934,6 +934,7 @@ typedef struct gfc_component > gfc_array_spec *as; > > tree backend_decl; > + tree backend_decl2; More descriptive name (e.g. restrict_backend_decl or target_backend_decl or ...) and comment explaining the need for it appreciated. One could have the combinatorial explosion of backend_decls here as said Michael. In any case it can be safely removed if needed, as it is just a cache. > locus loc; > struct gfc_expr *initializer; > struct gfc_component *next; > Index: fortran/trans-types.c > =================================================================== > --- fortran/trans-types.c (revision 168749) > +++ fortran/trans-types.c (working copy) > @@ -1746,6 +1746,80 @@ gfc_build_pointer_type (gfc_symbol * sym > else > return build_pointer_type (type); > } > + > +static tree > +gfc_nonrestricted_type (tree t) > +{ > + tree ret = t; > + if (!TYPE_LANG_SPECIFIC (t)) > + TYPE_LANG_SPECIFIC (t) > + = ggc_alloc_cleared_lang_type (sizeof (struct lang_type)); > + if (TYPE_LANG_SPECIFIC (t)->nonrestricted_type) > + return TYPE_LANG_SPECIFIC (t)->nonrestricted_type; > + switch (TREE_CODE (t)) > + { > + default: > + break; > + > + case POINTER_TYPE: > + case REFERENCE_TYPE: > + ret = build_qualified_type (t, TYPE_QUALS (t) & ~TYPE_QUAL_RESTRICT); Isn't it necessary to call gfc_nonrestricted_type on TREE_TYPE (t) here ? > + break; > + > + case ARRAY_TYPE: > + { > + tree elemtype = gfc_nonrestricted_type (TREE_TYPE (t)); > + if (elemtype == TREE_TYPE (t)) > + ret = t; > + else > + { > + ret = copy_node (t); > + TREE_TYPE (t) = elemtype; > + /* ??? Change some TYPE_LANG_SPECIFICs too? */ > + } > + } > + break; > + > + case RECORD_TYPE: > + case UNION_TYPE: > + case QUAL_UNION_TYPE: > + { > + tree field, *chain; > + for (field = TYPE_FIELDS (t); field; field = DECL_CHAIN (field)) > + if (TREE_CODE (field) == FIELD_DECL) > + { > + tree elemtype = gfc_nonrestricted_type (TREE_TYPE (field)); > + if (elemtype != TREE_TYPE (field)) > + break; > + } > + if (!field) > + break; > + ret = copy_node (t); > + TYPE_FIELDS (ret) = NULL_TREE; > + chain = &TYPE_FIELDS (ret); > + for (field = TYPE_FIELDS (t); field; field = DECL_CHAIN (field)) > + { > + tree newfield = copy_node (field); > + DECL_CONTEXT (newfield) = ret; > + DECL_CHAIN (newfield) = NULL_TREE; > + if (TYPE_FIELDS (ret) == NULL_TREE) > + TYPE_FIELDS (ret) = newfield; Those two lines seem to duplicate the line below (as initially chain points to &TYPE_FIELDS(ret)). > + *chain = newfield; > + chain = &DECL_CHAIN (newfield); > + > + if (TREE_CODE (field) == FIELD_DECL) > + { > + tree elemtype = gfc_nonrestricted_type (TREE_TYPE (field)); > + TREE_TYPE (newfield) = elemtype; > + } > + } > + } > + break; > + } > + TYPE_LANG_SPECIFIC (t)->nonrestricted_type = ret; Don't know if it is absolutely necessary, but one might add also : TYPE_LANG_SPECIFIC (ret)->nonrestricted_type = ret; > + return ret; > +} > + > > /* Return the type for a symbol. Special handling is required for character > types to get the correct level of indirection. > @@ -1789,6 +1863,9 @@ gfc_sym_type (gfc_symbol * sym) > else > type = gfc_typenode_for_spec (&sym->ts); > > + if (sym->attr.pointer) > + type = gfc_nonrestricted_type (type); > + This is missing the target attribute, so you may preferably use the restricted boolean a few lines below. > if (sym->attr.dummy && !sym->attr.function && !sym->attr.value) > byref = 1; > else > Index: fortran/trans.h > =================================================================== > --- fortran/trans.h (revision 168749) > +++ fortran/trans.h (working copy) > @@ -700,6 +700,7 @@ struct GTY((variable_size)) lang_type { > tree dataptr_type; > tree span; > tree base_decl[2]; > + tree nonrestricted_type; > }; > > struct GTY((variable_size)) lang_decl {
(In reply to comment #37) > > That's what we do ;) Wow! Middle-end gurus take design decisions of mine before I have ever thought them. They are real wizards after all. > And void * restrict is not compatible with > void *. So you get the ICE. Hum, may I suggest a --push-harder/--will-you-swallow-it option ? More seriously, I will test the patch above.
The patch from comment #35 requires another change in unrelated code, which I think actually fixes a pre-existing bug in type extension support: Index: fortran/trans-expr.c =================================================================== --- fortran/trans-expr.c (revision 168749) +++ fortran/trans-expr.c (working copy) @@ -549,11 +562,11 @@ conv_parent_component_references (gfc_se if (dt->attr.extension && dt->components) { - if (dt->attr.is_class) + if (1 || dt->attr.is_class) cmp = dt->components; else cmp = dt->components->next; - /* Return if the component is not in the parent type. */ + /* Return if the component is in this type. */ for (; cmp; cmp = cmp->next) if (strcmp (c->name, cmp->name) == 0) return; Otherwise the new assert will trigger on extends_*.f03 because the frontend is trying to generate obviously wrong component_refs.
(In reply to comment #39) > > void *. So you get the ICE. > Hum, may I suggest a --push-harder/--will-you-swallow-it option ? --enable-checking=release ?
(In reply to comment #40) There is indeed something fishy here. Your change does the right thing in the non-class case I think ; can't tell about the class case. Janus ? The code tries to match a reference to an inherited component but misses references to the base types themselves. In extends_1.f03, for: new_person%service%education%person%ss = ss the dump shows: new_person->service.education.person.service.education.person.education.person.person.ss = *ss; Despite this, the middle-end manages to produce something runnable. :-)
Yep. With my patch the saner looking new_person->service.education.person.ss = *ss; statement is generated. It's possible that class containers actually contain something as first component that shouldn't be looked up, then the condition would be reversed, or that meanwhile the parent type is always reachable via the first component. The latter is definitely the case for types that don't have is_class set.
(In reply to comment #42) > (In reply to comment #40) > > There is indeed something fishy here. > Your change does the right thing in the non-class case I think ; can't tell > about the class case. > Janus ? Sorry for the late answer. I think the patch is ok. It does not affect OOP anyway. And for non-class types there was a bug apparently. AFAICS one could simplify 'conv_parent_component_references' in the following way: Index: gcc/fortran/trans-expr.c =================================================================== --- gcc/fortran/trans-expr.c (revision 169891) +++ gcc/fortran/trans-expr.c (working copy) @@ -538,6 +538,11 @@ conv_parent_component_references (gfc_se * se, gfc dt = ref->u.c.sym; c = ref->u.c.component; + /* Return if the component is not in the parent type. */ + for (cmp = dt->components; cmp; cmp = cmp->next) + if (strcmp (c->name, cmp->name) == 0) + return; + /* Build a gfc_ref to recursively call gfc_conv_component_ref. */ parent.type = REF_COMPONENT; parent.next = NULL; @@ -546,24 +551,12 @@ conv_parent_component_references (gfc_se * se, gfc if (dt->backend_decl == NULL) gfc_get_derived_type (dt); - - if (dt->attr.extension && dt->components) - { - if (dt->attr.is_class) - cmp = dt->components; - else - cmp = dt->components->next; - /* Return if the component is not in the parent type. */ - for (; cmp; cmp = cmp->next) - if (strcmp (c->name, cmp->name) == 0) - return; - - /* Otherwise build the reference and call self. */ - gfc_conv_component_ref (se, &parent); - parent.u.c.sym = dt->components->ts.u.derived; - parent.u.c.component = c; - conv_parent_component_references (se, &parent); - } + + /* Otherwise build the reference and call self. */ + gfc_conv_component_ref (se, &parent); + parent.u.c.sym = dt->components->ts.u.derived; + parent.u.c.component = c; + conv_parent_component_references (se, &parent); } /* Return the contents of a variable. Also handles reference/pointer
Author: burnus Date: Sat Feb 12 13:09:03 2011 New Revision: 170072 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=170072 Log: 2011-02-12 Michael Matz <matz@suse.de> Janus Weil <janus@gcc.gnu.org> Tobias Burnus <burnus@net-b.de> PR fortran/45586 * trans-expr.c (conv_parent_component_references): Avoid unintendent skipping of parent compounds. Modified: trunk/gcc/fortran/ChangeLog trunk/gcc/fortran/trans-expr.c
(In reply to comment #45) > New Revision: 170072 That patch fixed the issue of comment 40 to comment 44. TODO: The actual restrict patch of comment 35 (attachment 23047 [details]), taking into account the review comment as comment 38
Author: mikael Date: Sat Feb 12 14:56:32 2011 New Revision: 170074 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=170074 Log: 2011-02-12 Mikael Morin <mikael.morin@sfr.fr> PR fortran/45586 * gfortran.dg/extends_11.f03: New. Added: trunk/gcc/testsuite/gfortran.dg/extends_11.f03 Modified: trunk/gcc/testsuite/ChangeLog
Author: matz Date: Fri Feb 18 19:52:16 2011 New Revision: 170284 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=170284 Log: PR fortran/45586 * gfortran.h (struct gfc_component): Add norestrict_decl member. * trans.h (struct lang_type): Add nonrestricted_type member. * trans-expr.c (gfc_conv_component_ref): Search fields with correct parent type. * trans-types.c (mirror_fields, gfc_nonrestricted_type): New. (gfc_sym_type): Use it. testsuite/ PR fortran/45586 * gfortran.dg/lto/pr45586_0.f90: New test. * gfortran.dg/typebound_proc_20.f90: Ditto. * gfortran.dg/typebound_proc_21.f90: Ditto. Added: trunk/gcc/testsuite/gfortran.dg/lto/pr45586_0.f90 trunk/gcc/testsuite/gfortran.dg/typebound_proc_20.f90 trunk/gcc/testsuite/gfortran.dg/typebound_proc_21.f90 Modified: trunk/gcc/fortran/ChangeLog trunk/gcc/fortran/gfortran.h trunk/gcc/fortran/trans-expr.c trunk/gcc/fortran/trans-types.c trunk/gcc/fortran/trans.h
Fixed.
The original problem in comment #0 fails (i.e. the build of CP2K) with trunk 4.7, at what I believe is essentially the same issue. Unfortunately the smaller testcase (comment #1) doesn't fail anymore. In file included from :363:0: /data03/vondele/cp2k_gcc/cp2k/makefiles/../src/qs_linres_current.F: In function ‘calculate_jrho_resp’: /data03/vondele/cp2k_gcc/cp2k/makefiles/../src/qs_linres_current.F:612:0: error: non-trivial conversion at assignment struct array3_real(kind=8) struct array3_real(kind=8) # .MEM_3879 = VDEF <.MEM_3388> my_gauge = D.66078_1423->r; /data03/vondele/cp2k_gcc/cp2k/makefiles/../src/qs_linres_current.F:612:0: internal compiler error: verify_gimple failed Please submit a full bug report, with preprocessed source if appropriate. See <http://gcc.gnu.org/bugs.html> for instructions. make[3]: *** [/dev/shm/vondele/cciVnmd6.ltrans0.ltrans.o] Error 1 make[3]: Target `all' not remade because of errors. lto-wrapper: make returned 2 exit status /data03/vondele/gnu/binutils-2.21/install/bin/ld.gold: fatal error: lto-wrapper failed collect2: ld returned 1 exit status
(In reply to comment #50) > The original problem in comment #0 fails (i.e. the build of CP2K) with trunk > 4.7, at what I believe is essentially the same issue. > Unfortunately the smaller testcase (comment #1) doesn't fail anymore. > /data03/vondele/cp2k_gcc/cp2k/makefiles/../src/qs_linres_current.F: > In function ‘calculate_jrho_resp’: > /data03/vondele/cp2k_gcc/cp2k/makefiles/../src/qs_linres_current.F:612:0: > error: non-trivial conversion at assignment Could you try to reduce the test case?
(In reply to comment #51) > (In reply to comment #50) > > The original problem in comment #0 fails (i.e. the build of CP2K) with trunk > Could you try to reduce the test case? I will give it a try later this week, but this kind of LTO ICEs are difficult to reduce (starting from ~1MLOC), since it can basically not be done automatically (and the obvious testcase doesn't seem to fail anymore).
reduced testcase for 4.7 MODULE M1 INTEGER, PARAMETER :: dp=8 TYPE realspace_grid_type REAL(KIND=dp), DIMENSION ( :, :, : ), ALLOCATABLE :: r END TYPE realspace_grid_type TYPE realspace_grid_p_type TYPE(realspace_grid_type), POINTER :: rs_grid END TYPE realspace_grid_p_type TYPE realspaces_grid_p_type TYPE(realspace_grid_p_type), DIMENSION(:), POINTER :: rs END TYPE realspaces_grid_p_type END MODULE MODULE M2 USE M1 CONTAINS SUBROUTINE S1() INTEGER :: i,j TYPE(realspaces_grid_p_type), DIMENSION(:), POINTER :: rs_gauge REAL(dp), DIMENSION(:, :, :), POINTER :: y y=>rs_gauge(i)%rs(j)%rs_grid%r END SUBROUTINE END MODULE USE M2 CALL S1() END > gfortran -O0 -flto test.f90 In file included from :0:0: test.f90: In function ‘s1’: test.f90:17:0: error: non-trivial conversion at assignment struct array3_real(kind=8) struct array3_real(kind=8) y = D.2087_27->r; test.f90:17:0: internal compiler error: verify_gimple failed
(In reply to comment #53) > reduced testcase for 4.7 Does not fail here - can you still reproduce it? (It might have been fixed by the patch for PR 48588. If it still occurs, I will try harder.)
(In reply to comment #54) > (In reply to comment #53) > > reduced testcase for 4.7 > > Does not fail here - can you still reproduce it? (It might have been fixed by > the patch for PR 48588. If it still occurs, I will try harder.) still fails for me with latest trunk (but notice the testcase only fails to compile at -O0 -flto). For completeness, the full -v output for me, might be gold or so related: gfortran -v -O0 -flto t.f90 Driving: gfortran -v -O0 -flto t.f90 -l gfortran -l m -shared-libgcc Using built-in specs. COLLECT_GCC=gfortran COLLECT_LTO_WRAPPER=/data03/vondele/gnu/gcc_trunk/install/libexec/gcc/x86_64-unknown-linux-gnu/4.7.0/lto-wrapper Target: x86_64-unknown-linux-gnu Configured with: ../gcc/configure --prefix=/data03/vondele/gnu/gcc_trunk/install --enable-languages=c,c++,fortran --disable-multilib --enable-plugins --enable-cloog-backend=isl --with-ppl=/data03/vondele/gnu/ppl-0.11/install --with-cloog=/data03/vondele/gnu/cloog-0.16.1/install/ --with-libelf=/data03/vondele/gnu/libelf-0.8.13/install --with-plugin-ld=ld.gold Thread model: posix gcc version 4.7.0 20110426 (experimental) [trunk revision 172980] (GCC) COLLECT_GCC_OPTIONS='-v' '-O0' '-flto' '-shared-libgcc' '-mtune=generic' '-march=x86-64' /data03/vondele/gnu/gcc_trunk/install/libexec/gcc/x86_64-unknown-linux-gnu/4.7.0/f951 t.f90 -quiet -dumpbase t.f90 -mtune=generic -march=x86-64 -auxbase t -O0 -version -flto -fintrinsic-modules-path /data03/vondele/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/4.7.0/finclude -o /dev/shm/vondele/cc5Lcxwt.s GNU Fortran (GCC) version 4.7.0 20110426 (experimental) [trunk revision 172980] (x86_64-unknown-linux-gnu) compiled by GNU C version 4.7.0 20110426 (experimental) [trunk revision 172980], GMP version 4.3.2, MPFR version 2.4.2, MPC version 0.8.2 GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096 GNU Fortran (GCC) version 4.7.0 20110426 (experimental) [trunk revision 172980] (x86_64-unknown-linux-gnu) compiled by GNU C version 4.7.0 20110426 (experimental) [trunk revision 172980], GMP version 4.3.2, MPFR version 2.4.2, MPC version 0.8.2 GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096 COLLECT_GCC_OPTIONS='-v' '-O0' '-flto' '-shared-libgcc' '-mtune=generic' '-march=x86-64' as --64 -o /dev/shm/vondele/ccHOHV4Q.o /dev/shm/vondele/cc5Lcxwt.s Reading specs from /data03/vondele/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/4.7.0/../../../../lib64/libgfortran.spec rename spec lib to liborig COLLECT_GCC_OPTIONS='-v' '-O0' '-flto' '-shared-libgcc' '-mtune=generic' '-march=x86-64' COMPILER_PATH=/data03/vondele/gnu/gcc_trunk/install/libexec/gcc/x86_64-unknown-linux-gnu/4.7.0/:/data03/vondele/gnu/gcc_trunk/install/libexec/gcc/x86_64-unknown-linux-gnu/4.7.0/:/data03/vondele/gnu/gcc_trunk/install/libexec/gcc/x86_64-unknown-linux-gnu/:/data03/vondele/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/4.7.0/:/data03/vondele/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/ LIBRARY_PATH=/data03/vondele/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/4.7.0/:/data03/vondele/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/4.7.0/../../../../lib64/:/lib/../lib64/:/usr/lib/../lib64/:/data03/vondele/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/4.7.0/../../../:/lib/:/usr/lib/ COLLECT_GCC_OPTIONS='-v' '-O0' '-flto' '-shared-libgcc' '-mtune=generic' '-march=x86-64' /data03/vondele/gnu/gcc_trunk/install/libexec/gcc/x86_64-unknown-linux-gnu/4.7.0/collect2 -flto --eh-frame-hdr -m elf_x86_64 -dynamic-linker /lib64/ld-linux-x86-64.so.2 /usr/lib/../lib64/crt1.o /usr/lib/../lib64/crti.o /data03/vondele/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/4.7.0/crtbegin.o -L/data03/vondele/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/4.7.0 -L/data03/vondele/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/4.7.0/../../../../lib64 -L/lib/../lib64 -L/usr/lib/../lib64 -L/data03/vondele/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/4.7.0/../../.. /dev/shm/vondele/ccHOHV4Q.o -lgfortran -lm -lgcc_s -lgcc -lquadmath -lm -lgcc_s -lgcc -lc -lgcc_s -lgcc /data03/vondele/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/4.7.0/crtend.o /usr/lib/../lib64/crtn.o gfortran @/dev/shm/vondele/ccrOSrYZ.args Using built-in specs. COLLECT_GCC=gfortran COLLECT_LTO_WRAPPER=/data03/vondele/gnu/gcc_trunk/install/libexec/gcc/x86_64-unknown-linux-gnu/4.7.0/lto-wrapper Target: x86_64-unknown-linux-gnu Configured with: ../gcc/configure --prefix=/data03/vondele/gnu/gcc_trunk/install --enable-languages=c,c++,fortran --disable-multilib --enable-plugins --enable-cloog-backend=isl --with-ppl=/data03/vondele/gnu/ppl-0.11/install --with-cloog=/data03/vondele/gnu/cloog-0.16.1/install/ --with-libelf=/data03/vondele/gnu/libelf-0.8.13/install --with-plugin-ld=ld.gold Thread model: posix gcc version 4.7.0 20110426 (experimental) [trunk revision 172980] (GCC) COLLECT_GCC_OPTIONS='-c' '-v' '-O0' '-shared-libgcc' '-mtune=generic' '-march=x86-64' '-fltrans-output-list=/dev/shm/vondele/ccU5I08B.ltrans.out' '-fwpa' /data03/vondele/gnu/gcc_trunk/install/libexec/gcc/x86_64-unknown-linux-gnu/4.7.0/lto1 -quiet -dumpbase ccHOHV4Q.o -mtune=generic -march=x86-64 -auxbase ccHOHV4Q -O0 -version -fltrans-output-list=/dev/shm/vondele/ccU5I08B.ltrans.out -fwpa @/dev/shm/vondele/ccFk202B GNU GIMPLE (GCC) version 4.7.0 20110426 (experimental) [trunk revision 172980] (x86_64-unknown-linux-gnu) compiled by GNU C version 4.7.0 20110426 (experimental) [trunk revision 172980], GMP version 4.3.2, MPFR version 2.4.2, MPC version 0.8.2 GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096 GNU GIMPLE (GCC) version 4.7.0 20110426 (experimental) [trunk revision 172980] (x86_64-unknown-linux-gnu) compiled by GNU C version 4.7.0 20110426 (experimental) [trunk revision 172980], GMP version 4.3.2, MPFR version 2.4.2, MPC version 0.8.2 GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096 COMPILER_PATH=/data03/vondele/gnu/gcc_trunk/install/libexec/gcc/x86_64-unknown-linux-gnu/4.7.0/:/data03/vondele/gnu/gcc_trunk/install/libexec/gcc/x86_64-unknown-linux-gnu/4.7.0/:/data03/vondele/gnu/gcc_trunk/install/libexec/gcc/x86_64-unknown-linux-gnu/:/data03/vondele/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/4.7.0/:/data03/vondele/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/:/data03/vondele/gnu/gcc_trunk/install/libexec/gcc/x86_64-unknown-linux-gnu/4.7.0/:/data03/vondele/gnu/gcc_trunk/install/libexec/gcc/x86_64-unknown-linux-gnu/4.7.0/:/data03/vondele/gnu/gcc_trunk/install/libexec/gcc/x86_64-unknown-linux-gnu/:/data03/vondele/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/4.7.0/:/data03/vondele/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/ LIBRARY_PATH=/data03/vondele/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/4.7.0/../../../../lib64/../lib64/:/lib/../lib64/../lib64/:/usr/lib/../lib64/../lib64/:/data03/vondele/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/4.7.0/../../../../lib64/:/lib/../lib64/:/usr/lib/../lib64/:/data03/vondele/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/4.7.0/:/data03/vondele/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/4.7.0/../../../../lib64/:/lib/../lib64/:/usr/lib/../lib64/:/data03/vondele/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/4.7.0/:/data03/vondele/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/4.7.0/../../../../lib64/:/lib/../lib64/:/usr/lib/../lib64/:/data03/vondele/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/4.7.0/../../../:/lib/:/usr/lib/:/data03/vondele/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/4.7.0/../../../:/lib/:/usr/lib/ COLLECT_GCC_OPTIONS='-c' '-v' '-O0' '-shared-libgcc' '-mtune=generic' '-march=x86-64' '-fltrans-output-list=/dev/shm/vondele/ccU5I08B.ltrans.out' '-fwpa' gfortran @/dev/shm/vondele/ccbrXBHL.args Using built-in specs. COLLECT_GCC=gfortran COLLECT_LTO_WRAPPER=/data03/vondele/gnu/gcc_trunk/install/libexec/gcc/x86_64-unknown-linux-gnu/4.7.0/lto-wrapper Target: x86_64-unknown-linux-gnu Configured with: ../gcc/configure --prefix=/data03/vondele/gnu/gcc_trunk/install --enable-languages=c,c++,fortran --disable-multilib --enable-plugins --enable-cloog-backend=isl --with-ppl=/data03/vondele/gnu/ppl-0.11/install --with-cloog=/data03/vondele/gnu/cloog-0.16.1/install/ --with-libelf=/data03/vondele/gnu/libelf-0.8.13/install --with-plugin-ld=ld.gold Thread model: posix gcc version 4.7.0 20110426 (experimental) [trunk revision 172980] (GCC) COLLECT_GCC_OPTIONS='-c' '-v' '-O0' '-shared-libgcc' '-mtune=generic' '-march=x86-64' '-fltrans' '-o' '/dev/shm/vondele/ccU5I08B.ltrans0.ltrans.o' /data03/vondele/gnu/gcc_trunk/install/libexec/gcc/x86_64-unknown-linux-gnu/4.7.0/lto1 -quiet -dumpbase ccU5I08B.ltrans0.o -mtune=generic -march=x86-64 -auxbase-strip /dev/shm/vondele/ccU5I08B.ltrans0.ltrans.o -O0 -version -fltrans @/dev/shm/vondele/ccZW67eE -o /dev/shm/vondele/ccktUf91.s GNU GIMPLE (GCC) version 4.7.0 20110426 (experimental) [trunk revision 172980] (x86_64-unknown-linux-gnu) compiled by GNU C version 4.7.0 20110426 (experimental) [trunk revision 172980], GMP version 4.3.2, MPFR version 2.4.2, MPC version 0.8.2 GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096 GNU GIMPLE (GCC) version 4.7.0 20110426 (experimental) [trunk revision 172980] (x86_64-unknown-linux-gnu) compiled by GNU C version 4.7.0 20110426 (experimental) [trunk revision 172980], GMP version 4.3.2, MPFR version 2.4.2, MPC version 0.8.2 GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096 In file included from :0:0: t.f90: In function ‘s1’: t.f90:17:0: error: non-trivial conversion at assignment struct array3_real(kind=8) struct array3_real(kind=8) y = D.2088_27->r; t.f90:17:0: internal compiler error: verify_gimple failed Please submit a full bug report, with preprocessed source if appropriate. See <http://gcc.gnu.org/bugs.html> for instructions. lto-wrapper: gfortran returned 1 exit status collect2: lto-wrapper returned 1 exit status
(In reply to comment #54) > (In reply to comment #53) > > reduced testcase for 4.7 > > Does not fail here - can you still reproduce it? (It might have been fixed by > the patch for PR 48588. If it still occurs, I will try harder.) an as a PS, --enable-checking=release also hides the bug.
(In reply to comment #54) > (In reply to comment #53) > > reduced testcase for 4.7 > > Does not fail here - can you still reproduce it? (It might have been fixed by > the patch for PR 48588. If it still occurs, I will try harder.) Fails for me here too, at -O0. Looking at the site of the error message, we get (note the "public unsigned DI" vs. "unsigned restrict DI": 3722 error ("non-trivial conversion at assignment"); (gdb) p debug_tree(lhs_type) <record_type 0x7ffff70dae70 array3_real(kind=8) BLK size <integer_cst 0x7ffff70d1420 type <integer_type 0x7ffff7ee50a8 bit_size_type> constant 768> unit size <integer_cst 0x7ffff70d1460 type <integer_type 0x7ffff7ee5000> constant 96> align 64 symtab 0 alias set -1 canonical type 0x7ffff70daf18 fields <field_decl 0x7ffff70df4c0 data type <pointer_type 0x7ffff7ee5dc8 type <void_type 0x7ffff7ee5d20 void> public unsigned DI size <integer_cst 0x7ffff7eea1e0 constant 64> unit size <integer_cst 0x7ffff7eea200 constant 8> align 64 symtab 0 alias set -1 canonical type 0x7ffff7ee5dc8 pointer_to_this <pointer_type 0x7ffff7ef7e70>> unsigned DI file test.f90 line 17 col 0 size <integer_cst 0x7ffff7eea1e0 64> unit size <integer_cst 0x7ffff7eea200 8> align 64 offset_align 128 offset <integer_cst 0x7ffff7ed5f00 constant 0> bit offset <integer_cst 0x7ffff7eea420 constant 0> context <record_type 0x7ffff70daf18 array_descriptor3> chain <field_decl 0x7ffff70df558 offset type <integer_type 0x7ffff7ee55e8 long int> DI file test.f90 line 17 col 0 size <integer_cst 0x7ffff7eea1e0 64> unit size <integer_cst 0x7ffff7eea200 8> align 64 offset_align 128 offset <integer_cst 0x7ffff7ed5f00 0> bit offset <integer_cst 0x7ffff7eea1e0 64> context <record_type 0x7ffff70daf18 array_descriptor3> chain <field_decl 0x7ffff70df5f0 dtype>>>> $1 = void (gdb) p debug_tree(rhs1_type) <record_type 0x7ffff70da7e0 array3_real(kind=8) BLK size <integer_cst 0x7ffff70d1420 type <integer_type 0x7ffff7ee50a8 bit_size_type> constant 768> unit size <integer_cst 0x7ffff70d1460 type <integer_type 0x7ffff7ee5000> constant 96> align 64 symtab 0 alias set -1 canonical type 0x7ffff70da930 fields <field_decl 0x7ffff7ededa8 data type <pointer_type 0x7ffff70da888 type <void_type 0x7ffff7ee5d20 void> unsigned restrict DI size <integer_cst 0x7ffff7eea1e0 constant 64> unit size <integer_cst 0x7ffff7eea200 constant 8> align 64 symtab 0 alias set -1 canonical type 0x7ffff70da888> unsigned DI file test.f90 line 17 col 0 size <integer_cst 0x7ffff7eea1e0 64> unit size <integer_cst 0x7ffff7eea200 8> align 64 offset_align 128 offset <integer_cst 0x7ffff7ed5f00 constant 0> bit offset <integer_cst 0x7ffff7eea420 constant 0> context <record_type 0x7ffff70da930 array_descriptor3> chain <field_decl 0x7ffff7edee40 offset type <integer_type 0x7ffff7ee55e8 long int> DI file test.f90 line 17 col 0 size <integer_cst 0x7ffff7eea1e0 64> unit size <integer_cst 0x7ffff7eea200 8> align 64 offset_align 128 offset <integer_cst 0x7ffff7ed5f00 0> bit offset <integer_cst 0x7ffff7eea1e0 64> context <record_type 0x7ffff70da930 array_descriptor3> chain <field_decl 0x7ffff7edeed8 dtype>>>> $2 = void (gdb) <record_type 0x7ffff70da7e0 array3_real(kind=8) BLK size <integer_cst 0x7ffff70d1420 type <integer_type 0x7ffff7ee50a8 bit_size_type> constant 768> unit size <integer_cst 0x7ffff70d1460 type <integer_type 0x7ffff7ee5000> constant 96> align 64 symtab 0 alias set -1 canonical type 0x7ffff70da930 fields <field_decl 0x7ffff7ededa8 data type <pointer_type 0x7ffff70da888 type <void_type 0x7ffff7ee5d20 void> unsigned restrict DI size <integer_cst 0x7ffff7eea1e0 constant 64> unit size <integer_cst 0x7ffff7eea200 constant 8> align 64 symtab 0 alias set -1 canonical type 0x7ffff70da888> unsigned DI file test.f90 line 17 col 0 size <integer_cst 0x7ffff7eea1e0 64> unit size <integer_cst 0x7ffff7eea200 8> align 64 offset_align 128 offset <integer_cst 0x7ffff7ed5f00 constant 0> bit offset <integer_cst 0x7ffff7eea420 constant 0> context <record_type 0x7ffff70da930 array_descriptor3> chain <field_decl 0x7ffff7edee40 offset type <integer_type 0x7ffff7ee55e8 long int> DI file test.f90 line 17 col 0 size <integer_cst 0x7ffff7eea1e0 64> unit size <integer_cst 0x7ffff7eea200 8> align 64 offset_align 128 offset <integer_cst 0x7ffff7ed5f00 0> bit offset <integer_cst 0x7ffff7eea1e0 64> context <record_type 0x7ffff70da930 array_descriptor3> chain <field_decl 0x7ffff7edeed8 dtype>>>> $3 = void
GCC 4.6.1 is being released.
(In reply to comment #53) > reduced testcase for 4.7 > y=>rs_gauge(i)%rs(j)%rs_grid%r Here, "%r->data" is marked as restricted as "r" is allocatable. However, all other references are a pointer. The derived type is obtained via a nested calls to gfc_get_derived_type. For "r" one calls gfc_get_array_type_bounds - seemingly with "restricted = 1". If one now declares "rs_gauge", one calls gfc_nonrestricted_type which removes the "restrict" qualifier from the type (i.e. "rs_gauge") and from the "rs_gauge->rs" but does not go higher up the chain. Thus, one somehow needs to walk the components and remove the "restrict" there ... With the following patch, one does not get any ICE anymore, but I am neither sure whether it is sufficient, nor whether it removes restrict qualifiers where it shouldn't, nor whether it is correct at all. --- a/gcc/fortran/trans-types.c +++ b/gcc/fortran/trans-types.c @@ -2421,6 +2421,9 @@ gfc_get_derived_type (gfc_symbol * derived) && !c->attr.proc_pointer) field_type = build_pointer_type (field_type); + if (c->attr.pointer) + field_type = gfc_nonrestricted_type (field_type); + /* vtype fields can point to different types to the base type. */ if (c->ts.type == BT_DERIVED && c->ts.u.derived->attr.vtype) field_type = build_pointer_type_for_mode (TREE_TYPE (field_type),
Author: burnus Date: Wed Jul 27 22:33:00 2011 New Revision: 176852 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=176852 Log: 2011-07-27 Tobias Burnus <burnus@net-b.de> PR fortran/45586 * trans-types.c (gfc_get_derived_type): Ensure that pointer component types are marked as nonrestricted. 2011-07-27 Tobias Burnus <burnus@net-b.de> PR fortran/45586 * gfortran.dg/lto/pr45586-2_0.f90: New. Added: trunk/gcc/testsuite/gfortran.dg/lto/pr45586-2_0.f90 Modified: trunk/gcc/fortran/ChangeLog trunk/gcc/fortran/trans-types.c trunk/gcc/testsuite/ChangeLog
Author: burnus Date: Thu Jul 28 05:40:21 2011 New Revision: 176858 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=176858 Log: 2011-07-28 Tobias Burnus <burnus@net-b.de> PR fortran/45586 * trans-types.c (gfc_get_derived_type): Ensure that pointer component types are marked as nonrestricted. 2011-07-28 Tobias Burnus <burnus@net-b.de> PR fortran/45586 * gfortran.dg/lto/pr45586-2_0.f90: New. Added: branches/gcc-4_6-branch/gcc/testsuite/gfortran.dg/lto/pr45586-2_0.f90 Modified: branches/gcc-4_6-branch/gcc/fortran/ChangeLog branches/gcc-4_6-branch/gcc/fortran/trans-types.c branches/gcc-4_6-branch/gcc/testsuite/ChangeLog
FIXED the issue of comment 53 on the 4.7 trunk and on the 4.6 branch. Thanks for the report!
With a (seemingly) unrelated patch (attached to PR52097) I'm back on ICEing for the gfortran.dg/lto/pr45586*.f90 testcases ... Even before the adjusted type merging we have (at compile-time) Breakpoint 5, output_gimple_stmt (ob=0x1c6c440, stmt=0x7ffff5b51730) at /space/rguenther/src/svn/trunk/gcc/gimple-streamer-out.c:138 138 stream_write_tree (ob, op, true); y = D.1925_27->r; (gdb) call debug_tree (gimple_assign_lhs (stmt)->typed.type) <record_type 0x7ffff5b4d2a0 array3_real(kind=8) type_1 BLK size <integer_cst 0x7ffff5b3e700 type <integer_type 0x7ffff5a2e0a8 bitsizetype> constant 768> unit size <integer_cst 0x7ffff5b1fc60 type <integer_type 0x7ffff5a2e000 sizetype> constant 96> align 64 symtab 0 alias set -1 canonical type 0x7ffff5b4a0a8 fields <field_decl 0x7ffff5b3d7b8 data type <pointer_type 0x7ffff5a2ec78 type <void_type 0x7ffff5a2ebd0 void> public unsigned DI size <integer_cst 0x7ffff5a1dec0 constant 64> unit size <integer_cst 0x7ffff5a1dee0 constant 8> align 64 symtab 0 alias set -1 canonical type 0x7ffff5a2ec78 pointer_to_this <pointer_type 0x7ffff5a41f18>> (gdb) call debug_tree (gimple_assign_rhs1 (stmt)->typed.type) <record_type 0x7ffff5b4a738 array3_real(kind=8) type_1 BLK size <integer_cst 0x7ffff5b3e700 type <integer_type 0x7ffff5a2e0a8 bitsizetype> constant 768> unit size <integer_cst 0x7ffff5b1fc60 type <integer_type 0x7ffff5a2e000 sizetype> constant 96> align 64 symtab 0 alias set -1 canonical type 0x7ffff5b4a0a8 fields <field_decl 0x7ffff5b3dab0 data type <pointer_type 0x7ffff5a2ec78 type <void_type 0x7ffff5a2ebd0 void> public unsigned DI size <integer_cst 0x7ffff5a1dec0 constant 64> unit size <integer_cst 0x7ffff5a1dee0 constant 8> align 64 symtab 0 alias set -1 canonical type 0x7ffff5a2ec78 pointer_to_this <pointer_type 0x7ffff5a41f18>> _BUT_(!) its main-variant is (gdb) call debug_tree (gimple_assign_rhs1 (stmt)->typed.type->type_common.main_variant) <record_type 0x7ffff5b4a000 array3_real(kind=8) type_1 BLK size <integer_cst 0x7ffff5b3e700 type <integer_type 0x7ffff5a2e0a8 bitsizetype> constant 768> unit size <integer_cst 0x7ffff5b1fc60 type <integer_type 0x7ffff5a2e000 sizetype> constant 96> align 64 symtab 0 alias set -1 canonical type 0x7ffff5b4a0a8 fields <field_decl 0x7ffff5b3d390 data type <pointer_type 0x7ffff5a41e70 type <void_type 0x7ffff5a2ebd0 void> unsigned restrict DI size <integer_cst 0x7ffff5a1dec0 constant 64> unit size <integer_cst 0x7ffff5a1dee0 constant 8> align 64 symtab 0 alias set -1 canonical type 0x7ffff5a41e70> thus has a restrict-qualified data field. That's bogus as TYPE_FIELDS is supposed to be shared amongst variant types.
Index: gcc/fortran/trans-types.c =================================================================== --- gcc/fortran/trans-types.c (revision 184203) +++ gcc/fortran/trans-types.c (working copy) @@ -2042,7 +2042,8 @@ gfc_nonrestricted_type (tree t) } if (!field) break; - ret = build_variant_type_copy (t); + ret = build_distinct_type_copy (t); + TYPE_CANONICAL (ret) = TYPE_CANONICAL (t); TYPE_FIELDS (ret) = NULL_TREE; /* Here we make sure that as soon as we know we have to copy works here, but I suspect it would not fix the Fortran -flto ICEs in PR51765 (which happen on pristine trunk, much to the same issue I suppose).
*** Bug 52516 has been marked as a duplicate of this bug. ***
Now exposed again.
The patch in comment #64 fixes the failures reported in pr52516 but introduces many regressions: === gfortran Summary for unix/-m64 === # of expected passes 41076 # of unexpected failures 92 # of expected failures 56 # of unresolved testcases 12 # of unsupported tests 71 === gfortran Summary === # of expected passes 81865 # of unexpected failures 184 # of expected failures 112 # of unresolved testcases 24 # of unsupported tests 280 The failing tests are FAIL: gfortran.dg/alloc_comp_assign_10.f90 -O0 (internal compiler error) FAIL: gfortran.dg/alloc_comp_basics_2.f90 -O0 (internal compiler error) FAIL: gfortran.dg/alloc_comp_initializer_1.f90 -O0 (internal compiler error) FAIL: gfortran.dg/dependency_25.f90 -O0 (internal compiler error) FAIL: gfortran.dg/dependency_37.f90 -O (internal compiler error) FAIL: gfortran.dg/typebound_proc_20.f90 -O (internal compiler error) FAIL: gfortran.dg/whole_file_31.f90 -O (internal compiler error) FAIL: gfortran.dg/lto/pr45586 f_lto_pr45586_0.o assemble, -O0 -flto -flto-partition=none (internal compiler error) and the internal compiler errors are /opt/gcc/work/gcc/testsuite/gfortran.dg/alloc_comp_assign_10.f90: In function 'tao_program': /opt/gcc/work/gcc/testsuite/gfortran.dg/alloc_comp_assign_10.f90:48:0: internal compiler error: in fold_convert_loc, at fold-const.c:2016 > ... but I suspect it would not fix the Fortran -flto ICEs in > PR51765 (which happen on pristine trunk, much to the same issue I suppose). confirmed AFACT: === gfortran Summary for unix/-m64/-flto === # of expected passes 40973 # of unexpected failures 186 # of expected failures 56 # of unresolved testcases 12 # of unsupported tests 71 === gfortran Summary === # of expected passes 81659 # of unexpected failures 372 # of expected failures 112 # of unresolved testcases 24 # of unsupported tests 280
*** Bug 53318 has been marked as a duplicate of this bug. ***
(In reply to comment #63) > With a (seemingly) unrelated patch (attached to PR52097) I'm back on ICEing > for the gfortran.dg/lto/pr45586*.f90 testcases ... > > Even before the adjusted type merging we have (at compile-time) > [...] > > _BUT_(!) its main-variant is > [...] > > thus has a restrict-qualified data field. That's bogus as TYPE_FIELDS > is supposed to be shared amongst variant types. So, is this a fortran front-end bug, a middle-end bug, or a lto bug ? (Hint: PR 51765 is a marked as a lto bug ;-) )
On Sat, 28 Jul 2012, mikael at gcc dot gnu.org wrote: > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45586 > > --- Comment #69 from Mikael Morin <mikael at gcc dot gnu.org> 2012-07-28 09:46:00 UTC --- > (In reply to comment #63) > > With a (seemingly) unrelated patch (attached to PR52097) I'm back on ICEing > > for the gfortran.dg/lto/pr45586*.f90 testcases ... > > > > Even before the adjusted type merging we have (at compile-time) > > > [...] > > > > _BUT_(!) its main-variant is > > > [...] > > > > thus has a restrict-qualified data field. That's bogus as TYPE_FIELDS > > is supposed to be shared amongst variant types. > > So, is this a fortran front-end bug, a middle-end bug, or a lto bug ? > (Hint: PR 51765 is a marked as a lto bug ;-) ) It's a frontend bug.
(In reply to comment #70) > On Sat, 28 Jul 2012, mikael at gcc dot gnu.org wrote: > > So, is this a fortran front-end bug, a middle-end bug, or a lto bug ? > > (Hint: PR 51765 is a marked as a lto bug ;-) ) > > It's a frontend bug. great :-(. (In reply to comment #63) > That's bogus as TYPE_FIELDS > is supposed to be shared amongst variant types. Then we'll have to revert Micha's recursive restrict work. Is it possible for the front-end to specify alias sets by hand (I mean without relying on the middle-end computing them based on types etc)?
On Mon, 30 Jul 2012, mikael at gcc dot gnu.org wrote: > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45586 > > --- Comment #71 from Mikael Morin <mikael at gcc dot gnu.org> 2012-07-30 10:35:48 UTC --- > (In reply to comment #70) > > On Sat, 28 Jul 2012, mikael at gcc dot gnu.org wrote: > > > So, is this a fortran front-end bug, a middle-end bug, or a lto bug ? > > > (Hint: PR 51765 is a marked as a lto bug ;-) ) > > > > It's a frontend bug. > > great :-(. > > > (In reply to comment #63) > > That's bogus as TYPE_FIELDS > > is supposed to be shared amongst variant types. > > Then we'll have to revert Micha's recursive restrict work. I don't think so, it merely has to be fixed. > Is it possible for the front-end to specify alias sets by hand (I mean without > relying on the middle-end computing them based on types etc)? Yes. But that does not work with LTO, nor does it address the original issue of supporting INTENT IN/OUT properly.
(In reply to comment #72) > > (In reply to comment #63) > > > That's bogus as TYPE_FIELDS > > > is supposed to be shared amongst variant types. > > > > Then we'll have to revert Micha's recursive restrict work. > > I don't think so, it merely has to be fixed. How so? by making it non-recursive? For variable to be type compatible for assignment, they shall be variants of the same type, and thus have the same TYPE_FIELDS. If they shall have the same TYPE_FIELDS, they shall have the same components, the same components have the same types, so that one cannot be restrict qualified. > > > Is it possible for the front-end to specify alias sets by hand (I mean without > > relying on the middle-end computing them based on types etc)? > > Yes. But that does not work with LTO, You mean calling the front-end's code does not work at LTO time or the alias sets are not saved/restored for LTO? > nor does it address the original > issue of supporting INTENT IN/OUT properly. Ah? Isn't a restricted type variable (resp. component, etc) merely one that has its own alias set? So if it works with restrict, it works with alias sets?
On Mon, 30 Jul 2012, mikael at gcc dot gnu.org wrote: > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45586 > > --- Comment #73 from Mikael Morin <mikael at gcc dot gnu.org> 2012-07-30 12:29:33 UTC --- > (In reply to comment #72) > > > (In reply to comment #63) > > > > That's bogus as TYPE_FIELDS > > > > is supposed to be shared amongst variant types. > > > > > > Then we'll have to revert Micha's recursive restrict work. > > > > I don't think so, it merely has to be fixed. > How so? by making it non-recursive? > For variable to be type compatible for assignment, they shall be variants of > the same type, and thus have the same TYPE_FIELDS. > If they shall have the same TYPE_FIELDS, they shall have the same components, > the same components have the same types, so that one cannot be restrict > qualified. The types should not be variant types ;) (see my hack patch in this or another bug) > > > Is it possible for the front-end to specify alias sets by hand (I mean without > > > relying on the middle-end computing them based on types etc)? > > > > Yes. But that does not work with LTO, > You mean calling the front-end's code does not work at LTO time or the alias > sets are not saved/restored for LTO? LTO re-computes alias-sets based on types. > > nor does it address the original > > issue of supporting INTENT IN/OUT properly. > Ah? Isn't a restricted type variable (resp. component, etc) merely one that has > its own alias set? So if it works with restrict, it works with alias sets? No, alias-sets and restrict are different beasts. It's important to have the data member restrict qualified for INTENT IN/OUT.
Created attachment 27919 [details] rough patch (In reply to comment #74) > > For variable to be type compatible for assignment, they shall be variants of > > the same type, and thus have the same TYPE_FIELDS. > > If they shall have the same TYPE_FIELDS, they shall have the same components, > > the same components have the same types, so that one cannot be restrict > > qualified. > > The types should not be variant types ;) (see my hack patch in > this or another bug) Ah OK, I thought it was just a hack. > > > nor does it address the original > > > issue of supporting INTENT IN/OUT properly. > > Ah? Isn't a restricted type variable (resp. component, etc) merely one that has > > its own alias set? So if it works with restrict, it works with alias sets? > > No, alias-sets and restrict are different beasts. It's important to > have the data member restrict qualified for INTENT IN/OUT. Ah. I have attached a (very rough) patch which is already in the testsuite past the first failing cases reported by Dominique. About your fix: (In reply to comment #64) > Index: gcc/fortran/trans-types.c > =================================================================== > --- gcc/fortran/trans-types.c (revision 184203) > +++ gcc/fortran/trans-types.c (working copy) > @@ -2042,7 +2042,8 @@ gfc_nonrestricted_type (tree t) > } > if (!field) > break; > - ret = build_variant_type_copy (t); > + ret = build_distinct_type_copy (t); > + TYPE_CANONICAL (ret) = TYPE_CANONICAL (t); > TYPE_FIELDS (ret) = NULL_TREE; > > /* Here we make sure that as soon as we know we have to copy there is another call to build_variant_type_copy before about array types. Should the same fix by applied or are variants OK there?
On Wed, 1 Aug 2012, mikael at gcc dot gnu.org wrote: > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45586 > > --- Comment #75 from Mikael Morin <mikael at gcc dot gnu.org> 2012-08-01 12:22:03 UTC --- > Created attachment 27919 [details] > --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27919 > rough patch > > (In reply to comment #74) > > > For variable to be type compatible for assignment, they shall be variants of > > > the same type, and thus have the same TYPE_FIELDS. > > > If they shall have the same TYPE_FIELDS, they shall have the same components, > > > the same components have the same types, so that one cannot be restrict > > > qualified. > > > > The types should not be variant types ;) (see my hack patch in > > this or another bug) > Ah OK, I thought it was just a hack. > > > > > nor does it address the original > > > > issue of supporting INTENT IN/OUT properly. > > > Ah? Isn't a restricted type variable (resp. component, etc) merely one that has > > > its own alias set? So if it works with restrict, it works with alias sets? > > > > No, alias-sets and restrict are different beasts. It's important to > > have the data member restrict qualified for INTENT IN/OUT. > Ah. > > I have attached a (very rough) patch which is already in the testsuite past the > first failing cases reported by Dominique. > About your fix: > (In reply to comment #64) > > Index: gcc/fortran/trans-types.c > > =================================================================== > > --- gcc/fortran/trans-types.c (revision 184203) > > +++ gcc/fortran/trans-types.c (working copy) > > @@ -2042,7 +2042,8 @@ gfc_nonrestricted_type (tree t) > > } > > if (!field) > > break; > > - ret = build_variant_type_copy (t); > > + ret = build_distinct_type_copy (t); > > + TYPE_CANONICAL (ret) = TYPE_CANONICAL (t); > > TYPE_FIELDS (ret) = NULL_TREE; > > > > /* Here we make sure that as soon as we know we have to copy > there is another call to build_variant_type_copy before about array types. > Should the same fix by applied or are variants OK there? You mean case ARRAY_TYPE: { tree elemtype = gfc_nonrestricted_type (TREE_TYPE (t)); if (elemtype == TREE_TYPE (t)) ret = t; else { ret = build_variant_type_copy (t); ? Yes, that also should be build_distinct_type_copy. Richard.
(In reply to comment #75) > Created attachment 27919 [details] > rough patch > About the patch: The failures are mostly(all?) due to structure constructors. Structure constructors use components' backend_decl which are of restricted types. however, if the lhs is a target, one should use the non-restricted components. I tried fixing the constructor using gfc_convert (see the patch). However, it doesn't work with scalarization, as the constructor is evaluated outside the loop into a variable (which gets the restricted type). So, this patch adds a flag restricted to gfc_conv_structure. the flag propagates to gfc_conv_expr, and gfc_conv_initializer, gfc_conv_initializer, etc. To know whether we need to call gfc_conv_expr with the restricted flag set or not in the scalarizer, a new flag field is added to the scalarizer's gfc_ss_info structures to tell whether we want a non-restricted expression. For it to be set appropriately, a flag is propagated to the gfc_walk_* functions.
(In reply to comment #76) > You mean > > [...] > > ? Yes, that also should be build_distinct_type_copy. > Even without that, the patch regtests cleanly (including the pr45586 tests), apart for typebound_proc_20.f90. That one fail in the following line from the `calc' subroutine: this%y = this%find_y() ! FAILS the lhs is a target, and the rhs is NOT a target, so that the middle-end types are different. :-(
On Wed, 1 Aug 2012, mikael at gcc dot gnu.org wrote: > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45586 > > --- Comment #78 from Mikael Morin <mikael at gcc dot gnu.org> 2012-08-01 15:01:59 UTC --- > (In reply to comment #76) > > You mean > > > > [...] > > > > ? Yes, that also should be build_distinct_type_copy. > > > Even without that, the patch regtests cleanly (including the pr45586 tests), > apart for typebound_proc_20.f90. That one fail in the following line from the > `calc' subroutine: > > this%y = this%find_y() ! FAILS > > the lhs is a target, and the rhs is NOT a target, so that the middle-end types > are different. :-( But how can this be a valid fortran program? You assign something that is not aliased to something that suddenly makes it aliased? If that's valid then you can make the middle-end happy by wrapping the RHS inside a VIEW_CONVERT_EXPR with the LHS type.
(In reply to comment #79) > > this%y = this%find_y() ! FAILS > > > > the lhs is a target, and the rhs is NOT a target, so that the middle-end types > > are different. :-( > > But how can this be a valid fortran program? You assign something > that is not aliased to something that suddenly makes it aliased? > If that's valid then you can make the middle-end happy by wrapping > the RHS inside a VIEW_CONVERT_EXPR with the LHS type. I have not closely looked at the dump, however, this%y = this%find_y() means that one assigns component-wise the values from the RHS to the LHS; if there are pointer components, the pointer address is assigned; if there are allocatable components, those are - if needed - first (re)allocated and then (element-wise) assigned. Thus, one only assigns the values and no pointers - and, hence, the RHS can be a nontarget while the LHS can be a target. In this example, "this%y" is a derived type with an allocatable-array component. I think the current algorithm is something like: *dst = *src; if (src->data) { (re)allocate dst->data memcpy (dst->data, src->data); } Thus, one first transfers the pointer address [besides the array bounds], but if "data" is not NULL, the data is copied. Thus, it should be safe - but an ARRAY_RANGE_REF could be nicer than a memcpy, and the VIEW_CONVERT_EXPR could be inserted.
(In reply to comment #79) > If that's valid then you can make the middle-end happy by wrapping > the RHS inside a VIEW_CONVERT_EXPR with the LHS type. OK. will try. I don't know yet though how to limit that to the very cases where it is necessary. (In reply to comment #80) > I have not closely looked at the dump, however, > this%y = this%find_y() > means that one assigns component-wise the values from the RHS to the LHS; if > there are pointer components, the pointer address is assigned; if there are > allocatable components, those are - if needed - first (re)allocated and then > (element-wise) assigned. > > Thus, one only assigns the values and no pointers - and, hence, the RHS can be > a nontarget while the LHS can be a target. > Actually, there is a wrong-code bug about exactly this line (see bugs #47455 and #47586). But I think that even if there wasn't, the middle-end wouldn't be happy about it. The values are assigned component-wise, and that's the only way the middle-end would accept it.
URL for the current version of the patch added.
Mikael, any progress on this one (BTW, the PR is not yet assigned)? It would be great to have LTO work with Fortran in 4.8 (especially with all the inlining improvements). However, I would guess that this is stage 1 material, and I'm assuming stage 1 is nearing its end.
(In reply to comment #83) > any progress on this one? Well, the patch at http://gcc.gnu.org/ml/fortran/2012-08/msg00150.html solves several issues, but it still has issues, cf. http://gcc.gnu.org/ml/fortran/2012-08/msg00176.html > It would be great to have LTO work with Fortran in 4.8 Well, it does for many codes; also the issue is merely* a tree-checking issues thus it should work* when GCC is compiled with release checking. (* As the tree-checking failure is related to the "restrict" qualifier / pointer/target attribute, the issue of this PR might lead to wrong code. But seemingly, the issue doesn't surface in most codes.)
(In reply to comment #84) > (In reply to comment #83) > > any progress on this one? > > Well, the patch at http://gcc.gnu.org/ml/fortran/2012-08/msg00150.html solves > several issues, but it still has issues, cf. > http://gcc.gnu.org/ml/fortran/2012-08/msg00176.html And some of the remaining issues are not present on trunk, so the patch is not a complete improvement. I'm not actively working on this (nor anything else actually) any more.
(In reply to comment #84) LTO might work for many codes, as using allocatables in derived types was not standard Fortran90 (IIRC) and appears needed to trigger the bug. Anyway, since most people will use released versions of gcc, this checking error will be hidden behind --enable-checking=release. Only very few people will be able to locate and in particular reduce wrong code generation that only happens with LTO, so I wouldn't expect bug reports for actual wrong code generation very quickly. Meanwhile a shorter testcase for 4.8, using gfortran -flto -O0. TYPE t REAL, DIMENSION(:), ALLOCATABLE :: r END TYPE t TYPE t_p TYPE(t), POINTER :: d_t END TYPE t_p REAL, DIMENSION(:), POINTER :: d TYPE(t_p) :: x d=>x%d_t%r END
I noticed this same error on the pr45586*.f90 test cases, but the reduced test case in Comment #86 ICEs for me too on powerpc64-linux.
reconfirmed with current trunk.
Just to repeat, the ICEs are with checking enabled only (but possibly cover up for wong-code). I believe the correct solution will involve implementing the proposal for introducing explicit restrict tags and using that in the fortran frontend (removing the restrict qualification work). See http://gcc.gnu.org/ml/gcc-patches/2011-10/msg01011.html.
(In reply to comment #89) > Just to repeat, the ICEs are with checking enabled only (but possibly cover up > for wong-code). I'm indeed worried that the release branches will as a result silently miscompile Fortran code in LTO mode, but I appreciate that the problem is hard to fix correctly. I wonder if an intermediate solution would be dropping the 'restrict qualifier' (in default of a better term) from allocatable components of derived types. This is a very small set of variables (as this was not allowed in Fortran90, IIRC) and should have small impact on the performance of typical programs. In exchange one would be able to use LTO without the risk of miscompilation, and presumably with significant benefit.
FWIW, fails for -mabi=32 on mips*-linux-gnu too (but not -mabi=n32 or -mabi=64).
GCC 4.8.0 is being released, adjusting target milestone.
reconfirmed with current (4.9) trunk. 4.8 branch now passes, obviously. An unfortunate side-effect is that I can't test LTO in my trunk based, checking enabled, tester. So, I overlooked that LTO fails, for a different reason, to build CP2K (I'll try to file a separate PR in the near future for that).
GCC 4.8.1 has been released.
Reconfirmed the failure of the testcase from comment #86 with current trunk...
GCC 4.8.2 has been released.
(In reply to Richard Biener from comment #89) > I believe the correct solution will involve implementing the proposal for > introducing explicit restrict tags and using that in the fortran frontend > (removing the restrict qualification work). > > See http://gcc.gnu.org/ml/gcc-patches/2011-10/msg01011.html. I guess that has now become: http://gcc.gnu.org/ml/gcc-patches/2013-11/msg02727.html progress!
It was not commited yet, or was it please? I still cannot compile scipy with -flto by trunk gcc.
Author: rguenth Date: Thu Jan 9 11:52:43 2014 New Revision: 206461 URL: http://gcc.gnu.org/viewcvs?rev=206461&root=gcc&view=rev Log: 2014-01-09 Richard Biener <rguenther@suse.de> PR lto/45586 * lto.c (hash_canonical_type): Do not hash TREE_ADDRESSABLE, TYPE_ALIGN, TYPE_RESTRICT or TYPE_REF_CAN_ALIAS_ALL. (gimple_canonical_types_compatible_p): Do not compare them either. Modified: trunk/gcc/lto/ChangeLog trunk/gcc/lto/lto.c
Fixed on trunk.
(In reply to Richard Biener from comment #100) > Fixed on trunk. Hi Richard, Do you intend to backport this to 4.8 or, come to that, is it even backportable? If 'no' to either, I'll close it out (I am engaged in regression bashing this afternoon). Cheers Paul PS Thanks to everybody for their intestinal fortitude and persistence in fixing this PR!
On February 8, 2014 1:47:14 PM GMT+01:00, "pault at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org> wrote: >http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45586 > >Paul Thomas <pault at gcc dot gnu.org> changed: > > What |Removed |Added >---------------------------------------------------------------------------- > CC| |pault at gcc dot gnu.org > >--- Comment #101 from Paul Thomas <pault at gcc dot gnu.org> --- >(In reply to Richard Biener from comment #100) >> Fixed on trunk. > >Hi Richard, > >Do you intend to backport this to 4.8 or, come to that, is it even >backportable? If 'no' to either, I'll close it out (I am engaged in >regression >bashing this afternoon). I don't intend to backport this. Richard. >Cheers > >Paul > >PS Thanks to everybody for their intestinal fortitude and persistence >in fixing >this PR!
(In reply to rguenther@suse.de from comment #102) > On February 8, 2014 1:47:14 PM GMT+01:00, "pault at gcc dot gnu.org" > >> Fixed on trunk. > I don't intend to backport this. Thus fixed for 4.9 & won't fix for earlier branches. Thanks!