Re: [Patch, Fortran, pr60322] was: [Patch 1/2, Fortran, pr60322] [OOP] Incorrect bounds on polymorphic dummy array
Dear Andre, I am in the UK as of last night. Before leaving, I bootstrapped and regtested your patch and all was well. I must drive to Cambridge this afternoon to see my mother and will try to get to it either this evening or tomorrow morning. There is so much of it and it touches many places; so I must give it a very careful looking over before giving the green light. Bear with me please. Great work though! Paul On 24 March 2015 at 18:06, Andre Vehreschild ve...@gmx.de wrote: Hi all, I have worked on the comments Mikael gave me. I am now checking for class_pointer in the way he pointed out. Furthermore did I *join the two parts* of the patch into this one, because keeping both in sync was no benefit but only tedious and did not prove to be reviewed faster. Paul, Dominique: I have addressed the LOC issue that came up lately. Or rather the patch addressed it already. I feel like this is not tested very well, not the loc() call nor the sizeof() call as given in the 57305 second's download. Unfortunately, is that download not runable. I would love to see a test similar to that download, but couldn't come up with one, that satisfied me. Given that the patch's review will last some days, I still have enough time to come up with something beautiful which I will add then. Bootstraps and regtests ok on x86_64-linux-gnu/F20. Regards, Andre On Tue, 24 Mar 2015 11:13:27 +0100 Paul Richard Thomas paul.richard.tho...@gmail.com wrote: Dear Andre, Dominique pointed out to me that the 'loc' patch causes a ICE in the testsuite. It seems that 'loc' should provide the address of the class container in some places and the address of the data in others. I will put my thinking cap on tonight :-) Cheers Paul On 23 March 2015 at 13:43, Andre Vehreschild ve...@gmx.de wrote: Hi Mikael, thanks for looking at the patch. Please note, that Paul has sent an addendum to the patches for 60322, which I deliberately have attached. 26/02/2015 18:17, Andre Vehreschild a écrit : This first patch is only preparatory and does not change any of the semantics of gfortran at all. Sure? With the counterexample you found below, this of course is a wrong statement. diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c index ab6f7a5..d28cf77 100644 --- a/gcc/fortran/expr.c +++ b/gcc/fortran/expr.c @@ -4059,10 +4060,10 @@ gfc_lval_expr_from_sym (gfc_symbol *sym) lval-symtree = gfc_find_symtree (sym-ns-sym_root, sym-name); /* It will always be a full array. */ - lval-rank = sym-as ? sym-as-rank : 0; + as = sym-as; + lval-rank = as ? as-rank : 0; if (lval-rank) -gfc_add_full_array_ref (lval, sym-ts.type == BT_CLASS ? - CLASS_DATA (sym)-as : sym-as); +gfc_add_full_array_ref (lval, as); This is a change of semantics. Or do you know that sym-ts.type != BT_CLASS? You are completely right. I have made a mistake here. I have to tell the truth, I never ran a regtest with only part 1 of the patches applied. The second part of the patch will correct this, by setting the variable as depending on whether type == BT_CLASS or not. Sorry for the mistake. diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c index 3664824..e571a17 100644 --- a/gcc/fortran/trans-decl.c +++ b/gcc/fortran/trans-decl.c @@ -1013,16 +1017,24 @@ gfc_build_dummy_array_decl (gfc_symbol * sym, tree dummy) tree decl; tree type; gfc_array_spec *as; + symbol_attribute *array_attr; char *name; gfc_packed packed; int n; bool known_size; - if (sym-attr.pointer || sym-attr.allocatable - || (sym-as sym-as-type == AS_ASSUMED_RANK)) + /* Use the array as and attr. */ + as = sym-as; + array_attr = sym-attr; + + /* The pointer attribute is always set on a _data component, therefore check + the sym's attribute only. */ + if (sym-attr.pointer || array_attr-allocatable + || (as as-type == AS_ASSUMED_RANK)) return dummy; Any reason to sometimes use array_attr, sometimes not, like here? By the way, the comment is misleading: for classes, there is the class_pointer attribute (and it is a pain, I know). Yes, and a good one. Array_attr is sometimes sym-attr and sometimes CLASS_DATA(sym)-attr aka sym-ts.u.derived-components-attr. In the later case .pointer is always set to 1 in the _data component's attr. I.e., the above if, would always yield true for a class_array, which is not intended, but rather destructive. I know about the class_pointer attribute, but I figured, that it is not relevant here. Any idea how to formulate the comment better, to reflect what I just explained? Regards, Andre -- Andre Vehreschild * Email: vehre ad gmx dot de -- Forwarded message -- From: Paul Richard Thomas paul.richard.tho...@gmail.com
Re: [Patch 1/2, Fortran, pr60322] [OOP] Incorrect bounds on polymorphic dummy array
Dear Andre, Dominique pointed out to me that the 'loc' patch causes a ICE in the testsuite. It seems that 'loc' should provide the address of the class container in some places and the address of the data in others. I will put my thinking cap on tonight :-) Cheers Paul On 23 March 2015 at 13:43, Andre Vehreschild ve...@gmx.de wrote: Hi Mikael, thanks for looking at the patch. Please note, that Paul has sent an addendum to the patches for 60322, which I deliberately have attached. 26/02/2015 18:17, Andre Vehreschild a écrit : This first patch is only preparatory and does not change any of the semantics of gfortran at all. Sure? With the counterexample you found below, this of course is a wrong statement. diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c index ab6f7a5..d28cf77 100644 --- a/gcc/fortran/expr.c +++ b/gcc/fortran/expr.c @@ -4059,10 +4060,10 @@ gfc_lval_expr_from_sym (gfc_symbol *sym) lval-symtree = gfc_find_symtree (sym-ns-sym_root, sym-name); /* It will always be a full array. */ - lval-rank = sym-as ? sym-as-rank : 0; + as = sym-as; + lval-rank = as ? as-rank : 0; if (lval-rank) -gfc_add_full_array_ref (lval, sym-ts.type == BT_CLASS ? - CLASS_DATA (sym)-as : sym-as); +gfc_add_full_array_ref (lval, as); This is a change of semantics. Or do you know that sym-ts.type != BT_CLASS? You are completely right. I have made a mistake here. I have to tell the truth, I never ran a regtest with only part 1 of the patches applied. The second part of the patch will correct this, by setting the variable as depending on whether type == BT_CLASS or not. Sorry for the mistake. diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c index 3664824..e571a17 100644 --- a/gcc/fortran/trans-decl.c +++ b/gcc/fortran/trans-decl.c @@ -1013,16 +1017,24 @@ gfc_build_dummy_array_decl (gfc_symbol * sym, tree dummy) tree decl; tree type; gfc_array_spec *as; + symbol_attribute *array_attr; char *name; gfc_packed packed; int n; bool known_size; - if (sym-attr.pointer || sym-attr.allocatable - || (sym-as sym-as-type == AS_ASSUMED_RANK)) + /* Use the array as and attr. */ + as = sym-as; + array_attr = sym-attr; + + /* The pointer attribute is always set on a _data component, therefore check + the sym's attribute only. */ + if (sym-attr.pointer || array_attr-allocatable + || (as as-type == AS_ASSUMED_RANK)) return dummy; Any reason to sometimes use array_attr, sometimes not, like here? By the way, the comment is misleading: for classes, there is the class_pointer attribute (and it is a pain, I know). Yes, and a good one. Array_attr is sometimes sym-attr and sometimes CLASS_DATA(sym)-attr aka sym-ts.u.derived-components-attr. In the later case .pointer is always set to 1 in the _data component's attr. I.e., the above if, would always yield true for a class_array, which is not intended, but rather destructive. I know about the class_pointer attribute, but I figured, that it is not relevant here. Any idea how to formulate the comment better, to reflect what I just explained? Regards, Andre -- Andre Vehreschild * Email: vehre ad gmx dot de -- Forwarded message -- From: Paul Richard Thomas paul.richard.tho...@gmail.com To: Andre Vehreschild ve...@gmx.de, Dominique Dhumieres domi...@lps.ens.fr Cc: Date: Sun, 22 Mar 2015 21:20:20 +0100 Subject: Bug in intrinsic LOC for scalar class objects Dear Andre and Dominique, I have found that LOC is returning the address of the class container rather than the _data component for class scalars. See the source below, which you will recognise! A fix is attached. Note that the scalar allocate fails with MOLD= and so I substituted SOURCE=. Cheers Paul class(*), allocatable :: a(:), e ! Change 'e' to an array and second memcpy works correctly ! Problem is with loc(e), which returns the address of the ! class container. allocate (e, source = 99.0) allocate (a(2), source = [1.0, 2.0]) call add_element_poly (a,e) select type (a) type is (real) print *, a end select contains subroutine add_element_poly(a,e) use iso_c_binding class(*),allocatable,intent(inout),target :: a(:) class(*),intent(in),target :: e class(*),allocatable,target :: tmp(:) type(c_ptr) :: dummy interface function memcpy(dest,src,n) bind(C,name=memcpy) result(res) import type(c_ptr) :: res integer(c_intptr_t),value :: dest integer(c_intptr_t),value :: src integer(c_size_t),value :: n end function end interface if (.not.allocated(a)) then allocate(a(1), source=e) else allocate
Re: [Patch, Fortran, pr55901, v1] [OOP] type is (character(len=*)) misinterpreted as array
Dear Andre, Yes, that's right. The first three (vtab rework 1/2 and pr64787) are combined and reformatted in the .diff file that I sent you. Please use that and then apply the pr55901 patch. This is what I am okaying. Cheers Paul On 23 March 2015 at 10:45, Andre Vehreschild ve...@gmx.de wrote: Hi Paul, thanks for the reviews. Let me ask one questions before I do something wrong. You have reviewed and approved (with changes) the patches: - vtab_access_rework1_v1.patch https://gcc.gnu.org/ml/fortran/2015-03/msg00074.html - vtab_access_rework2_v1.patch https://gcc.gnu.org/ml/fortran/2015-03/msg00075.html - pr64787_v2.patch https://gcc.gnu.org/ml/fortran/2015-03/msg00085.html and - pr55901_v1.patch https://gcc.gnu.org/ml/fortran/2015-03/msg00086.html , right? I am asking so explicitly, because there are four more patches from me in the wild, that await review (not necessarily from you, Paul), namely: - pr60322_base_1.patch https://gcc.gnu.org/ml/fortran/2015-02/msg00105.html - pr60322_3.patch https://gcc.gnu.org/ml/fortran/2015-03/msg00032.html - crashfix2_v1.patch (small patch, ~100 loc)) https://gcc.gnu.org/ml/fortran/2015-03/msg00063.html and - cosm_simp.patch (tiny patch, ~20 loc) https://gcc.gnu.org/ml/fortran/2015-03/msg00088.html Please don't get me wrong on this. I just want to prevent misunderstandings here. The latter four patches are not yet approved, right? I will now apply the 4.9-trunk patch and wait for your answer before applying the above four on vtab_rework pr64787 and pr55901. Regards, Andre On Mon, 23 Mar 2015 08:33:51 +0100 Paul Richard Thomas paul.richard.tho...@gmail.com wrote: Dear Andre, I am persuaded by the arguments of Jerry and Dominique that this is good for trunk. Please commit as early as possible in order that any regressions can be caught, if possible, before release. Thanks Paul On 21 March 2015 at 15:11, Paul Richard Thomas paul.richard.tho...@gmail.com wrote: Dear Andre, I have applied the three preliminary patches but have not yet applied the attached one for PR55901. As advertised the composite patch bootstraps and regtests on FC21,x86_64. I went through gfc_trans_allocate and cleaned up the formatting and some of the text in the comments. You did a heroic job to tidy up this function and so I thought that I should do my bit - one of the feature, previously, was that the line length often went well in excess of the gcc style guide limit of 72 and this tended to make it somewhat unreadable. I have not been rigorous about this, especially when readability would be impaired thereby, but it does look a lot better now. The composite diff is attached. Not only does the Metcalf example run correctly but also the PGI Insider linked list example. I have attached a version of this modified to function as a gfortran.dg testcase. With the attributions in there, I do not think that there are any copyright issues. The article itself has no copyright notice. I would very much like to say that this is OK for trunk but we are hard up against the end of stage 4 and so it should really wait for backporting to 5.2. Thanks for the patches Paul On 19 March 2015 at 16:13, Andre Vehreschild ve...@gmx.de wrote: Hi all, please find attached the parts missing to stop valgrind's complaining about the use of uninitialized memory. The issue was, that when constructing a temporary class-object to call a routine with unlimited polymorphic arguments, the _len component was never set. This is fixed by this patch now. Note, the patch is based on all these preliminary patches: https://gcc.gnu.org/ml/fortran/2015-03/msg00074.html https://gcc.gnu.org/ml/fortran/2015-03/msg00075.html https://gcc.gnu.org/ml/fortran/2015-03/msg00085.html Bootstraps and regtests ok on x86_64-linux-gnu/F20. Please review! - Andre -- Andre Vehreschild * Email: vehre ad gmx dot de -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx -- Andre Vehreschild * Email: vehre ad gmx dot de -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [Patch, Fortran, pr55901, v1] [OOP] type is (character(len=*)) misinterpreted as array
Dear Andre, I am persuaded by the arguments of Jerry and Dominique that this is good for trunk. Please commit as early as possible in order that any regressions can be caught, if possible, before release. Thanks Paul On 21 March 2015 at 15:11, Paul Richard Thomas paul.richard.tho...@gmail.com wrote: Dear Andre, I have applied the three preliminary patches but have not yet applied the attached one for PR55901. As advertised the composite patch bootstraps and regtests on FC21,x86_64. I went through gfc_trans_allocate and cleaned up the formatting and some of the text in the comments. You did a heroic job to tidy up this function and so I thought that I should do my bit - one of the feature, previously, was that the line length often went well in excess of the gcc style guide limit of 72 and this tended to make it somewhat unreadable. I have not been rigorous about this, especially when readability would be impaired thereby, but it does look a lot better now. The composite diff is attached. Not only does the Metcalf example run correctly but also the PGI Insider linked list example. I have attached a version of this modified to function as a gfortran.dg testcase. With the attributions in there, I do not think that there are any copyright issues. The article itself has no copyright notice. I would very much like to say that this is OK for trunk but we are hard up against the end of stage 4 and so it should really wait for backporting to 5.2. Thanks for the patches Paul On 19 March 2015 at 16:13, Andre Vehreschild ve...@gmx.de wrote: Hi all, please find attached the parts missing to stop valgrind's complaining about the use of uninitialized memory. The issue was, that when constructing a temporary class-object to call a routine with unlimited polymorphic arguments, the _len component was never set. This is fixed by this patch now. Note, the patch is based on all these preliminary patches: https://gcc.gnu.org/ml/fortran/2015-03/msg00074.html https://gcc.gnu.org/ml/fortran/2015-03/msg00075.html https://gcc.gnu.org/ml/fortran/2015-03/msg00085.html Bootstraps and regtests ok on x86_64-linux-gnu/F20. Please review! - Andre -- Andre Vehreschild * Email: vehre ad gmx dot de -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [Patch, Fortran, pr55901, v1] [OOP] type is (character(len=*)) misinterpreted as array
Dear Andre, I have applied the three preliminary patches but have not yet applied the attached one for PR55901. As advertised the composite patch bootstraps and regtests on FC21,x86_64. I went through gfc_trans_allocate and cleaned up the formatting and some of the text in the comments. You did a heroic job to tidy up this function and so I thought that I should do my bit - one of the feature, previously, was that the line length often went well in excess of the gcc style guide limit of 72 and this tended to make it somewhat unreadable. I have not been rigorous about this, especially when readability would be impaired thereby, but it does look a lot better now. The composite diff is attached. Not only does the Metcalf example run correctly but also the PGI Insider linked list example. I have attached a version of this modified to function as a gfortran.dg testcase. With the attributions in there, I do not think that there are any copyright issues. The article itself has no copyright notice. I would very much like to say that this is OK for trunk but we are hard up against the end of stage 4 and so it should really wait for backporting to 5.2. Thanks for the patches Paul On 19 March 2015 at 16:13, Andre Vehreschild ve...@gmx.de wrote: Hi all, please find attached the parts missing to stop valgrind's complaining about the use of uninitialized memory. The issue was, that when constructing a temporary class-object to call a routine with unlimited polymorphic arguments, the _len component was never set. This is fixed by this patch now. Note, the patch is based on all these preliminary patches: https://gcc.gnu.org/ml/fortran/2015-03/msg00074.html https://gcc.gnu.org/ml/fortran/2015-03/msg00075.html https://gcc.gnu.org/ml/fortran/2015-03/msg00085.html Bootstraps and regtests ok on x86_64-linux-gnu/F20. Please review! - Andre -- Andre Vehreschild * Email: vehre ad gmx dot de -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx Index: gcc/fortran/class.c === *** gcc/fortran/class.c (revision 221500) --- gcc/fortran/class.c (working copy) *** gfc_add_component_ref (gfc_expr *e, cons *** 234,239 --- 234,242 } if (*tail != NULL strcmp (name, _data) == 0) next = *tail; + else + /* Avoid losing memory. */ + gfc_free_ref_list (*tail); (*tail) = gfc_get_ref(); (*tail)-next = next; (*tail)-type = REF_COMPONENT; *** find_intrinsic_vtab (gfc_typespec *ts) *** 2562,2574 c-attr.access = ACCESS_PRIVATE; /* Build a minimal expression to make use of !target-memory.c/gfc_element_size for 'size'. */ e = gfc_get_expr (); e-ts = *ts; e-expr_type = EXPR_VARIABLE; c-initializer = gfc_get_int_expr (gfc_default_integer_kind, NULL, !(int)gfc_element_size (e)); gfc_free_expr (e); /* Add component _extends. */ --- 2565,2583 c-attr.access = ACCESS_PRIVATE; /* Build a minimal expression to make use of !target-memory.c/gfc_element_size for 'size'. Special handling !for character arrays, that are not constant sized: to support !len(str)*kind, only the kind information is stored in the !vtab. */ e = gfc_get_expr (); e-ts = *ts; e-expr_type = EXPR_VARIABLE; c-initializer = gfc_get_int_expr (gfc_default_integer_kind, NULL, !ts-type == BT_CHARACTER ! charlen == 0 ? ! ts-kind : ! (int)gfc_element_size (e)); gfc_free_expr (e); /* Add component _extends. */ Index: gcc/fortran/gfortran.h === *** gcc/fortran/gfortran.h (revision 221500) --- gcc/fortran/gfortran.h (working copy) *** void gfc_add_component_ref (gfc_expr *, *** 3168,3173 --- 3168,3174 void gfc_add_class_array_ref (gfc_expr *); #define gfc_add_data_component(e) gfc_add_component_ref(e,_data) #define gfc_add_vptr_component(e) gfc_add_component_ref(e,_vptr) + #define gfc_add_len_component(e) gfc_add_component_ref(e,_len) #define gfc_add_hash_component(e) gfc_add_component_ref(e,_hash) #define gfc_add_size_component(e) gfc_add_component_ref(e,_size) #define gfc_add_def_init_component(e) gfc_add_component_ref(e,_def_init) Index: gcc/fortran/trans-array.c
Re: [Patch, fortran] PR59198 - [4.8/4.9/5 Regression] ICE on cyclically dependent polymorphic types
Oh bother! Thanks Jakub - I did wonder about that but thought that the same function would yield the same result :-( Thanks. I will put it right. Paul On 17 March 2015 at 18:32, Jakub Jelinek ja...@redhat.com wrote: On Tue, Mar 17, 2015 at 06:28:03AM +0100, Paul Richard Thomas wrote: Dear Tobias, As far as I can see, without the patch, gfc_get _derived_type goes into a continuous loop trying to build the abstract type. Why this is not the case with an additional non-procedure pointer component, I do not know. I suspect that there is a corner case out there that will challenge this patch but I was unable to generate it. I decided therefore to commit, with an additional condition in the loop to prevent repeated attempts to build the component field. Committed to trunk as revision 221474. Note, the proc_ptr_comp_45.f90 testcase fails on i686-linux (unless -mfpmath=sse -msse2 or -ffloat-store) even at -O0, cos (1.570796327) is folded into constant and the runtime function doesn't return exactly the same result, because of extended precision. So, either you should allow a few ulps epsilon, or use some other function where you get reasonably exact result. Jakub -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [Patch, fortran] PR59198 - [4.8/4.9/5 Regression] ICE on cyclically dependent polymorphic types
Dear Tobias, As far as I can see, without the patch, gfc_get _derived_type goes into a continuous loop trying to build the abstract type. Why this is not the case with an additional non-procedure pointer component, I do not know. I suspect that there is a corner case out there that will challenge this patch but I was unable to generate it. I decided therefore to commit, with an additional condition in the loop to prevent repeated attempts to build the component field. Committed to trunk as revision 221474. 4.8 and 4.9 to follow. Cheers Paul On 16 March 2015 at 09:39, Paul Richard Thomas paul.richard.tho...@gmail.com wrote: Dear Tobias, I think that I have a partial understanding now and will attempt to verify it tonight. Certainly to not build the components, when a derived type is flagged to have proc_pointer components cannot be right just because there can be other components as in the original testcase. This led to the lack of DECL_SIZE information for the field 'term' and the subsequent ICE. The key to understanding is the problem with typebound_operator_9.f03, when the check for proc_pointer components is omitted completely. This implies that there is something missing from the function (sorry, I forget its name) that build proc_pointer fields. I will check that idea - enlightenment might lead to a different patch. Thanks for the review. Paul On 16 March 2015 at 09:07, Tobias Burnus tobias.bur...@physik.fu-berlin.de wrote: Dear Paul, Paul Richard Thomas wrote: The ChangeLog says it all. If the check is not done for components that are not procedure pointers, typebound_operator_9.f03 breaks. I am not entirely sure why this is the case but the fix works fine. Bootstraps and regtests on FC21/x86_64 - OK for 4.8, 4.9 and 5.0? OK. I have to admit that I also do not really understand why that's required. Tobias -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [Patch, fortran] PR59198 - [4.8/4.9/5 Regression] ICE on cyclically dependent polymorphic types
Dear Tobias, I think that I have a partial understanding now and will attempt to verify it tonight. Certainly to not build the components, when a derived type is flagged to have proc_pointer components cannot be right just because there can be other components as in the original testcase. This led to the lack of DECL_SIZE information for the field 'term' and the subsequent ICE. The key to understanding is the problem with typebound_operator_9.f03, when the check for proc_pointer components is omitted completely. This implies that there is something missing from the function (sorry, I forget its name) that build proc_pointer fields. I will check that idea - enlightenment might lead to a different patch. Thanks for the review. Paul On 16 March 2015 at 09:07, Tobias Burnus tobias.bur...@physik.fu-berlin.de wrote: Dear Paul, Paul Richard Thomas wrote: The ChangeLog says it all. If the check is not done for components that are not procedure pointers, typebound_operator_9.f03 breaks. I am not entirely sure why this is the case but the fix works fine. Bootstraps and regtests on FC21/x86_64 - OK for 4.8, 4.9 and 5.0? OK. I have to admit that I also do not really understand why that's required. Tobias -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
[Patch, fortran] PR59198 - [4.8/4.9/5 Regression] ICE on cyclically dependent polymorphic types
Dear All, As will be apparent from the PR, I have spent a silly amount of time on this one :-( Once I became 'de-obsessed' with the fact that the reduced testcase worked, when 'rng' was made a pointer and concentrated on the procedure pointer component 'obs1_int', finding the problem was rather more straightforward, even if not 'obvious'. The ChangeLog says it all. If the check is not done for components that are not procedure pointers, typebound_operator_9.f03 breaks. I am not entirely sure why this is the case but the fix works fine. Bootstraps and regtests on FC21/x86_64 - OK for 4.8, 4.9 and 5.0? Paul 2014-03-15 Paul Thomas pa...@gcc.gnu.org PR fortran/59198 * trans-types.c (gfc_get_derived_type): If an abstract derived type with procedure pointer components has no other type of component, return the backend_decl directly. Otherwise build the components. 2014-03-15 Paul Thomas pa...@gcc.gnu.org PR fortran/59198 * gfortran.dg/proc_ptr_comp_44.f90 : New test * gfortran.dg/proc_ptr_comp_45.f90 : New test Index: gcc/fortran/trans-types.c === *** gcc/fortran/trans-types.c (revision 221333) --- gcc/fortran/trans-types.c (working copy) *** gfc_get_derived_type (gfc_symbol * deriv *** 2448,2456 /* Its components' backend_decl have been built or we are seeing recursion through the formal arglist of a procedure pointer component. */ ! if (TYPE_FIELDS (derived-backend_decl) ! || derived-attr.proc_pointer_comp) return derived-backend_decl; else typenode = derived-backend_decl; } --- 2448,2469 /* Its components' backend_decl have been built or we are seeing recursion through the formal arglist of a procedure pointer component. */ ! if (TYPE_FIELDS (derived-backend_decl)) return derived-backend_decl; + else if (derived-attr.proc_pointer_comp derived-attr.abstract) + { + /* If an abstract derived type with procedure pointer components +has no other type of component, return the backend_decl. +Otherwise build the components. */ + for (c = derived-components; c; c = c-next) + { + if (!c-attr.proc_pointer) + break; + else if (c-next == NULL) + return derived-backend_decl; + } + typenode = derived-backend_decl; + } else typenode = derived-backend_decl; } Index: gcc/testsuite/gfortran.dg/proc_ptr_comp_44.f90 === *** gcc/testsuite/gfortran.dg/proc_ptr_comp_44.f90 (revision 0) --- gcc/testsuite/gfortran.dg/proc_ptr_comp_44.f90 (working copy) *** *** 0 --- 1,71 + ! { dg-do compile } + ! Test the fix for PR59198, where the field for the component 'term' in + ! the derived type 'decay_gen_t' was not being built. + ! + ! Contributed by Juergen Reuter juergen.reu...@desy.de + ! + module decays + abstract interface + function obs_unary_int () + end function obs_unary_int + end interface + + type, abstract :: any_config_t +contains + procedure (any_config_final), deferred :: final + end type any_config_t + + type :: decay_term_t + type(unstable_t), dimension(:), pointer :: unstable_product = null () + end type decay_term_t + + type, abstract :: decay_gen_t + type(decay_term_t), dimension(:), allocatable :: term + procedure(obs_unary_int), nopass, pointer :: obs1_int = null () + end type decay_gen_t + + type, extends (decay_gen_t) :: decay_root_t +contains + procedure :: final = decay_root_final + end type decay_root_t + + type, abstract :: rng_t + end type rng_t + + type, extends (decay_gen_t) :: decay_t + class(rng_t), allocatable :: rng +contains + procedure :: final = decay_final + end type decay_t + + type, extends (any_config_t) :: unstable_config_t +contains + procedure :: final = unstable_config_final + end type unstable_config_t + + type :: unstable_t + type(unstable_config_t), pointer :: config = null () + type(decay_t), dimension(:), allocatable :: decay + end type unstable_t + + interface + subroutine any_config_final (object) +import +class(any_config_t), intent(inout) :: object + end subroutine any_config_final + end interface + + contains + subroutine decay_root_final (object) + class(decay_root_t), intent(inout) :: object + end subroutine decay_root_final + + recursive subroutine decay_final (object) + class(decay_t), intent(inout) :: object + end subroutine decay_final + + recursive subroutine unstable_config_final (object) + class(unstable_config_t), intent(inout) :: object + end subroutine unstable_config_final + + end module
Re: [Patch, fortran] PR64952 - Missing temporary in assignment from elemental function
Dear Mikael, I have been on the road and have only had time for trivial bits and pieces. The weather forecast from tomorrow afternoon onwards is not good and so I think that this patch will get some attention :-) Cheers Paul On 12 March 2015 at 19:04, Mikael Morin mikael.mo...@sfr.fr wrote: Hello Paul, have you had time to look at this again? Mikael -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [Patch, fortran] PR65024 - [4.9/5 Regression] [OOP] ICE concerning unlimited polymorphic pointer
ping!!! On 26 February 2015 at 06:33, Paul Richard Thomas paul.richard.tho...@gmail.com wrote: Dear All, This patch has something of a band aid flavour about it. However, the more I look at it the more I like it and it cannot do any harm. In any case, I spent a silly amount of time trying to understand why this component fails to get its backend_decl in the usual way and failed. That it is specific to unlimited polymorphic components and only appears when 'bug' precedes 'next' were, in principle, giveaways that didn't do it for me. Bootstraps and regtests on x86_64/FC21 - OK for trunk and 4.9? Paul 2015-02-26 Paul Thomas pa...@gcc.gnu.org PR fortran/65024 * trans-expr.c (gfc_conv_component_ref): If the component backend declaration is missing and the derived type symbol is available in the reference, call gfc_build_derived_type. 2015-02-26 Paul Thomas pa...@gcc.gnu.org PR fortran/65024 * gfortran.dg/unlimited_polymorphic_23.f90: New test -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [Patch, Fortran] Fix ATOMIC checks
Dear Tobias, This looks kind of 'obvious' to me! OK for trunk. Thanks for the patch. Paul On 1 March 2015 at 23:35, Tobias Burnus bur...@net-b.de wrote: This patch changes the coarray/coindexed check to match what the error message already claimed. OK for the trunk? Tobias -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [Patch, fortran] PR60898 premature release of entry symbols
Dear Mikael, That looks to be OK for 4.8, 4.9 and 5.0. Strange testcase, though... :-) Thanks for the patch Paul On 15 February 2015 at 18:48, Mikael Morin mikael.mo...@sfr.fr wrote: Hello, I propose a fix for PR60898, where a symbol is freed despite remaining reachable in the symbol tree. The problem comes from this code in resolve_symbol: /* If we find that a flavorless symbol is an interface in one of the parent namespaces, find its symtree in this namespace, free the symbol and set the symtree to point to the interface symbol. */ for (ns = gfc_current_ns-parent; ns; ns = ns-parent) { symtree = gfc_find_symtree (ns-sym_root, sym-name); if (symtree [...]) { this_symtree = gfc_find_symtree (gfc_current_ns-sym_root, sym-name); gfc_release_symbol (sym); symtree-n.sym-refs++; this_symtree-n.sym = symtree-n.sym; return; } } Here, the target of an element of the current namespace's name tree is changed to point to the outer symbol. And the current symbol is freed, without checking that it really was what was in the name tree before. In the testcase https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60898#c7 , the problematic symbol is an entry, which is available in the name tree only through a mangled name (created by gfc_get_unique_symtree in get_proc_name), so gfc_find_symtree won't find it by name lookup. In this case, what gfc_find_symtree finds is a symbol that is already the outer interface symbol, so reassigning this_symtree.n.sym would be a no-op. The patch proposed checks that sym == this_symtree-n.sym, so that the symbol reassignment is only made in that case. Otherwise, the regular symbol resolution happens normally. This patch is a stripped down version of what I posted before in the PR, which contained a symbol.c part which was increasing the reference count locally in do_traverse_symtree, to delay symbol release after all of them have been processed. That part was useless because if a symbol had to be processed more than once (meaning it was available under different names), it will have the corresponding reference count set so that it won't be freed too early in any case. Worse, that part was interacting badly with the hack used to break circular references in gfc_release_symbol, so it was better left out. Anyway, this is regression tested[*] on x86_64-unknown-linux-gnu. OK for trunk/4.9/4.8 ? Mikael [*] I have a few failing testcases (also without the patch), namely the following; does this ring a bell ? FAIL: gfortran.dg/erf_3.F90 FAIL: gfortran.dg/fmt_g0_7.f08 FAIL: gfortran.dg/fmt_en.f90 FAIL: gfortran.dg/nan_7.f90 FAIL: gfortran.dg/quad_2.f90 FAIL: gfortran.dg/quad_3.f90 FAIL: gfortran.dg/round_4.f90 -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
[Patch, fortran] PR65024 - [4.9/5 Regression] [OOP] ICE concerning unlimited polymorphic pointer
Dear All, This patch has something of a band aid flavour about it. However, the more I look at it the more I like it and it cannot do any harm. In any case, I spent a silly amount of time trying to understand why this component fails to get its backend_decl in the usual way and failed. That it is specific to unlimited polymorphic components and only appears when 'bug' precedes 'next' were, in principle, giveaways that didn't do it for me. Bootstraps and regtests on x86_64/FC21 - OK for trunk and 4.9? Paul 2015-02-26 Paul Thomas pa...@gcc.gnu.org PR fortran/65024 * trans-expr.c (gfc_conv_component_ref): If the component backend declaration is missing and the derived type symbol is available in the reference, call gfc_build_derived_type. 2015-02-26 Paul Thomas pa...@gcc.gnu.org PR fortran/65024 * gfortran.dg/unlimited_polymorphic_23.f90: New test Index: gcc/fortran/trans-expr.c === *** gcc/fortran/trans-expr.c(revision 220653) --- gcc/fortran/trans-expr.c(working copy) *** gfc_conv_component_ref (gfc_se * se, gfc *** 1930,1939 c = ref-u.c.component; ! gcc_assert (c-backend_decl); field = c-backend_decl; ! gcc_assert (TREE_CODE (field) == FIELD_DECL); decl = se-expr; /* Components can correspond to fields of different containing --- 1930,1941 c = ref-u.c.component; ! if (c-backend_decl == NULL_TREE !ref-u.c.sym != NULL) ! gfc_get_derived_type (ref-u.c.sym); field = c-backend_decl; ! gcc_assert (field TREE_CODE (field) == FIELD_DECL); decl = se-expr; /* Components can correspond to fields of different containing Index: gcc/testsuite/gfortran.dg/unlimited_polymorphic_23.f90 === *** gcc/testsuite/gfortran.dg/unlimited_polymorphic_23.f90 (revision 0) --- gcc/testsuite/gfortran.dg/unlimited_polymorphic_23.f90 (working copy) *** *** 0 --- 1,35 + ! {dg-do run } + ! + ! Test the fix for PR65024, in which the structure for the 'info' + ! component of type 'T' was not being converted into TREE_SSA and + ! so caused an ICE in trans-expr.c:gfc_conv_component_ref. + ! + ! Reported by m...@gneilson.plus.com + ! + MODULE X + TYPE T + CLASS(*), pointer :: info + END TYPE + END MODULE + + PROGRAM P + call bug + CONTAINS + SUBROUTINE BUG + USE X + CLASS(T), pointer :: e + integer, target :: i = 42 + allocate(e) + e%info = NULL () ! used to ICE + if (.not.associated(e%info)) e%info = i ! used to ICE + select type (z = e%info) + type is (integer) + if (z .ne.i) call abort + end select + END SUBROUTINE + + SUBROUTINE NEXT + USE X + CLASS (T), pointer :: e + END SUBROUTINE + END
Re: [Patch, fortran] PR64952 - Missing temporary in assignment from elemental function
Dear Dominique, The patch works as advertised! I have two remarks: Of course it does :-) (1) AFAIU there is no need for a temporary for Indeed not. I cannot see how that can be avoided without a much more elaborate patch. Frankly, I do not see that it would be warranted. Much better some extra temporaries in corner cases, than wrong code. PROGRAM Main INTEGER :: i, index(5) = (/ (i, i = 1,5) /) REAL :: tmp(5), array(5) = (/ (i+0.0, i = 1,5) /) tmp = Fred(index,array) array = tmp PRINT *, array CONTAINS ELEMENTAL FUNCTION Fred (n, x) REAL :: Fred INTEGER, INTENT(IN) :: n REAL, INTENT(IN) :: x ! In general, this would be in an external procedure Fred = x+SUM(array(:n-1))+SUM(array(n+1:)) END FUNCTION Fred END PROGRAM Main However I get [Book15] f90/bug% gfc elemental_weird_db_2.f90 -Warray-temporaries elemental_weird_db_2.f90:4:10: tmp = Fred(index,array) 1 Warning: Creating array temporary at (1) [-Warray-temporaries] (2) You wrote: However, this works and has no perceivable effect on Polyhedron timings. . This is hardly a surprise since the Polyhedron tests don't use any elemental procedure. :-) You might have gathered that I didn't check! Within errors, it didn't affect compile times either. Thanks for the feedback Paul Thanks, Dominique Le 10 févr. 2015 à 23:35, Paul Richard Thomas paul.richard.tho...@gmail.com a écrit : Dear Mikael, dear all, Thank you for the previous review. I believe that the attached responds to all of your comments and correctly compiles the three testcases that you provided. Two of these have been included in the original testcase and the third appears separately. Bootstrapped and reg tested on FC21/x86_64 - OK for trunk? Cheers Paul 2015-02-10 Paul Thomas pa...@gcc.gnu.org PR fortran/64952 * gfortran.h : Add 'array_outer_dependency' to symbol_attr. * trans.h : Add 'array_outer_dependency' to gfc_ss_info. * module.c : Add AB_ARRAY_OUTER_DEPENDENCY to ab_attribute. Add same to attr_bits. (mio_symbol_attribute): Handle 'array_outer_dependency' attr in module read and write. * resolve.c (resolve_function): If an elemental function is referenced that is marked as having an external array reference and the current namespace is that of an elemental function, mark the containing function likewise. (resolve_variable): Mark elemental function symbol as 'array_outer_dependency' if it has an array reference from outside its own namespace. * trans-array.c (gfc_conv_resolve_dependencies): If any ss is marked as 'array_outer_dependency' generate a temporary. (gfc_walk_function_expr): If the function is marked as 'array_outer_dependency', likewise mark the head gfc_ss. 2015-02-10 Paul Thomas pa...@gcc.gnu.org PR fortran/64952 * gfortran.dg/elemental_dependency_4.f90: New test * gfortran.dg/elemental_dependency_5.f90: New test -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [Patch, fortran] PR64952 - Missing temporary in assignment from elemental function
Dear Mikael, dear all, Thank you for the previous review. I believe that the attached responds to all of your comments and correctly compiles the three testcases that you provided. Two of these have been included in the original testcase and the third appears separately. Bootstrapped and reg tested on FC21/x86_64 - OK for trunk? Cheers Paul 2015-02-10 Paul Thomas pa...@gcc.gnu.org PR fortran/64952 * gfortran.h : Add 'array_outer_dependency' to symbol_attr. * trans.h : Add 'array_outer_dependency' to gfc_ss_info. * module.c : Add AB_ARRAY_OUTER_DEPENDENCY to ab_attribute. Add same to attr_bits. (mio_symbol_attribute): Handle 'array_outer_dependency' attr in module read and write. * resolve.c (resolve_function): If an elemental function is referenced that is marked as having an external array reference and the current namespace is that of an elemental function, mark the containing function likewise. (resolve_variable): Mark elemental function symbol as 'array_outer_dependency' if it has an array reference from outside its own namespace. * trans-array.c (gfc_conv_resolve_dependencies): If any ss is marked as 'array_outer_dependency' generate a temporary. (gfc_walk_function_expr): If the function is marked as 'array_outer_dependency', likewise mark the head gfc_ss. 2015-02-10 Paul Thomas pa...@gcc.gnu.org PR fortran/64952 * gfortran.dg/elemental_dependency_4.f90: New test * gfortran.dg/elemental_dependency_5.f90: New test On 8 February 2015 at 19:16, Paul Richard Thomas paul.richard.tho...@gmail.com wrote: Dear Mikael, Thank you very much for the review. You raise some points that I had thought about and others that I hadn't. I also realised that such things as blocks, within the elemental function would through the fix as well. I'll defer doing anything with it until tomorrow night. I reason that there is always going to be an 'ss', although I should check that it is not gfc_ss_terminator, and that it does not matter which one is flagged. I should add a comment to that effect; it's not quite as hackish as it looks, methinks. I will be back! Paul On 8 February 2015 at 18:27, Mikael Morin mikael.mo...@sfr.fr wrote: Hello Paul, comments below Le 08/02/2015 16:24, Paul Richard Thomas a écrit : Index: gcc/fortran/gfortran.h === *** gcc/fortran/gfortran.h(revision 220482) --- gcc/fortran/gfortran.h(working copy) *** typedef struct *** 789,794 --- 789,798 cannot alias. Note that this is zero for PURE procedures. */ unsigned implicit_pure:1; + /* This set for an elemental function that contains expressions for + arrays coming from outside its namespace. */ + unsigned potentially_aliased:1; + aliased is more something about pointers, so how about naming it something like array_outer_dependency? Anyway, that's minor. I wonder whether we should negate the meaning, that is set the flag if there is no external dependency. If we can get the conditions to set it exhaustively right, both are equivalent. Otherwise... maybe not. /* This is set if the subroutine doesn't return. Currently, this is only possible for intrinsic subroutines. */ unsigned noreturn:1; Index: gcc/fortran/trans.h === *** gcc/fortran/trans.h (revision 220481) --- gcc/fortran/trans.h (working copy) *** typedef struct gfc_ss_info *** 226,231 --- 226,235 /* Suppresses precalculation of scalars in WHERE assignments. */ unsigned where:1; + /* Signals that an array argument of an elemental function might be aliased, + thereby generating a temporary in assignments. */ + unsigned potentially_aliased:1; + /* Tells whether the SS is for an actual argument which can be a NULL reference. In other words, the associated dummy argument is OPTIONAL. Used to handle elemental procedures. */ Index: gcc/fortran/resolve.c === *** gcc/fortran/resolve.c (revision 220481) --- gcc/fortran/resolve.c (working copy) *** resolve_variable (gfc_expr *e) *** 5054,5059 --- 5054,5067 gfc_current_ns-parent-parent == sym-ns))) sym-attr.host_assoc = 1; + if (sym-attr.dimension +(sym-ns != gfc_current_ns + || sym-attr.use_assoc + || sym-attr.in_common) +gfc_elemental (NULL) +gfc_current_ns-proc_name-attr.function) + gfc_current_ns-proc_name-attr.potentially_aliased = 1; I would expect the flag to also be copied between procedures in some cases; namely if A calls B, and B has the flag, then A has the flag. There is also the case of external procedures (for which the flag
[Patch, fortran] PR64952 - Missing temporary in assignment from elemental function
Dear All, This came up at https://groups.google.com/forum/#!topic/comp.lang.fortran/TvVY5j3GPmg gfortran produces wrong result from: PROGRAM Main INTEGER :: i, index(5) = (/ (i, i = 1,5) /) REAL :: array(5) = (/ (i+0.0, i = 1,5) /) array = Fred(index,array) PRINT *, array CONTAINS ELEMENTAL FUNCTION Fred (n, x) REAL :: Fred INTEGER, INTENT(IN) :: n REAL, INTENT(IN) :: x ! In general, this would be in an external procedure Fred = x+SUM(array(:n-1))+SUM(array(n+1:)) END FUNCTION Fred END PROGRAM Main outputs 15.000 29.000 56.000 109.00 214.00 when result should be 5*15.0 A temporary should be produced for array = Fred(index, array). See the clf thread for the reasoning. In a nutshell, the reason is: The execution of the assignment shall have the same effect as if the evaluation of expr and the evaluation of all expressions in variable occurred before any portion of the variable is defined by the assignment. The evaluation of expressions within variable shall neither affect nor be affected by the evaluation of expr. Clearly, the above code violates this requirement because of the references to 'array' in 'Fred'. I think that we will have to provide an attribute that marks up array valued elemental functions that have any external array references and provide a temporary for assignment from one of these. Clearly something less brutal could be done, such as attaching a list of external arrays (to the elemental function, that is) to the symbol of the elemental function and comparing them with the lhs of an assignment. However, this works and has no perceivable effect on Polyhedron timings. I will change the name of the flags to potentially_aliasing. Bootstrapped and regtested on FC21/x86_64 - OK for trunk? Paul 2015-02-08 Paul Thomas pa...@gcc.gnu.org PR fortran/64952 * gfortran.h : Add 'potentially_aliased' field to symbol_attr. * trans.h : Add 'potentially_aliased' field to gfc_ss_info. * resolve.c (resolve_variable): Mark elemental function symbol as 'potentially_aliased' if it has an array reference from outside its own namespace. * trans-array.c (gfc_conv_resolve_dependencies): If any ss is marked as 'potentially_aliased' generate a temporary. (gfc_walk_function_expr): If the function is marked as 'potentially_aliased', likewise mark the head gfc_ss. 2015-02-08 Paul Thomas pa...@gcc.gnu.org PR fortran/64952 * gfortran.dg/finalize_28.f90: New test Index: gcc/fortran/trans-stmt.c === *** gcc/fortran/trans-stmt.c(revision 220481) --- gcc/fortran/trans-stmt.c(working copy) *** gfc_trans_deallocate (gfc_code *code) *** 5575,5585 if (expr-rank || gfc_is_coarray (expr)) { if (expr-ts.type == BT_DERIVED expr-ts.u.derived-attr.alloc_comp !gfc_is_finalizable (expr-ts.u.derived, NULL)) { - gfc_ref *ref; gfc_ref *last = NULL; for (ref = expr-ref; ref; ref = ref-next) if (ref-type == REF_COMPONENT) last = ref; --- 5575,5587 if (expr-rank || gfc_is_coarray (expr)) { + gfc_ref *ref; + if (expr-ts.type == BT_DERIVED expr-ts.u.derived-attr.alloc_comp !gfc_is_finalizable (expr-ts.u.derived, NULL)) { gfc_ref *last = NULL; + for (ref = expr-ref; ref; ref = ref-next) if (ref-type == REF_COMPONENT) last = ref; *** gfc_trans_deallocate (gfc_code *code) *** 5590,5602 !(!last expr-symtree-n.sym-attr.pointer)) { tmp = gfc_deallocate_alloc_comp (expr-ts.u.derived, se.expr, ! expr-rank); gfc_add_expr_to_block (se.pre, tmp); } } ! tmp = gfc_array_deallocate (se.expr, pstat, errmsg, errlen, ! label_finish, expr); ! gfc_add_expr_to_block (se.pre, tmp); if (al-expr-ts.type == BT_CLASS) gfc_reset_vptr (se.pre, al-expr); } --- 5592,5636 !(!last expr-symtree-n.sym-attr.pointer)) { tmp = gfc_deallocate_alloc_comp (expr-ts.u.derived, se.expr, ! expr-rank); gfc_add_expr_to_block (se.pre, tmp); } } ! ! if (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (se.expr))) ! { ! tmp = gfc_array_deallocate (se.expr, pstat, errmsg, errlen, ! label_finish, expr); ! gfc_add_expr_to_block (se.pre, tmp); ! } ! else if (TREE_CODE (se.expr)
Re: [Patch, fortran] PR64952 - Missing temporary in assignment from elemental function
Dear Mikael, Thank you very much for the review. You raise some points that I had thought about and others that I hadn't. I also realised that such things as blocks, within the elemental function would through the fix as well. I'll defer doing anything with it until tomorrow night. I reason that there is always going to be an 'ss', although I should check that it is not gfc_ss_terminator, and that it does not matter which one is flagged. I should add a comment to that effect; it's not quite as hackish as it looks, methinks. I will be back! Paul On 8 February 2015 at 18:27, Mikael Morin mikael.mo...@sfr.fr wrote: Hello Paul, comments below Le 08/02/2015 16:24, Paul Richard Thomas a écrit : Index: gcc/fortran/gfortran.h === *** gcc/fortran/gfortran.h(revision 220482) --- gcc/fortran/gfortran.h(working copy) *** typedef struct *** 789,794 --- 789,798 cannot alias. Note that this is zero for PURE procedures. */ unsigned implicit_pure:1; + /* This set for an elemental function that contains expressions for + arrays coming from outside its namespace. */ + unsigned potentially_aliased:1; + aliased is more something about pointers, so how about naming it something like array_outer_dependency? Anyway, that's minor. I wonder whether we should negate the meaning, that is set the flag if there is no external dependency. If we can get the conditions to set it exhaustively right, both are equivalent. Otherwise... maybe not. /* This is set if the subroutine doesn't return. Currently, this is only possible for intrinsic subroutines. */ unsigned noreturn:1; Index: gcc/fortran/trans.h === *** gcc/fortran/trans.h (revision 220481) --- gcc/fortran/trans.h (working copy) *** typedef struct gfc_ss_info *** 226,231 --- 226,235 /* Suppresses precalculation of scalars in WHERE assignments. */ unsigned where:1; + /* Signals that an array argument of an elemental function might be aliased, + thereby generating a temporary in assignments. */ + unsigned potentially_aliased:1; + /* Tells whether the SS is for an actual argument which can be a NULL reference. In other words, the associated dummy argument is OPTIONAL. Used to handle elemental procedures. */ Index: gcc/fortran/resolve.c === *** gcc/fortran/resolve.c (revision 220481) --- gcc/fortran/resolve.c (working copy) *** resolve_variable (gfc_expr *e) *** 5054,5059 --- 5054,5067 gfc_current_ns-parent-parent == sym-ns))) sym-attr.host_assoc = 1; + if (sym-attr.dimension +(sym-ns != gfc_current_ns + || sym-attr.use_assoc + || sym-attr.in_common) +gfc_elemental (NULL) +gfc_current_ns-proc_name-attr.function) + gfc_current_ns-proc_name-attr.potentially_aliased = 1; I would expect the flag to also be copied between procedures in some cases; namely if A calls B, and B has the flag, then A has the flag. There is also the case of external procedures (for which the flag is not known - assume the worst) + resolve_procedure: if (t !resolve_procedure_expression (e)) t = false; Index: gcc/fortran/trans-array.c === *** gcc/fortran/trans-array.c (revision 220482) --- gcc/fortran/trans-array.c (working copy) *** gfc_conv_resolve_dependencies (gfc_loopi *** 4391,4396 --- 4391,4402 { ss_expr = ss-info-expr; + if (ss-info-potentially_aliased) + { + nDepend = 1; + break; + } + if (ss-info-type != GFC_SS_SECTION) { if (flag_realloc_lhs *** gfc_walk_function_expr (gfc_ss * ss, gfc *** 9096,9104 /* Walk the parameters of an elemental function. For now we always pass by reference. */ if (sym-attr.elemental || (comp comp-attr.elemental)) ! return gfc_walk_elemental_function_args (ss, expr-value.function.actual, gfc_get_proc_ifc_for_expr (expr), GFC_SS_REFERENCE); /* Scalar functions are OK as these are evaluated outside the scalarization loop. Pass back and let the caller deal with it. */ --- 9102,9114 /* Walk the parameters of an elemental function. For now we always pass by reference. */ if (sym-attr.elemental || (comp comp-attr.elemental)) ! { ! ss = gfc_walk_elemental_function_args (ss, expr-value.function.actual, gfc_get_proc_ifc_for_expr (expr
Re: [Patch, fortran] PR64952 - Missing temporary in assignment from elemental function
Dear All, Dominique has just flagged up a slight technical problem with the patch... it's not for this PR :-( Please find the correct patch attached. Paul On 8 February 2015 at 12:42, Paul Richard Thomas paul.richard.tho...@gmail.com wrote: Dear All, This came up at https://groups.google.com/forum/#!topic/comp.lang.fortran/TvVY5j3GPmg gfortran produces wrong result from: PROGRAM Main INTEGER :: i, index(5) = (/ (i, i = 1,5) /) REAL :: array(5) = (/ (i+0.0, i = 1,5) /) array = Fred(index,array) PRINT *, array CONTAINS ELEMENTAL FUNCTION Fred (n, x) REAL :: Fred INTEGER, INTENT(IN) :: n REAL, INTENT(IN) :: x ! In general, this would be in an external procedure Fred = x+SUM(array(:n-1))+SUM(array(n+1:)) END FUNCTION Fred END PROGRAM Main outputs 15.000 29.000 56.000 109.00 214.00 when result should be 5*15.0 A temporary should be produced for array = Fred(index, array). See the clf thread for the reasoning. In a nutshell, the reason is: The execution of the assignment shall have the same effect as if the evaluation of expr and the evaluation of all expressions in variable occurred before any portion of the variable is defined by the assignment. The evaluation of expressions within variable shall neither affect nor be affected by the evaluation of expr. Clearly, the above code violates this requirement because of the references to 'array' in 'Fred'. I think that we will have to provide an attribute that marks up array valued elemental functions that have any external array references and provide a temporary for assignment from one of these. Clearly something less brutal could be done, such as attaching a list of external arrays (to the elemental function, that is) to the symbol of the elemental function and comparing them with the lhs of an assignment. However, this works and has no perceivable effect on Polyhedron timings. I will change the name of the flags to potentially_aliasing. Bootstrapped and regtested on FC21/x86_64 - OK for trunk? Paul 2015-02-08 Paul Thomas pa...@gcc.gnu.org PR fortran/64952 * gfortran.h : Add 'potentially_aliased' field to symbol_attr. * trans.h : Add 'potentially_aliased' field to gfc_ss_info. * resolve.c (resolve_variable): Mark elemental function symbol as 'potentially_aliased' if it has an array reference from outside its own namespace. * trans-array.c (gfc_conv_resolve_dependencies): If any ss is marked as 'potentially_aliased' generate a temporary. (gfc_walk_function_expr): If the function is marked as 'potentially_aliased', likewise mark the head gfc_ss. 2015-02-08 Paul Thomas pa...@gcc.gnu.org PR fortran/64952 * gfortran.dg/finalize_28.f90: New test -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx Index: gcc/fortran/gfortran.h === *** gcc/fortran/gfortran.h (revision 220482) --- gcc/fortran/gfortran.h (working copy) *** typedef struct *** 789,794 --- 789,798 cannot alias. Note that this is zero for PURE procedures. */ unsigned implicit_pure:1; + /* This set for an elemental function that contains expressions for + arrays coming from outside its namespace. */ + unsigned potentially_aliased:1; + /* This is set if the subroutine doesn't return. Currently, this is only possible for intrinsic subroutines. */ unsigned noreturn:1; Index: gcc/fortran/trans.h === *** gcc/fortran/trans.h (revision 220481) --- gcc/fortran/trans.h (working copy) *** typedef struct gfc_ss_info *** 226,231 --- 226,235 /* Suppresses precalculation of scalars in WHERE assignments. */ unsigned where:1; + /* Signals that an array argument of an elemental function might be aliased, + thereby generating a temporary in assignments. */ + unsigned potentially_aliased:1; + /* Tells whether the SS is for an actual argument which can be a NULL reference. In other words, the associated dummy argument is OPTIONAL. Used to handle elemental procedures. */ Index: gcc/fortran/resolve.c === *** gcc/fortran/resolve.c (revision 220481) --- gcc/fortran/resolve.c (working copy) *** resolve_variable (gfc_expr *e) *** 5054,5059 --- 5054,5067 gfc_current_ns-parent-parent == sym-ns))) sym-attr.host_assoc = 1; + if (sym-attr.dimension +(sym-ns != gfc_current_ns + || sym-attr.use_assoc + || sym-attr.in_common) +gfc_elemental (NULL) +gfc_current_ns-proc_name-attr.function) + gfc_current_ns
Re: [Patch, fortran] PR63744 accept duplicate use-rename
Dear Mikael, It looks good to me for trunk and the branches. Thanks for the patch Paul On 6 February 2015 at 21:31, Mikael Morin mikael.mo...@sfr.fr wrote: Hello, we currently reject programs of the form module m integer :: s end module m subroutine s use m, only: x = s, x = s end subroutine s with an error stating that S is the name of the current program unit. Interestingly, the duplicate rename is necessary to trigger it. There doesn't seem to be a consensus as to whether it should be accepted, but I think it should be. Quoting Dominique's comment in the PR: if use m, only: A = X use m, only: B = X is valid, I don't see why use m, only: A = X use m, only: A = X should not. The problem is we check the original (symbol) name instead of the local (symtree) name. The fix is close to obvious, and should be safe for the branches (this is a regression) Regression tested on x86_64-linux. OK for trunk/4.9/4.8 ? Mikael -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
[Patch, fortran] PR64932 [4.9/5 Regression] ICE in gfc_conv_descriptor_data_get for generated finalizer
Dear All, This is a slight development of the patch posted on the PR itself. class.c(finalize_component) is not able to deal correctly with non-allocatable, derived type array components that have allocatable components. Rather than generating loops in finalize_component, the condition is detected in trans-stmt.c(gfc_trans_deallocate) and gfc_deallocate_alloc_comp is called after obtaining the derived type for the array and checking that it is not finalizable. Happily, this fix does not generate the error: Error: Two or more part references with nonzero rank must not be specified at (1) which occurs if the code is written explicitly. Bootstraps and regtests on FC21/x86_64 OK for trunk and 4.9? Paul 2015-02-07 Paul Thomas pa...@gcc.gnu.org PR fortran/64932 * trans-stmt.c (gfc_trans_deallocate): If a component array expression is not a descriptor type and it is a derived type that has allocatable components and is not finalizable, then deallocate the allocatable components. 2015-02-07 Paul Thomas pa...@gcc.gnu.org PR fortran/6432 * gfortran.dg/finalize_28.f90: New test Index: gcc/fortran/trans-stmt.c === *** gcc/fortran/trans-stmt.c(revision 220481) --- gcc/fortran/trans-stmt.c(working copy) *** gfc_trans_deallocate (gfc_code *code) *** 5575,5585 if (expr-rank || gfc_is_coarray (expr)) { if (expr-ts.type == BT_DERIVED expr-ts.u.derived-attr.alloc_comp !gfc_is_finalizable (expr-ts.u.derived, NULL)) { - gfc_ref *ref; gfc_ref *last = NULL; for (ref = expr-ref; ref; ref = ref-next) if (ref-type == REF_COMPONENT) last = ref; --- 5575,5587 if (expr-rank || gfc_is_coarray (expr)) { + gfc_ref *ref; + if (expr-ts.type == BT_DERIVED expr-ts.u.derived-attr.alloc_comp !gfc_is_finalizable (expr-ts.u.derived, NULL)) { gfc_ref *last = NULL; + for (ref = expr-ref; ref; ref = ref-next) if (ref-type == REF_COMPONENT) last = ref; *** gfc_trans_deallocate (gfc_code *code) *** 5590,5602 !(!last expr-symtree-n.sym-attr.pointer)) { tmp = gfc_deallocate_alloc_comp (expr-ts.u.derived, se.expr, ! expr-rank); gfc_add_expr_to_block (se.pre, tmp); } } ! tmp = gfc_array_deallocate (se.expr, pstat, errmsg, errlen, ! label_finish, expr); ! gfc_add_expr_to_block (se.pre, tmp); if (al-expr-ts.type == BT_CLASS) gfc_reset_vptr (se.pre, al-expr); } --- 5592,5636 !(!last expr-symtree-n.sym-attr.pointer)) { tmp = gfc_deallocate_alloc_comp (expr-ts.u.derived, se.expr, ! expr-rank); gfc_add_expr_to_block (se.pre, tmp); } } ! ! if (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (se.expr))) ! { ! tmp = gfc_array_deallocate (se.expr, pstat, errmsg, errlen, ! label_finish, expr); ! gfc_add_expr_to_block (se.pre, tmp); ! } ! else if (TREE_CODE (se.expr) == COMPONENT_REF ! TREE_CODE (TREE_TYPE (se.expr)) == ARRAY_TYPE ! TREE_CODE (TREE_TYPE (TREE_TYPE (se.expr))) ! == RECORD_TYPE) ! { ! /* class.c(finalize_component) generates these, when a !finalizable entity has a non-allocatable derived type array !component, which has allocatable components. Obtain the !derived type of the array and deallocate the allocatable !components. */ ! for (ref = expr-ref; ref; ref = ref-next) ! { ! if (ref-u.c.component-attr.dimension ! ref-u.c.component-ts.type == BT_DERIVED) ! break; ! } ! ! if (ref ref-u.c.component-ts.u.derived-attr.alloc_comp ! !gfc_is_finalizable (ref-u.c.component-ts.u.derived, ! NULL)) ! { ! tmp = gfc_deallocate_alloc_comp ! (ref-u.c.component-ts.u.derived, !se.expr, expr-rank); ! gfc_add_expr_to_block (se.pre, tmp); ! } ! } ! if (al-expr-ts.type == BT_CLASS) gfc_reset_vptr (se.pre, al-expr); } Index: gcc/testsuite/gfortran.dg/finalize_28.f90 === ***
Re: [patch, libgfortran] [4.8/4.9/5 Regression] error reading (and writing) large text files in gfortran
Dear Jerry, This is OK for trunk. Maybe it is best to leave it for a week or two before committing to the branches? Thanks for the patch. Paul On 7 February 2015 at 03:08, Jerry DeLisle jvdeli...@charter.net wrote: With the attached patch I create a special version of fbuf_flush that is only called with list directed I/O. There can be no tabbing back and forth so it is safe to flush the buffer whenever we want. The bug occurs when the buffer keeps growing to no end until no more allocations can be made and the OS gives an error. fbuf_flush can be called as much as one wants to, but to keep overhead low, I introduce a new version with an arbitrary limit. If below the limit, just return doing no flushing. When the limit is exceeded, fbuf is flushed. The code to do this is duplicated from the original fbuf_flush so it is very safe and well tested. I played with the allowable maximum position limit for flushing and ran some timing tests. My machine here uses a solid state drive which may bias the results somewhat. Others are welcome to test and see what values they get as well. I settled on 524588 based on these results, favoring writing over reading a little. The patch has zero impact on any other type of I/O including normal formatted I/O. I also tested to confirm that formatted I/O does not have the problem. It is flushed regularly in the main transfer loop. As far as I can tell only list directed has the issue. I get the following timings using the attached test program. WRITING: Array Size-- 100 1 Buf Limit 16384 2.107 210.9 32768 2.026 292.1 65636 2.232 235.8 5242881.958 193.5 10485762.023 203.5 READING: Buf Limit 16384 1.843 184.4 32768 1.841 186.8 65636 1.816 197.6 5242881.879 186.5 10485761.834 185.2 Regression tested on x86-64 Linux. OK for trunk followed by backports? I can not include a specific testsuite test case, it would take way too long to run. Regards, Jerry 2015-02-07 Jerry DeLisle jvdeli...@gcc.gnu.org PR libgfortran/60956 * io/fbuf.c (fbuf_flush_list): New function that only flushes if current fbuf position exceeds a limit. * io/fbuf.h: Declare the new function. * io/io.h (enum unit_mode): Add two new modes. * io/list_read.c (list_formatted_read_scalar): Call new function. * io/write.c: Include fbuf.h. (list_formatted_write_scalar): Call new function. -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [Patch, Fortran] PR64943 Fix I/O diagnostic for structure constructors
Dear Tobias, That is OK for trunk. Thanks for the patch. Paul On 5 February 2015 at 22:04, Tobias Burnus bur...@net-b.de wrote: A rather simple patch. Build and reg-tested on x86-64-gnu-linux. OK for the trunk? Tobias -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [Patch, fortran] PR 64757 - [5 Regression] ICE in fold_convert_loc, at fold-const.c:2353
Dear All, Please find attached a reworked version of the patch for this PR. I have no idea at all, why the original version worked for array components on my laptop. In this version, the treatment of scalar and array components is cleanly separated. Bootstrapped and regtested on FC21/x86_64. OK for trunk? Paul 2015-02-04 Paul Thomas pa...@gcc.gnu.org PR fortran/640757 * resolve.c (resolve_structure_cons): Obtain the rank of class components. * trans-expr.c (gfc_trans_alloc_subarray_assign): Do the assignment to allocatable class array components. (alloc_scalar_allocatable_for_subcomponent_assignment): If comp is a class component, allocate to the _data field. (gfc_trans_subcomponent_assign): If a class component with a derived type expression set the _vptr field and for array components, call gfc_trans_alloc_subarray_assign. For scalars, the assignment is performed here. 2015-02-04 Paul Thomas pa...@gcc.gnu.org PR fortran/640757 * gfortran.dg/type_to_class_2.f90: New test * gfortran.dg/type_to_class_3.f90: New test On 3 February 2015 at 22:36, Paul Richard Thomas paul.richard.tho...@gmail.com wrote: Dear Dominique, I have fixed all the problems except the last one. For that case, the other brand gives type_to_class_30.f90(19): error #7822: Variables containing ultimate allocatable array components are forbidden from appearing directly in input/output lists. print *, TestReference([Test(99), Test(199)]) -^ compilation aborted for type_to_class_30.f90 (code 1) which seems to me to be correct. I'll see what I can do to fix it. Thanks for the help Paul On 2 February 2015 at 17:53, Dominique Dhumieres domi...@lps.ens.fr wrote: Dear Paul, I have tested your patch at https://gcc.gnu.org/ml/fortran/2015-01/txtwnaoa1115V.txt (the latest version) and I found that the test type_to_class_3.f03 is miscompiled (FAIL) with -flto -O0 -m64 (this does not happens with -flto -O0 -m32 or with -Ox and x!=0). In addition, while the reduced test type :: Test integer :: i end type type :: TestReference class(Test), allocatable :: test(:) end type type(TestReference) :: testList type(test), allocatable :: x(:) allocate (testList%test(2), source = [Test(99), Test(199)]) ! Works, of course print *, size(testList%test) x = testList%test print *, x end gives what I expect, i.e., 2 99 199 type :: Test integer :: i end type type :: TestReference class(Test), allocatable :: test(:) end type type(TestReference) :: testList type(test), allocatable :: x(:) testList = TestReference([Test(99), Test(199)]) ! Gave: The rank of the element in the ! structure constructor at (1) does not ! match that of the component (1/0) print *, size(testList%test) x = testList%test print *, x end gives 1 99 Last problem I see, print *, TestReference([Test(99), Test(199)]) gives the following ICE f951: internal compiler error: Bad IO basetype (7) type_to_class_3_red_2.f03:12:0: print *, TestReference([Test(99), Test(199)]) Cheers, Dominique -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx Index: gcc/fortran/resolve.c === *** gcc/fortran/resolve.c (revision 220305) --- gcc/fortran/resolve.c (working copy) *** resolve_structure_cons (gfc_expr *expr, *** 1155,1160 --- 1155,1163 } rank = comp-as ? comp-as-rank : 0; + if (comp-ts.type == BT_CLASS CLASS_DATA (comp)-as) + rank = CLASS_DATA (comp)-as-rank; + if (cons-expr-expr_type != EXPR_NULL rank != cons-expr-rank (comp-attr.allocatable || cons-expr-rank)) { Index: gcc/fortran/trans-expr.c === *** gcc/fortran/trans-expr.c(revision 220305) --- gcc/fortran/trans-expr.c(working copy) *** gfc_trans_alloc_subarray_assign (tree de *** 6211,6216 --- 6211,6230 tmp = gfc_copy_alloc_comp (cm-ts.u.derived, se.expr, dest, cm-as-rank); + else if (cm-ts.type == BT_CLASS expr-ts.type == BT_DERIVED + CLASS_DATA(cm)-attr.allocatable) + { + if (cm-ts.u.derived-attr.alloc_comp) + tmp = gfc_copy_alloc_comp (expr-ts.u.derived, + se.expr, dest, + expr-rank); + else + { + tmp = TREE_TYPE (dest); + tmp = gfc_duplicate_allocatable (dest, se.expr
Re: [Patch, fortran] PR 64757 - [5 Regression] ICE in fold_convert_loc, at fold-const.c:2353
Dear Dominique, I have fixed all the problems except the last one. For that case, the other brand gives type_to_class_30.f90(19): error #7822: Variables containing ultimate allocatable array components are forbidden from appearing directly in input/output lists. print *, TestReference([Test(99), Test(199)]) -^ compilation aborted for type_to_class_30.f90 (code 1) which seems to me to be correct. I'll see what I can do to fix it. Thanks for the help Paul On 2 February 2015 at 17:53, Dominique Dhumieres domi...@lps.ens.fr wrote: Dear Paul, I have tested your patch at https://gcc.gnu.org/ml/fortran/2015-01/txtwnaoa1115V.txt (the latest version) and I found that the test type_to_class_3.f03 is miscompiled (FAIL) with -flto -O0 -m64 (this does not happens with -flto -O0 -m32 or with -Ox and x!=0). In addition, while the reduced test type :: Test integer :: i end type type :: TestReference class(Test), allocatable :: test(:) end type type(TestReference) :: testList type(test), allocatable :: x(:) allocate (testList%test(2), source = [Test(99), Test(199)]) ! Works, of course print *, size(testList%test) x = testList%test print *, x end gives what I expect, i.e., 2 99 199 type :: Test integer :: i end type type :: TestReference class(Test), allocatable :: test(:) end type type(TestReference) :: testList type(test), allocatable :: x(:) testList = TestReference([Test(99), Test(199)]) ! Gave: The rank of the element in the ! structure constructor at (1) does not ! match that of the component (1/0) print *, size(testList%test) x = testList%test print *, x end gives 1 99 Last problem I see, print *, TestReference([Test(99), Test(199)]) gives the following ICE f951: internal compiler error: Bad IO basetype (7) type_to_class_3_red_2.f03:12:0: print *, TestReference([Test(99), Test(199)]) Cheers, Dominique -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [Patch, fortran] PR 64757 - [5 Regression] ICE in fold_convert_loc, at fold-const.c:2353
Dear Dominique, On transferring from my laptop to my workstation, I find that it segfaults in runtime - both are x86_64/FC21. If I can, I intend to investigate tonight. Thanks for the report. Paul On 2 February 2015 at 17:53, Dominique Dhumieres domi...@lps.ens.fr wrote: Dear Paul, I have tested your patch at https://gcc.gnu.org/ml/fortran/2015-01/txtwnaoa1115V.txt (the latest version) and I found that the test type_to_class_3.f03 is miscompiled (FAIL) with -flto -O0 -m64 (this does not happens with -flto -O0 -m32 or with -Ox and x!=0). In addition, while the reduced test type :: Test integer :: i end type type :: TestReference class(Test), allocatable :: test(:) end type type(TestReference) :: testList type(test), allocatable :: x(:) allocate (testList%test(2), source = [Test(99), Test(199)]) ! Works, of course print *, size(testList%test) x = testList%test print *, x end gives what I expect, i.e., 2 99 199 type :: Test integer :: i end type type :: TestReference class(Test), allocatable :: test(:) end type type(TestReference) :: testList type(test), allocatable :: x(:) testList = TestReference([Test(99), Test(199)]) ! Gave: The rank of the element in the ! structure constructor at (1) does not ! match that of the component (1/0) print *, size(testList%test) x = testList%test print *, x end gives 1 99 Last problem I see, print *, TestReference([Test(99), Test(199)]) gives the following ICE f951: internal compiler error: Bad IO basetype (7) type_to_class_3_red_2.f03:12:0: print *, TestReference([Test(99), Test(199)]) Cheers, Dominique -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [Patch, fortran] PR 64757 - [5 Regression] ICE in fold_convert_loc, at fold-const.c:2353
Dear All, I noticed last night that the component array version of Michael's testcase doesn't even get past resolution. The attached is an updated version of the patch that fixes that. Although the additional bits of the patch do not fix a regression, I think that it is worth having the extra functionality; especially since it is somewhat clearer than using allocate with a source expression. A ChangeLog will follow later on. Please note that I changed the name of the original testcase because it had class and type the wrong way round :-) Bootstraps and regtests on x86_64 - OK for trunk? Best regards Paul On 28 January 2015 at 21:09, Paul Richard Thomas paul.richard.tho...@gmail.com wrote: Dear All, This regression was caused by the patch for PR60357. The fix is straightforward. Please note however, that I have not checked for other fallout yet - I have merely addressed the reported failure. I will check around the reported testcase tomorrow night. Dominique, thanks for the rapid feedback. class_to_type_4.f90 is reserved for the patch for PR63205. Bootstrapped and regtested on x86_64/FC21 - OK for trunk? Michael, many thanks for a prompt report. Please come back to us with any more bugs that you find! Cheers Paul 2015-01-28 Paul Thomas pa...@gcc.gnu.org PR fortran/640757 * trans-expr.c (alloc_scalar_allocatable_for_subcomponent_assignment): If comp is a class component, get the data pointer. (gfc_trans_subcomponent_assign): If a class component with a derived type expression get the data pointer for the assignment and set the vptr. 2015-01-28 Paul Thomas pa...@gcc.gnu.org PR fortran/640757 * gfortran.dg/class_to_type_5.f90: New test -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx Index: gcc/fortran/resolve.c === *** gcc/fortran/resolve.c (revision 220083) --- gcc/fortran/resolve.c (working copy) *** resolve_structure_cons (gfc_expr *expr, *** 1155,1160 --- 1155,1163 } rank = comp-as ? comp-as-rank : 0; + if (comp-ts.type == BT_CLASS CLASS_DATA (comp)-as) + rank = CLASS_DATA (comp)-as-rank; + if (cons-expr-expr_type != EXPR_NULL rank != cons-expr-rank (comp-attr.allocatable || cons-expr-rank)) { Index: gcc/fortran/trans-expr.c === *** gcc/fortran/trans-expr.c(revision 220083) --- gcc/fortran/trans-expr.c(working copy) *** alloc_scalar_allocatable_for_subcomponen *** 6335,6340 --- 6335,6341 gfc_symbol *sym) { tree tmp; + tree ptr; tree size; tree size_in_bytes; tree lhs_cl_size = NULL_TREE; *** alloc_scalar_allocatable_for_subcomponen *** 6400,6407 tmp = build_call_expr_loc (input_location, builtin_decl_explicit (BUILT_IN_MALLOC), 1, size_in_bytes); ! tmp = fold_convert (TREE_TYPE (comp), tmp); ! gfc_add_modify (block, comp, tmp); } if (cm-ts.type == BT_CHARACTER cm-ts.deferred) --- 6401,6412 tmp = build_call_expr_loc (input_location, builtin_decl_explicit (BUILT_IN_MALLOC), 1, size_in_bytes); ! if (GFC_CLASS_TYPE_P (TREE_TYPE (comp))) ! ptr = gfc_class_data_get (comp); ! else ! ptr = comp; ! tmp = fold_convert (TREE_TYPE (ptr), tmp); ! gfc_add_modify (block, ptr, tmp); } if (cm-ts.type == BT_CHARACTER cm-ts.deferred) *** gfc_trans_subcomponent_assign (tree dest *** 6498,6510 /* The remainder of these instructions follow the if (cm-attr.pointer) if (!cm-attr.dimension) part above. */ gfc_init_se (se, NULL); ! gfc_conv_expr (se, expr); gfc_add_block_to_block (block, se.pre); if (expr-symtree expr-symtree-n.sym-attr.proc_pointer expr-symtree-n.sym-attr.dummy) se.expr = build_fold_indirect_ref_loc (input_location, se.expr); ! tmp = build_fold_indirect_ref_loc (input_location, dest); /* For deferred strings insert a memcpy. */ if (cm-ts.type == BT_CHARACTER cm-ts.deferred) { --- 6503,6533 /* The remainder of these instructions follow the if (cm-attr.pointer) if (!cm-attr.dimension) part above. */ gfc_init_se (se, NULL); ! if (expr-rank) ! gfc_conv_expr_descriptor (se, expr); ! else ! gfc_conv_expr (se, expr); gfc_add_block_to_block (block, se.pre); if (expr-symtree expr-symtree-n.sym-attr.proc_pointer expr-symtree-n.sym-attr.dummy) se.expr = build_fold_indirect_ref_loc (input_location, se.expr
Re: [Patch, fortran] PR63205 - [OOP] Wrongly rejects type = class (for identical declared type)
ping! On 27 January 2015 at 20:38, Paul Richard Thomas paul.richard.tho...@gmail.com wrote: Dear All, This patch enables the passing of an allocatable class object, scalar or array, to a derived type of the declared type, either in an assignment or as an actual argument. Much of the effort went into sorting out the finalization call so that the 'left over' allocatable components added by the dynamic type do not leak memory. At the moment, the existence of the finalization function is tested for. A check to see if the dynamic type is the same as the declared type could be added. Note that adding the 'must_finalize' field to gfc_expr will be useful in enabling the missing mandatory finalization calls. There are still interrogation marks about the patch; especially in build_class_array_ref, where I do not understand why the added code does not work in general, except for hidden function results. Nonetheless, the code does not leak memory, apart perhaps from the compound derived type constructors, with allocatable components that already show leaks elsewhere. It is also well ringfenced and so should not cause any regressions... touch wood! Bootstraps and regtests on x86_64/FC21 - OK for trunk? Paul 2015-01-27 Paul Thomas pa...@gcc.gnu.org PR fortran/63205 * gfortran.h: Add 'must finalize' field to gfc_expr and prototypes for gfc_is_alloc_class_scalar_function and for gfc_is_alloc_class_array_function. * expr.c (gfc_is_alloc_class_scalar_function, gfc_is_alloc_class_array_function): New functions. * trans-array.c (gfc_add_loop_ss_code): Do not move the expression for allocatable class scalar functions outside the loop. (conv_array_index_offset): Cope with deltas being NULL_TREE. (build_class_array_ref): Do not return with allocatable class array functions. Add code to pick out the returned class array. Dereference if necessary and return if not a class object. (gfc_conv_scalarized_array_ref): Cope with offsets being NULL. (gfc_walk_function_expr): Return an array ss for the result of an allocatable class array function. * trans-expr.c (gfc_conv_subref_array_arg): Remove the assert that the argument should be a variable. If an allocatable class array function, set the offset to zero and skip the write-out loop in this case. (gfc_conv_procedure_call): Add allocatable class array function to the assert. Call gfc_conv_subref_array_arg for allocatable class array function arguments with derived type formal arg.. Add the code for handling allocatable class functions, including finalization calls to prevent memory leaks. (arrayfunc_assign_needs_temporary): Return if an allocatable class array function. (gfc_trans_assignment_1): Set must_finalize to rhs expression for allocatable class functions. Set scalar_to_array as needed for scalar class allocatable functions assigned to an array. Nullify the allocatable components corresponding the the lhs derived type so that the finalization does not free them. 2015-01-27 Paul Thomas pa...@gcc.gnu.org PR fortran/63205 * gfortran.dg/class_to_type_4.f90: New test -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
[Patch, fortran] PR 64757 - [5 Regression] ICE in fold_convert_loc, at fold-const.c:2353
Dear All, This regression was caused by the patch for PR60357. The fix is straightforward. Please note however, that I have not checked for other fallout yet - I have merely addressed the reported failure. I will check around the reported testcase tomorrow night. Dominique, thanks for the rapid feedback. class_to_type_4.f90 is reserved for the patch for PR63205. Bootstrapped and regtested on x86_64/FC21 - OK for trunk? Michael, many thanks for a prompt report. Please come back to us with any more bugs that you find! Cheers Paul 2015-01-28 Paul Thomas pa...@gcc.gnu.org PR fortran/640757 * trans-expr.c (alloc_scalar_allocatable_for_subcomponent_assignment): If comp is a class component, get the data pointer. (gfc_trans_subcomponent_assign): If a class component with a derived type expression get the data pointer for the assignment and set the vptr. 2015-01-28 Paul Thomas pa...@gcc.gnu.org PR fortran/640757 * gfortran.dg/class_to_type_5.f90: New test Index: gcc/fortran/trans-expr.c === --- gcc/fortran/trans-expr.c(revision 220083) +++ gcc/fortran/trans-expr.c(working copy) @@ -6335,6 +6335,7 @@ gfc_symbol *sym) { tree tmp; + tree ptr; tree size; tree size_in_bytes; tree lhs_cl_size = NULL_TREE; @@ -6400,8 +6401,12 @@ tmp = build_call_expr_loc (input_location, builtin_decl_explicit (BUILT_IN_MALLOC), 1, size_in_bytes); - tmp = fold_convert (TREE_TYPE (comp), tmp); - gfc_add_modify (block, comp, tmp); + if (GFC_CLASS_TYPE_P (TREE_TYPE (comp))) + ptr = gfc_class_data_get (comp); + else + ptr = comp; + tmp = fold_convert (TREE_TYPE (ptr), tmp); + gfc_add_modify (block, ptr, tmp); } if (cm-ts.type == BT_CHARACTER cm-ts.deferred) @@ -6504,7 +6509,21 @@ if (expr-symtree expr-symtree-n.sym-attr.proc_pointer expr-symtree-n.sym-attr.dummy) se.expr = build_fold_indirect_ref_loc (input_location, se.expr); - tmp = build_fold_indirect_ref_loc (input_location, dest); + + if (GFC_CLASS_TYPE_P (TREE_TYPE (dest)) expr-ts.type == BT_DERIVED) + { + tree vtab; + tmp = gfc_class_data_get (dest); + tmp = build_fold_indirect_ref_loc (input_location, tmp); + vtab = gfc_get_symbol_decl (gfc_find_vtab (expr-ts)); + vtab = gfc_build_addr_expr (NULL_TREE, vtab); + gfc_add_modify (block, gfc_class_vptr_get (dest), +fold_convert (TREE_TYPE (gfc_class_vptr_get (dest)), vtab)); + } + else + tmp = build_fold_indirect_ref_loc (input_location, dest); + + /* For deferred strings insert a memcpy. */ if (cm-ts.type == BT_CHARACTER cm-ts.deferred) { Index: gcc/testsuite/gfortran.dg/class_to_type_5.f03 === --- gcc/testsuite/gfortran.dg/class_to_type_5.f03 (revision 0) +++ gcc/testsuite/gfortran.dg/class_to_type_5.f03 (working copy) @@ -0,0 +1,30 @@ +! { dg-do run } +! +! Test the fix for PR64757. +! +! Contributed by Michael Lee Rilee m...@rilee.net +! + type :: Test +integer :: i + end type + + type :: TestReference + class(Test), allocatable :: test + end type + + type(TestReference) :: testList + type(test) :: x + + testList = TestReference(Test(99)) ! ICE in fold_convert_loc was here + + x = testList%test + + select type (y = testList%test)! Check vptr set +type is (Test) + if (x%i .ne. y%i) call abort +class default + call abort + end select +end + +
Bug 62044 - [4.8/4.9 Regression] ICE in USE statement with RENAME for extended derived type
Dear All, The highly embarrassing bug in mold = allocations to class entities has been fixed in revisions 220140 and 220191 for trunk and 4.9 respectively. The PR has been set as RESOLVED. Cheers Paul
[Patch, fortran] PR63205 - [OOP] Wrongly rejects type = class (for identical declared type)
Dear All, This patch enables the passing of an allocatable class object, scalar or array, to a derived type of the declared type, either in an assignment or as an actual argument. Much of the effort went into sorting out the finalization call so that the 'left over' allocatable components added by the dynamic type do not leak memory. At the moment, the existence of the finalization function is tested for. A check to see if the dynamic type is the same as the declared type could be added. Note that adding the 'must_finalize' field to gfc_expr will be useful in enabling the missing mandatory finalization calls. There are still interrogation marks about the patch; especially in build_class_array_ref, where I do not understand why the added code does not work in general, except for hidden function results. Nonetheless, the code does not leak memory, apart perhaps from the compound derived type constructors, with allocatable components that already show leaks elsewhere. It is also well ringfenced and so should not cause any regressions... touch wood! Bootstraps and regtests on x86_64/FC21 - OK for trunk? Paul 2015-01-27 Paul Thomas pa...@gcc.gnu.org PR fortran/63205 * gfortran.h: Add 'must finalize' field to gfc_expr and prototypes for gfc_is_alloc_class_scalar_function and for gfc_is_alloc_class_array_function. * expr.c (gfc_is_alloc_class_scalar_function, gfc_is_alloc_class_array_function): New functions. * trans-array.c (gfc_add_loop_ss_code): Do not move the expression for allocatable class scalar functions outside the loop. (conv_array_index_offset): Cope with deltas being NULL_TREE. (build_class_array_ref): Do not return with allocatable class array functions. Add code to pick out the returned class array. Dereference if necessary and return if not a class object. (gfc_conv_scalarized_array_ref): Cope with offsets being NULL. (gfc_walk_function_expr): Return an array ss for the result of an allocatable class array function. * trans-expr.c (gfc_conv_subref_array_arg): Remove the assert that the argument should be a variable. If an allocatable class array function, set the offset to zero and skip the write-out loop in this case. (gfc_conv_procedure_call): Add allocatable class array function to the assert. Call gfc_conv_subref_array_arg for allocatable class array function arguments with derived type formal arg.. Add the code for handling allocatable class functions, including finalization calls to prevent memory leaks. (arrayfunc_assign_needs_temporary): Return if an allocatable class array function. (gfc_trans_assignment_1): Set must_finalize to rhs expression for allocatable class functions. Set scalar_to_array as needed for scalar class allocatable functions assigned to an array. Nullify the allocatable components corresponding the the lhs derived type so that the finalization does not free them. 2015-01-27 Paul Thomas pa...@gcc.gnu.org PR fortran/63205 * gfortran.dg/class_to_type_4.f90: New test Index: gcc/fortran/gfortran.h === *** gcc/fortran/gfortran.h (revision 208092) --- gcc/fortran/gfortran.h (working copy) *** typedef struct gfc_expr *** 1753,1758 --- 1753,1761 /* Mark an expression as being a MOLD argument of ALLOCATE. */ unsigned int mold : 1; + /* Will require finalization after use. */ + unsigned int must_finalize : 1; + /* If an expression comes from a Hollerith constant or compile-time evaluation of a transfer statement, it may have a prescribed target- memory representation, and these cannot always be backformed from *** bool gfc_expr_check_typed (gfc_expr*, gf *** 2804,2809 --- 2807,2814 gfc_component * gfc_get_proc_ptr_comp (gfc_expr *); bool gfc_is_proc_ptr_comp (gfc_expr *); + bool gfc_is_alloc_class_scalar_function (gfc_expr *); + bool gfc_is_alloc_class_array_function (gfc_expr *); bool gfc_ref_this_image (gfc_ref *ref); bool gfc_is_coindexed (gfc_expr *); Index: gcc/fortran/expr.c === *** gcc/fortran/expr.c (revision 208092) --- gcc/fortran/expr.c (working copy) *** gfc_is_proc_ptr_comp (gfc_expr *expr) *** 4274,4279 --- 4274,4313 } + /* Determine if an expression is a function with an allocatable class scalar +result. */ + bool + gfc_is_alloc_class_scalar_function (gfc_expr *expr) + { + if (expr-expr_type == EXPR_FUNCTION +expr-value.function.esym +expr-value.function.esym-result +expr-value.function.esym-result-ts.type == BT_CLASS +!CLASS_DATA (expr-value.function.esym-result)-attr.dimension +CLASS_DATA (expr-value.function.esym-result)-attr.allocatable) + return true; + + return false; + } + + + /* Determine if an expression is a
Re: [Patch, Fortran, OOP] PR 64230: [4.9/5 Regression] Invalid memory reference in a compiler-generated finalizer for allocatable component
Hi Janus, The testcase has a dependence on libubsan.so, which my LD_LIBRARY_PATH does not seem to be able to resolve. It therefore fails in the regression test. When I point to ~/lib64, I get the message: /svn/trunk/gcc/testsuite/gfortran.dg/class_allocate_18.f90:8: runtime error: signed integer overflow: 214358001592 * 214358001560 cannot be represented in type 'integer(kind=8)' Cheers Paul On 26 January 2015 at 15:57, Janus Weil ja...@gcc.gnu.org wrote: 2015-01-24 18:18 GMT+01:00 Tobias Burnus bur...@net-b.de: this is a second patch dealing with finalization-related regressions, [...] This patch fixes an invalid memory reference inside the finalizer routine (at runtime), which apparently was caused by dereferencing a pointer without checking if it's NULL. I simply insert a call to ASSOCIATED. [...] This one regtests cleanly on x86_64-unknown-linux-gnu. Ok for trunk and 4.9? OK. Thanks for the patch! Thanks for the review, Tobias. Committed to trunk as r220125. Will backport to 4.9 soon. Cheers, Janus -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [Patch, fortran] PR62044 ICE when loading use-renamed extended derived type
Dear All, The fix for the failing testcases in comments #6 and #7 in this PR have been fixed as 'obvious' in r220140. Thanks to Mikael for reopening the PR as a placeholder. This is such an embarrassing bug that I intend to commit to 4.9 as well even if it is not a regression, unless there are objections. Notice that in 4.9, the testcase will change to that of comment #7. Cheers Paul 2015-01-26 Paul Thomas pa...@gcc.gnu.org PR fortran/62044 * resolve.c (resolve_allocate_expr): If the default initializer is NULL, keep the original MOLD expression so that the correct typespec is available. 2015-01-26 Paul Thomas pa...@gcc.gnu.org PR fortran/62044 * gfortran.dg/allocate_with_mold_1.f90: New test On 25 January 2015 at 14:49, Mikael Morin mikael.mo...@sfr.fr wrote: Hello, I have had a look at PR62044 where the compiler ICEs when loading the extensions of a derived type from a module file, because one use-renamed extended type is not found under its original name. After scratching my head on the tree of extended types (rooted at derived-f2k_derived-sym_root), wondering whether the renamed or original name should be used, I decided to remove all that nonsense. So here is the patch, regression tested on x86_64-unknown-linux-gnu. There is a failure, graphite/pr42393.f90, also present on mainline. OK for trunk/4.9/4.8 ? Mikael -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [Patch, Fortran, OOP] PR 64230: [4.9/5 Regression] Invalid memory reference in a compiler-generated finalizer for allocatable component
Dear Janus, I know somewhat less than nothing about such matters. I suggest that you contact the authors of the testcases that use libubsan that you mention. Sorry Paul On 26 January 2015 at 22:20, Janus Weil ja...@gcc.gnu.org wrote: Hi Paul, The testcase has a dependence on libubsan.so, which my LD_LIBRARY_PATH does not seem to be able to resolve. It therefore fails in the regression test. When I point to ~/lib64, I get the message: /svn/trunk/gcc/testsuite/gfortran.dg/class_allocate_18.f90:8: runtime error: signed integer overflow: 214358001592 * 214358001560 cannot be represented in type 'integer(kind=8)' of course I tested it but did not see any problem on my machine (Ubuntu 14.10, x86_64-unknown-linux-gnu). In fact class_allocate_18.f90 seems to be the only testcase in gfortran.dg/ which uses any -fsanitize options, but there are plenty in other sections (mostly c-c++-common/ubsan/ and g++.dg/ubsan/). Could it be a problem with your local setup or a dejagnu issue that only applears on certain platforms? I have no idea how to fix this. Do you have any suggestion? Cheers, Janus On 26 January 2015 at 15:57, Janus Weil ja...@gcc.gnu.org wrote: 2015-01-24 18:18 GMT+01:00 Tobias Burnus bur...@net-b.de: this is a second patch dealing with finalization-related regressions, [...] This patch fixes an invalid memory reference inside the finalizer routine (at runtime), which apparently was caused by dereferencing a pointer without checking if it's NULL. I simply insert a call to ASSOCIATED. [...] This one regtests cleanly on x86_64-unknown-linux-gnu. Ok for trunk and 4.9? OK. Thanks for the patch! Thanks for the review, Tobias. Committed to trunk as r220125. Will backport to 4.9 soon. Cheers, Janus -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [Patch, Fortran, OOP] PR 60922: [4.9/5 regression] Memory leak with allocatable CLASS components
Dear Janus, As it happens I loaded this patch up last night to see if I could plug the last leak in my patch for PR63205, which is making heavy use of the default finalizers. It seemed to do the job. As you said in the PR it appears to be a typo and really is obvious. I can canfirm that it bootstraps and regtests OK. Good for 4.9 and 5.0 Thanks for the patch Paul On 22 January 2015 at 16:37, Janus Weil ja...@gcc.gnu.org wrote: Ping! 2015-01-18 11:46 GMT+01:00 Janus Weil ja...@gcc.gnu.org: Hi all, the attached patch is close to trivial and fixes a memory-leak regression that appeared after the implementation of finalization. My suspicion it that it's simply a copy'n'paste error, where the wrong attribute was copied from very similar code a few lines above, but I'd like to have that confirmed by the original author (which should be Tobias). Regtested on x86_64-unknown-linux-gnu. Ok for trunk and 4.9? Cheers, Janus 2015-01-18 Janus Weil ja...@gcc.gnu.org PR fortran/60922 * class.c (finalize_component): Apply the check for 'fini_coarray' only to coarray components. 2015-01-18 Janus Weil ja...@gcc.gnu.org PR fortran/60922 * gfortran.dg/class_allocate_17.f90: Extended. -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
[Patch, fortran] PR57959 - [F03] ICE with structure constructor with scalar allocatable components
Dear All, Andre's patch for PR60357 did not fix this PR as we had hoped. The fix needed is trivial, nay 'obvious'. The problem was that a deep copy was not being performed; just an assignement of the pointers to the data. In consequence the testcase was being clobbered with a double free in the 'finally' block generated by the fortran block. Note that the PR title shows this as an F03 bug. However, the example crashed because of the block construct, which is F2008. For this reason, the testcase is f08. Boostrapped and regtested on x86_64/FC21 - I will commit to trunk as 'obvious' Cheers Paul 2015-01-18 Paul Thomas pa...@gcc.gnu.org PR fortran/64578 * trans-expr.c (gfc_trans_subcomponent_assign): Use a deep copy for allocatable components, where the source is a variable. 2015-01-18 Paul Thomas pa...@gcc.gnu.org PR fortran/64578 * gfortran.dg/block_13.f08: New test -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx Index: gcc/fortran/trans-expr.c === *** gcc/fortran/trans-expr.c(revision 219801) --- gcc/fortran/trans-expr.c(working copy) *** gfc_trans_pointer_assignment (gfc_expr * *** 7075,7080 --- 7075,7081 rse.expr = gfc_class_data_get (rse.expr); else { + gfc_add_block_to_block (block, rse.pre); tmp = gfc_create_var (TREE_TYPE (rse.expr), ptrtemp); gfc_add_modify (lse.pre, tmp, rse.expr); *** gfc_trans_pointer_assignment (gfc_expr * *** 7146,7151 --- 7147,7153 } else { + gfc_add_block_to_block (block, rse.pre); tmp = gfc_create_var (TREE_TYPE (rse.expr), ptrtemp); gfc_add_modify (lse.pre, tmp, rse.expr); Index: gcc/fortran/trans-intrinsic.c === *** gcc/fortran/trans-intrinsic.c (revision 219800) --- gcc/fortran/trans-intrinsic.c (working copy) *** gfc_conv_associated (gfc_se *se, gfc_exp *** 6554,6560 --- 6554,6564 arg1se.expr = build_fold_indirect_ref_loc (input_location, arg1se.expr); if (arg1-expr-ts.type == BT_CLASS) + { tmp2 = gfc_class_data_get (arg1se.expr); + if (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (tmp2))) + tmp2 = gfc_conv_descriptor_data_get (tmp2); + } else tmp2 = arg1se.expr; } Index: gcc/testsuite/gfortran.dg/unlimited_polymorphic_21.f90 === *** gcc/testsuite/gfortran.dg/unlimited_polymorphic_21.f90 (revision 0) --- gcc/testsuite/gfortran.dg/unlimited_polymorphic_21.f90 (working copy) *** *** 0 --- 1,35 + ! { dg-do run } + ! Tests the fix for PR64578. + ! + ! Contributed by Damian Rouson dam...@sourceryinstitute.org + ! + type foo + real, allocatable :: component(:) + end type + type (foo), target :: f + class(*), pointer :: ptr(:) + allocate(f%component(1),source=[0.99]) + call associate_pointer(f,ptr) + select type (ptr) + type is (real) + if (abs (ptr(1) - 0.99) 1e-5) call abort + end select + ptr = return_pointer(f) ! runtime segmentation fault + if (associated(return_pointer(f)) .neqv. .true.) call abort + select type (ptr) + type is (real) + if (abs (ptr(1) - 0.99) 1e-5) call abort + end select + contains + subroutine associate_pointer(this, item) + class(foo), target :: this + class(*), pointer :: item(:) + item = this%component + end subroutine + function return_pointer(this) + class(foo), target :: this + class(*), pointer :: return_pointer(:) + return_pointer = this%component + end function + end +
[Patch, fortran] PR55901 - [OOP] type is (character(len=*)) misinterpreted as array - part 1
Dear All, The attached patch fixes the confusion between substrings of character associate-names and scalars being misused as arrays. It is sufficiently obvious and has been tested by Dominique that I will commit it later today if there are no opinions to the contrary. I will now turn my attention to Andre's patches for PR60357 and PR61275. Afterwards, I will fix the rest of the fix for this PR, via Andre's patch for PR60255 and the query in the final message in the thread. Bootstrapped and regtested on x86_64/FC21 - OK for trunk? Cheers Paul 2015-01-17 Paul Thomas pa...@gcc.gnu.org PR fortran/55901 * primary.c (gfc_match_varspec): Exclude dangling associate- names with dimension 0 from being counted as arrays. * resolve.c (resolve_assoc_var): Sub-strings are permissible for associate-names, so exclude characters from the test for misuse as arrays. * trans-decl.c (gfc_get_symbol_decl): Associate-names can use the hidden string length variable of their associated target. Signal this by setting 'length' to a constant, if the decl for the string length is a variable. 2015-01-17 Paul Thomas pa...@gcc.gnu.org PR fortran/55901 * gfortran.dg/associate_1.f03: Allow test for character with automatic length. -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [PATCH, fortran] PR fortran/60255 Deferred character length
Dear Andre, Perhaps, rather than calling the new component _len, we should call it _mem_size or some such? Cheers Paul On 9 January 2015 at 11:52, Andre Vehreschild ve...@gmx.de wrote: Hi all, hi Paul, I started to implement the changes requested below, but I stumbled over an oddity: For a deferred length kind4 char array, the length of the string is stored without multiplication by 4 in the length variable attached. So when we now decide to store the length of the string in an unlimited polymorphic entity in bytes in the component formerly called _len and the size of each character in _vtype-_size then we have an inconsistency with the style deferred char lengths are stored. IMHO we should store this consistently, i.e., both 'length'-variables store either the length of the string ('length' = array_len) or the size of the memory needed ('length' = array_len * char_size). What do you think? Furthermore, think about debugging: When looking at an unlimited polymorphic entity storing a kind-4-char-array of length 7, then having a 'length' component set to 28 will lead to confusion. I humbly predict, that this will produce many entries in the bugtracker, because people don't understand that 'length' stores the product of elem_size times string_len, because all they see is an assignment of a length-7 char array. What do we do about it? Regards, Andre On Thu, 8 Jan 2015 20:56:43 +0100 Paul Richard Thomas paul.richard.tho...@gmail.com wrote: Dear Andre, Thanks for the patch. As I have said to you, off list, I think that the _size field in the vtable should contain the kind information and that the _len field should carry the length of the string in bytes. I think that it is better to optimise array access this way than to avoid the division in evaluating LEN (). I am happy to accept contrary opinions from the others. I do not believe that the bind_c issue is an issue. Your patch correctly deals with it IMHO. Subject to the above change in the value of _len, I think that your patch is OK for trunk. With best regards Paul On 4 January 2015 at 13:40, Andre Vehreschild ve...@gmx.de wrote: Hi Janus, hi Paul, hi Tobias, Janus: During code review, I found that I had the code in gfc_get_len_component() duplicated. So I now reintroduced and documented the routine making is more commonly usable and added more documentation. The call sites are now simplify.c (gfc_simplify_len) and trans-expr.c (gfc_trans_pointer_assignment). Attached is the reworked version of the patch. Paul, Tobias: Can one of you have a look at line 253 of the patch? I need some expertise on the bind_c behavior. My patch needs the check for is_bind_c added in trans_expr.c (gfc_conv_expr) to prevent mistyping an associated variable in a select type() during the conv. Background: This code fragment taken from the testcase in the patch: MODULE m contains subroutine bar (arg, res) class(*) :: arg character(100) :: res select type (w = arg) type is (character(*)) write (res, '(I2)') len(w) end select end subroutine END MODULE has the conditions required for line trans-expr.c:6630 of gfc_conv_expr when the associate variable w is converted. This transforms the type of the associate variable to something unexpected in the further processing leading to some issues during fortraning. Janus told me, that the f90_type has been abused for some other things (unlimited polymorphic treatment). Although I believe that reading the comments above the if in question, the check I had to enhance is treating bind_c stuff (see the threads content for more). I would feel safer when one of you gfortran gurus can have a look and given an opinion, whether the change is problematic. I couldn't figure why w is resolved to meet the criteria (any ideas). Btw, all regtest are ok reporting no issues at all. Bootstraps and regtests ok on x86_64-linux-gnu Regards, Andre On Sat, 3 Jan 2015 16:45:07 +0100 Janus Weil ja...@gcc.gnu.org wrote: Hi Andre, For the second one (in gfc_conv_expr), I don't directly see how it's related to deferred char-len. Why is this change needed? That change is needed, because in some rare case where an associated variable in a select type () is used, then the type and f90_type match the condition while them not really being in a bind_c context. Therefore I have added the check for bind_c. Btw, I now have removed the TODO, because that case is covered by the regression tests. I don't understand how f90_type can be BT_VOID without being in a BIND_C context, but I'm not really a ISO_C_BINDING expert. Which test case is the one that triggered this? This case is triggered by the test-case in the patch, where in the select type (w = arg) in module m routine bar the w meets the criteria to make
Re: [PATCH, fortran] PR fortran/60255 Deferred character length
Dear Andre, I am open to either - what do the others think? The reason why I am for differentiating between unlimited_polymorphic objects and the deferred length character arrays is because of the difference in the way in which arrays are accessed. The former uses pointer arithmetic and the latter array references. I was trying to avoid divisions by KIND within scalarization loops. Also, I found that in developing your patch, that allocating with unlimited polymorphic sources looks neatest when the _len contains the memory size of the elements of any dynamic type, since a priori it is not known at compile time whether it is a character or not. Of course, one could interrogate the _hash field of the vtable, at the expense of more runtime code. Cheers Paul PS I have your patches for PR60357 and 61275 regtesting right now. Both look OK to me. At the risk of making potential regressions more complicated to unravel, to save my time I intend to commit both at once, unless anybody objects. On 17 January 2015 at 13:10, Andre Vehreschild ve...@gmx.de wrote: Hi Paul, I am open on what to call the new component. Have you thought about my findings, that for deferred length char arrays the length is stored in characters and not in bytes, I.e., for a character(kind=4, Len=:) the length is stored in number of characters and not in bytes needed, which would be Len*4. IMHO both concepts should be changed, or none. I favor to keep storing the string length of both concepts (deferred char arrays and chararrays in unlimited polymorphic entities) interchangeable w/o computation. What's your opinion? Regards, Andre Andre Vehreschild * Kreuzherrenstr. 8 * 52062 Aachen Tel. +49 241.9291018 * ve...@gmx.de Paul Richard Thomas paul.richard.tho...@gmail.com schrieb: Dear Andre, Perhaps, rather than calling the new component _len, we should call it _mem_size or some such? Cheers Paul On 9 January 2015 at 11:52, Andre Vehreschild ve...@gmx.de wrote: Hi all, hi Paul, I started to implement the changes requested below, but I stumbled over an oddity: For a deferred length kind4 char array, the length of the string is stored without multiplication by 4 in the length variable attached. So when we now decide to store the length of the string in an unlimited polymorphic entity in bytes in the component formerly called _len and the size of each character in _vtype-_size then we have an inconsistency with the style deferred char lengths are stored. IMHO we should store this consistently, i.e., both 'length'-variables store either the length of the string ('length' = array_len) or the size of the memory needed ('length' = array_len * char_size). What do you think? Furthermore, think about debugging: When looking at an unlimited polymorphic entity storing a kind-4-char-array of length 7, then having a 'length' component set to 28 will lead to confusion. I humbly predict, that this will produce many entries in the bugtracker, because people don't understand that 'length' stores the product of elem_size times string_len, because all they see is an assignment of a length-7 char array. What do we do about it? Regards, Andre On Thu, 8 Jan 2015 20:56:43 +0100 Paul Richard Thomas paul.richard.tho...@gmail.com wrote: Dear Andre, Thanks for the patch. As I have said to you, off list, I think that the _size field in the vtable should contain the kind information and that the _len field should carry the length of the string in bytes. I think that it is better to optimise array access this way than to avoid the division in evaluating LEN (). I am happy to accept contrary opinions from the others. I do not believe that the bind_c issue is an issue. Your patch correctly deals with it IMHO. Subject to the above change in the value of _len, I think that your patch is OK for trunk. With best regards Paul On 4 January 2015 at 13:40, Andre Vehreschild ve...@gmx.de wrote: Hi Janus, hi Paul, hi Tobias, Janus: During code review, I found that I had the code in gfc_get_len_component() duplicated. So I now reintroduced and documented the routine making is more commonly usable and added more documentation. The call sites are now simplify.c (gfc_simplify_len) and trans-expr.c (gfc_trans_pointer_assignment). Attached is the reworked version of the patch. Paul, Tobias: Can one of you have a look at line 253 of the patch? I need some expertise on the bind_c behavior. My patch needs the check for is_bind_c added in trans_expr.c (gfc_conv_expr) to prevent mistyping an associated variable in a select type() during the conv. Background: This code fragment taken from the testcase in the patch: MODULE m contains subroutine bar (arg, res) class(*) :: arg character(100) :: res select type (w = arg) type is (character(*)) write (res, '(I2)') len(w) end select end subroutine END MODULE
Re: [PATCH, fortran] PR fortran/60255 Deferred character length
Cancel that - It should be multiplies by kind, shouldn't it ? :-) OK, string length it is. We will probably have to set _len = 1 for other dynamic types, though, so that the pointer arising from an array reference is base_address + _len*vptr-size*index Cheers Paul On 17 January 2015 at 13:44, Paul Richard Thomas paul.richard.tho...@gmail.com wrote: Dear Andre, I am open to either - what do the others think? The reason why I am for differentiating between unlimited_polymorphic objects and the deferred length character arrays is because of the difference in the way in which arrays are accessed. The former uses pointer arithmetic and the latter array references. I was trying to avoid divisions by KIND within scalarization loops. Also, I found that in developing your patch, that allocating with unlimited polymorphic sources looks neatest when the _len contains the memory size of the elements of any dynamic type, since a priori it is not known at compile time whether it is a character or not. Of course, one could interrogate the _hash field of the vtable, at the expense of more runtime code. Cheers Paul PS I have your patches for PR60357 and 61275 regtesting right now. Both look OK to me. At the risk of making potential regressions more complicated to unravel, to save my time I intend to commit both at once, unless anybody objects. On 17 January 2015 at 13:10, Andre Vehreschild ve...@gmx.de wrote: Hi Paul, I am open on what to call the new component. Have you thought about my findings, that for deferred length char arrays the length is stored in characters and not in bytes, I.e., for a character(kind=4, Len=:) the length is stored in number of characters and not in bytes needed, which would be Len*4. IMHO both concepts should be changed, or none. I favor to keep storing the string length of both concepts (deferred char arrays and chararrays in unlimited polymorphic entities) interchangeable w/o computation. What's your opinion? Regards, Andre Andre Vehreschild * Kreuzherrenstr. 8 * 52062 Aachen Tel. +49 241.9291018 * ve...@gmx.de Paul Richard Thomas paul.richard.tho...@gmail.com schrieb: Dear Andre, Perhaps, rather than calling the new component _len, we should call it _mem_size or some such? Cheers Paul On 9 January 2015 at 11:52, Andre Vehreschild ve...@gmx.de wrote: Hi all, hi Paul, I started to implement the changes requested below, but I stumbled over an oddity: For a deferred length kind4 char array, the length of the string is stored without multiplication by 4 in the length variable attached. So when we now decide to store the length of the string in an unlimited polymorphic entity in bytes in the component formerly called _len and the size of each character in _vtype-_size then we have an inconsistency with the style deferred char lengths are stored. IMHO we should store this consistently, i.e., both 'length'-variables store either the length of the string ('length' = array_len) or the size of the memory needed ('length' = array_len * char_size). What do you think? Furthermore, think about debugging: When looking at an unlimited polymorphic entity storing a kind-4-char-array of length 7, then having a 'length' component set to 28 will lead to confusion. I humbly predict, that this will produce many entries in the bugtracker, because people don't understand that 'length' stores the product of elem_size times string_len, because all they see is an assignment of a length-7 char array. What do we do about it? Regards, Andre On Thu, 8 Jan 2015 20:56:43 +0100 Paul Richard Thomas paul.richard.tho...@gmail.com wrote: Dear Andre, Thanks for the patch. As I have said to you, off list, I think that the _size field in the vtable should contain the kind information and that the _len field should carry the length of the string in bytes. I think that it is better to optimise array access this way than to avoid the division in evaluating LEN (). I am happy to accept contrary opinions from the others. I do not believe that the bind_c issue is an issue. Your patch correctly deals with it IMHO. Subject to the above change in the value of _len, I think that your patch is OK for trunk. With best regards Paul On 4 January 2015 at 13:40, Andre Vehreschild ve...@gmx.de wrote: Hi Janus, hi Paul, hi Tobias, Janus: During code review, I found that I had the code in gfc_get_len_component() duplicated. So I now reintroduced and documented the routine making is more commonly usable and added more documentation. The call sites are now simplify.c (gfc_simplify_len) and trans-expr.c (gfc_trans_pointer_assignment). Attached is the reworked version of the patch. Paul, Tobias: Can one of you have a look at line 253 of the patch? I need some expertise on the bind_c behavior. My patch needs the check for is_bind_c added in trans_expr.c (gfc_conv_expr) to prevent mistyping
Re: [PATCH, fortran] PR fortran/60255 Deferred character length
Dear Andre, Don't worry about the lack of a laptop. I am on to it :-) Paul On 17 January 2015 at 16:14, Andre Vehreschild ve...@gmx.de wrote: Dear Paul, Setting _Len to one by default should be quite simple. When I remember correctly, then there is only one place where the current code sets it to zero. Could be in gfc_conv_structure but that is only a guess. Unfortunately am I on travel 'till Sunday and don't have my laptop with me. Sorry for that. Regards, Andre Andre Vehreschild * Kreuzherrenstr. 8 * 52062 Aachen Tel. +49 241.9291018 * ve...@gmx.de Paul Richard Thomas paul.richard.tho...@gmail.com schrieb: Cancel that - It should be multiplies by kind, shouldn't it ? :-) OK, string length it is. We will probably have to set _len = 1 for other dynamic types, though, so that the pointer arising from an array reference is base_address + _len*vptr-size*index Cheers Paul On 17 January 2015 at 13:44, Paul Richard Thomas paul.richard.tho...@gmail.com wrote: Dear Andre, I am open to either - what do the others think? The reason why I am for differentiating between unlimited_polymorphic objects and the deferred length character arrays is because of the difference in the way in which arrays are accessed. The former uses pointer arithmetic and the latter array references. I was trying to avoid divisions by KIND within scalarization loops. Also, I found that in developing your patch, that allocating with unlimited polymorphic sources looks neatest when the _len contains the memory size of the elements of any dynamic type, since a priori it is not known at compile time whether it is a character or not. Of course, one could interrogate the _hash field of the vtable, at the expense of more runtime code. Cheers Paul PS I have your patches for PR60357 and 61275 regtesting right now. Both look OK to me. At the risk of making potential regressions more complicated to unravel, to save my time I intend to commit both at once, unless anybody objects. On 17 January 2015 at 13:10, Andre Vehreschild ve...@gmx.de wrote: Hi Paul, I am open on what to call the new component. Have you thought about my findings, that for deferred length char arrays the length is stored in characters and not in bytes, I.e., for a character(kind=4, Len=:) the length is stored in number of characters and not in bytes needed, which would be Len*4. IMHO both concepts should be changed, or none. I favor to keep storing the string length of both concepts (deferred char arrays and chararrays in unlimited polymorphic entities) interchangeable w/o computation. What's your opinion? Regards, Andre Andre Vehreschild * Kreuzherrenstr. 8 * 52062 Aachen Tel. +49 241.9291018 * ve...@gmx.de Paul Richard Thomas paul.richard.tho...@gmail.com schrieb: Dear Andre, Perhaps, rather than calling the new component _len, we should call it _mem_size or some such? Cheers Paul On 9 January 2015 at 11:52, Andre Vehreschild ve...@gmx.de wrote: Hi all, hi Paul, I started to implement the changes requested below, but I stumbled over an oddity: For a deferred length kind4 char array, the length of the string is stored without multiplication by 4 in the length variable attached. So when we now decide to store the length of the string in an unlimited polymorphic entity in bytes in the component formerly called _len and the size of each character in _vtype-_size then we have an inconsistency with the style deferred char lengths are stored. IMHO we should store this consistently, i.e., both 'length'-variables store either the length of the string ('length' = array_len) or the size of the memory needed ('length' = array_len * char_size). What do you think? Furthermore, think about debugging: When looking at an unlimited polymorphic entity storing a kind-4-char-array of length 7, then having a 'length' component set to 28 will lead to confusion. I humbly predict, that this will produce many entries in the bugtracker, because people don't understand that 'length' stores the product of elem_size times string_len, because all they see is an assignment of a length-7 char array. What do we do about it? Regards, Andre On Thu, 8 Jan 2015 20:56:43 +0100 Paul Richard Thomas paul.richard.tho...@gmail.com wrote: Dear Andre, Thanks for the patch. As I have said to you, off list, I think that the _size field in the vtable should contain the kind information and that the _len field should carry the length of the string in bytes. I think that it is better to optimise array access this way than to avoid the division in evaluating LEN (). I am happy to accept contrary opinions from the others. I do not believe that the bind_c issue is an issue. Your patch correctly deals with it IMHO. Subject to the above change in the value of _len, I think that your patch is OK for trunk. With best regards Paul On 4 January 2015 at 13:40, Andre Vehreschild
Re: [Fortran, Patch] Fix for PR60357 and possibly also for 55932, 57857 and others
Committed with the patch for PR61275 as revision 219801. Also fixes PR55932 for which a testcase has been added. Will follow with a commit to 4.9 during the week. Cheers Paul On 16 January 2015 at 12:30, Andre Vehreschild ve...@gmx.de wrote: Hi all, please find attached a fix for pr60357. This patch includes work published by Janus Weil in the bug. I have made the extensions to support allocatable scalar components in structure constructors. This patch also addresses allocatable deferred length char arrays in structure constructors, which are now supported. Furthermore is the artificial string-length component set correctly now. I hope I have covered all paths. Please note, that this patch does not fix allocatable deferred length char array components in types that are defined and exported in/from a module. For this bug Tobias Burnus wrote a patch, that will hopefully be published soon. During development I have found several related issues in the bugtracker notably: pr55932 - [F03] ICE for structure constructor with scalar allocatable component pr57959 - [F03] ICE with structure constructor with scalar allocatable comp. pr61275 - Invalid initialization expression for ALLOCATABLE component in structure constructor at (1) I haven't check which ones are covered by the patch, too. I hope for support of Dominique here, who is a valued resource for checking conflicts and suddenly fixed bugs. :-) Would you do that for me Dominique? All comments welcome. Bootstraps and regtests ok on x86_64-linux-gnu. Regards, Andre -- Andre Vehreschild * Kreuzherrenstr. 8 * 52062 Aachen Tel.: +49 241 9291018 * Email: ve...@gmx.de -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [Patch, fortran] PR55901 - [OOP] type is (character(len=*)) misinterpreted as array - part 1
Dear All, Steve Kargl has pointed out to me that the attachment was a null attachment. As is well known, these are the Swiss Army knife of patches and will fix anything. In fact, this patch was already posted long while since with the PR. However, since I am now in a slightly more pedestrian mood, I have attached it to this message too :-) Cheers Paul On 17 January 2015 at 11:55, Paul Richard Thomas paul.richard.tho...@gmail.com wrote: Dear All, The attached patch fixes the confusion between substrings of character associate-names and scalars being misused as arrays. It is sufficiently obvious and has been tested by Dominique that I will commit it later today if there are no opinions to the contrary. I will now turn my attention to Andre's patches for PR60357 and PR61275. Afterwards, I will fix the rest of the fix for this PR, via Andre's patch for PR60255 and the query in the final message in the thread. Bootstrapped and regtested on x86_64/FC21 - OK for trunk? Cheers Paul 2015-01-17 Paul Thomas pa...@gcc.gnu.org PR fortran/55901 * primary.c (gfc_match_varspec): Exclude dangling associate- names with dimension 0 from being counted as arrays. * resolve.c (resolve_assoc_var): Sub-strings are permissible for associate-names, so exclude characters from the test for misuse as arrays. * trans-decl.c (gfc_get_symbol_decl): Associate-names can use the hidden string length variable of their associated target. Signal this by setting 'length' to a constant, if the decl for the string length is a variable. 2015-01-17 Paul Thomas pa...@gcc.gnu.org PR fortran/55901 * gfortran.dg/associate_1.f03: Allow test for character with automatic length. -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx Index: gcc/fortran/primary.c === *** gcc/fortran/primary.c (revision 216426) --- gcc/fortran/primary.c (working copy) *** gfc_match_varspec (gfc_expr *primary, in *** 1855,1861 Thus if we have one and parentheses follow, we have to assume that it actually is one for now. The final decision will be made at resolution time, of course. */ ! if (sym-assoc gfc_peek_ascii_char () == '(') sym-attr.dimension = 1; if ((equiv_flag gfc_peek_ascii_char () == '(') --- 1855,1864 Thus if we have one and parentheses follow, we have to assume that it actually is one for now. The final decision will be made at resolution time, of course. */ ! if (sym-assoc gfc_peek_ascii_char () == '(' !!(sym-assoc-dangling sym-assoc-st ! sym-assoc-st-n.sym ! sym-assoc-st-n.sym-attr.dimension == 0)) sym-attr.dimension = 1; if ((equiv_flag gfc_peek_ascii_char () == '(') Index: gcc/fortran/resolve.c === *** gcc/fortran/resolve.c (revision 216427) --- gcc/fortran/resolve.c (working copy) *** resolve_assoc_var (gfc_symbol* sym, bool *** 7882,7887 --- 7882,7890 /* Finally resolve if this is an array or not. */ if (sym-attr.dimension target-rank == 0) { + /* primary.c makes the assumption that a reference to an associate +name followed by a left parenthesis is an array reference. */ + if (sym-ts.type != BT_CHARACTER) gfc_error (Associate-name '%s' at %L is used as array, sym-name, sym-declared_at); sym-attr.dimension = 0; Index: gcc/fortran/trans-decl.c === *** gcc/fortran/trans-decl.c(revision 216426) --- gcc/fortran/trans-decl.c(working copy) *** gfc_get_symbol_decl (gfc_symbol * sym) *** 1435,1441 --- 1435,1448 /* Create string length decl first so that they can be used in the type declaration. */ if (sym-ts.type == BT_CHARACTER) + { + if (sym-attr.associate_var + sym-ts.u.cl-backend_decl + TREE_CODE (sym-ts.u.cl-backend_decl) == VAR_DECL) + length = gfc_index_zero_node; + else length = gfc_create_string_length (sym); + } /* Create the decl for the variable. */ decl = build_decl (sym-declared_at.lb-location, *** gfc_get_symbol_decl (gfc_symbol * sym) *** 1497,1502 --- 1504,1511 /* Character variables need special handling. */ gfc_allocate_lang_decl (decl); + /* Associate names can use the hidden string length variable +of their associated target. */ if (TREE_CODE (length) != INTEGER_CST) { gfc_finish_var_decl (length, sym); Index: gcc/testsuite/gfortran.dg/associate_1.f03
[Patch, fortran] PR64578 - [OOP] Seg-fault and ICE with unlimited polymorphic array pointer function
Applied as 'obvious' in revision 219802. 2015-01-17 Paul Thomas pa...@gcc.gnu.org PR fortran/64578 * trans-expr.c (gfc_trans_pointer_assignment): Make sure that before reinitializing rse, to add the rse.pre to block before creating 'ptrtemp'. * trans-intrinsic.c (gfc_conv_associated): Deal with the class data being a descriptor. 2015-01-17 Paul Thomas pa...@gcc.gnu.org PR fortran/64578 * gfortran.dg/unlimited_polymorphic_21.f90: New test Cheers Paul -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx Index: gcc/fortran/trans-expr.c === *** gcc/fortran/trans-expr.c(revision 219801) --- gcc/fortran/trans-expr.c(working copy) *** gfc_trans_pointer_assignment (gfc_expr * *** 7075,7080 --- 7075,7081 rse.expr = gfc_class_data_get (rse.expr); else { + gfc_add_block_to_block (block, rse.pre); tmp = gfc_create_var (TREE_TYPE (rse.expr), ptrtemp); gfc_add_modify (lse.pre, tmp, rse.expr); *** gfc_trans_pointer_assignment (gfc_expr * *** 7146,7151 --- 7147,7153 } else { + gfc_add_block_to_block (block, rse.pre); tmp = gfc_create_var (TREE_TYPE (rse.expr), ptrtemp); gfc_add_modify (lse.pre, tmp, rse.expr); Index: gcc/fortran/trans-intrinsic.c === *** gcc/fortran/trans-intrinsic.c (revision 219800) --- gcc/fortran/trans-intrinsic.c (working copy) *** gfc_conv_associated (gfc_se *se, gfc_exp *** 6554,6560 --- 6554,6564 arg1se.expr = build_fold_indirect_ref_loc (input_location, arg1se.expr); if (arg1-expr-ts.type == BT_CLASS) + { tmp2 = gfc_class_data_get (arg1se.expr); + if (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (tmp2))) + tmp2 = gfc_conv_descriptor_data_get (tmp2); + } else tmp2 = arg1se.expr; } Index: gcc/testsuite/gfortran.dg/unlimited_polymorphic_21.f90 === *** gcc/testsuite/gfortran.dg/unlimited_polymorphic_21.f90 (revision 0) --- gcc/testsuite/gfortran.dg/unlimited_polymorphic_21.f90 (working copy) *** *** 0 --- 1,35 + ! { dg-do run } + ! Tests the fix for PR64578. + ! + ! Contributed by Damian Rouson dam...@sourceryinstitute.org + ! + type foo + real, allocatable :: component(:) + end type + type (foo), target :: f + class(*), pointer :: ptr(:) + allocate(f%component(1),source=[0.99]) + call associate_pointer(f,ptr) + select type (ptr) + type is (real) + if (abs (ptr(1) - 0.99) 1e-5) call abort + end select + ptr = return_pointer(f) ! runtime segmentation fault + if (associated(return_pointer(f)) .neqv. .true.) call abort + select type (ptr) + type is (real) + if (abs (ptr(1) - 0.99) 1e-5) call abort + end select + contains + subroutine associate_pointer(this, item) + class(foo), target :: this + class(*), pointer :: item(:) + item = this%component + end subroutine + function return_pointer(this) + class(foo), target :: this + class(*), pointer :: return_pointer(:) + return_pointer = this%component + end function + end +
Re: [Patch, Fortran, OOP] PR 63733: [4.8/4.9/5 Regression] wrong resolution for OPERATOR generics
Dear Janus, Since it is a regression, by all means update the branches. We usually, propose delaying a bit but I am not convinced that this is effective for this kind of bug fix - usually, further problems take a long time to emerge. Thus, I would recommend that you get on with it. Thanks Paul On 11 January 2015 at 23:01, Janus Weil ja...@gcc.gnu.org wrote: Well done for sorting that out. OK for trunk. Thanks, Paul. Committed as r219440. What about the branches? Cheers, Janus On 11 January 2015 at 14:38, Janus Weil ja...@gcc.gnu.org wrote: Hi all, this patch fixes a wrong-code regression related to operators, by making sure that we look for typebound operators first, before looking for non-typebound ones. (Note: Each typebound operator is also added to the list of non-typebound ones, for reasons of diagnostics.) Regtested on x86_64-unknown-linux-gnu. Ok for trunk? 4.9/4.8? Cheers, Janus 2015-01-11 Janus Weil ja...@gcc.gnu.org PR fortran/63733 * interface.c (gfc_extend_expr): Look for type-bound operators before non-typebound ones. 2015-01-11 Janus Weil ja...@gcc.gnu.org PR fortran/63733 * gfortran.dg/typebound_operator_20.f90: New. -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [Fortran, Patch] PR60334 - Segmentation fault on character pointer assignments
Hi Andre, + if (INDIRECT_REF_P (parmse.string_length)) +/* In chains of functions/procedure calls the string_length already + is a pointer to the variable holding the length. Therefore + remove the deref on call. */ +parmse.string_length = TREE_OPERAND (parmse.string_length, 0); This is OK but I would use instead: + if (POINTER_TYPE_P (parmse.string_length)) +/* In chains of functions/procedure calls the string_length already + is a pointer to the variable holding the length. Therefore + remove the deref on call. */ +parmse.string_length = build_fold_indirect_ref (parmse.string_length); If you look in ~/gcc/fold-const.c:15751, you will see that TREE_OPERAND (parmse.string_length, 0) but that it is preceded by cleaning up of NOOPS and, in any case, its usage will preserve the standard API just in case the internals change :-) of course, using TREE_OPERAND (xxx, 0) in the various fortran class functions makes such an assumption ;-) Apart from that, the patch is fine. I'll have a session of doing some commits later this week and will do this patch at that time. Cheers Paul On 11 January 2015 at 16:21, Andre Vehreschild ve...@gmx.de wrote: Hi Paul, thanks for the review. I do not have commits rights. Unfortunately is the patch not ok. I figured today, that it needs an extension when function calls that return deferred char len arrays are nested. In this special case the string length would have been lost. The attached extended version fixes this issue. Sorry for the duplicate work. Bootstraps and regtests ok on x86_64-linux-gnu. Regards, Andre On Sun, 11 Jan 2015 16:11:10 +0100 Paul Richard Thomas paul.richard.tho...@gmail.com wrote: Dear Andre, This is OK for trunk. I have not been keeping track of whether or not you have commit rights yet. If not, I will get to it sometime this week. Thanks for the patch. Paul On 10 January 2015 at 15:59, Andre Vehreschild ve...@gmx.de wrote: Hi all, attached patch fixes the bug reported in pr 60334. The issue here was that the function's result being (a pointer to) a deferred length char array. The string length for the result value was wrapped in a local variable, whose value was never written back to the string length of the result. This lead the calling routine to take the length of the result to be random leading to a crash. This patch addresses the issue by preventing the instantiation of the local var and instead using a reference to the parameter. This not only saves one value on the stack, but also because for small functions the compiler will hold all parameters in registers for a significant level of optimization, all the overhead of memory access (I hope :-). Bootstraps and regtests ok on x86_64-linux-gnu. - Andre -- Andre Vehreschild * Kreuzherrenstr. 8 * 52062 Aachen Tel.: +49 241 9291018 * Email: ve...@gmx.de -- Andre Vehreschild * Kreuzherrenstr. 8 * 52062 Aachen Tel.: +49 241 9291018 * Email: ve...@gmx.de -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [Fortran, Patch] PR60334 - Segmentation fault on character pointer assignments
Dear Andre, This is OK for trunk. I have not been keeping track of whether or not you have commit rights yet. If not, I will get to it sometime this week. Thanks for the patch. Paul On 10 January 2015 at 15:59, Andre Vehreschild ve...@gmx.de wrote: Hi all, attached patch fixes the bug reported in pr 60334. The issue here was that the function's result being (a pointer to) a deferred length char array. The string length for the result value was wrapped in a local variable, whose value was never written back to the string length of the result. This lead the calling routine to take the length of the result to be random leading to a crash. This patch addresses the issue by preventing the instantiation of the local var and instead using a reference to the parameter. This not only saves one value on the stack, but also because for small functions the compiler will hold all parameters in registers for a significant level of optimization, all the overhead of memory access (I hope :-). Bootstraps and regtests ok on x86_64-linux-gnu. - Andre -- Andre Vehreschild * Kreuzherrenstr. 8 * 52062 Aachen Tel.: +49 241 9291018 * Email: ve...@gmx.de -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [Patch, Fortran, F03] PR 64508: interface check missing for procedure pointer component as actual argument
Dear Janus, As you say, the patch is pretty straightforward :-) OK for trunk. Thanks for the patch Paul On 11 January 2015 at 11:21, Janus Weil ja...@gcc.gnu.org wrote: Ping! (I think this patch is pretty straightforward ...) Cheers, Janus 2015-01-06 16:19 GMT+01:00 Janus Weil ja...@gcc.gnu.org: Hi all, here is a patch which adds an interface check for procedure pointer components as acual arguments. Such a check is there already for ordinary procedures and procedure pointers, but missing for PPCs. It checks the interface of the actual argument versus the interface of the dummy procedure, according to the usual rules. Regtested on x86_64-unknown-linux-gnu. Ok for trunk? Cheers, Janus 2015-01-06 Janus Weil ja...@gcc.gnu.org PR fortran/64508 * interface.c (compare_parameter): Interface check for procedure-pointer component as actual argument. 2015-01-06 Janus Weil ja...@gcc.gnu.org PR fortran/64508 * gfortran.dg/proc_ptr_comp_41.f90: New. -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [Patch, Fortran, OOP] PR 63733: [4.8/4.9/5 Regression] wrong resolution for OPERATOR generics
Dear Janus, Well done for sorting that out. OK for trunk. Thanks for the patch. Paul On 11 January 2015 at 14:38, Janus Weil ja...@gcc.gnu.org wrote: Hi all, this patch fixes a wrong-code regression related to operators, by making sure that we look for typebound operators first, before looking for non-typebound ones. (Note: Each typebound operator is also added to the list of non-typebound ones, for reasons of diagnostics.) Regtested on x86_64-unknown-linux-gnu. Ok for trunk? 4.9/4.8? Cheers, Janus 2015-01-11 Janus Weil ja...@gcc.gnu.org PR fortran/63733 * interface.c (gfc_extend_expr): Look for type-bound operators before non-typebound ones. 2015-01-11 Janus Weil ja...@gcc.gnu.org PR fortran/63733 * gfortran.dg/typebound_operator_20.f90: New. -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [PATCH, fortran] PR fortran/60255 Deferred character length
Dear Andre, Thanks for the patch. As I have said to you, off list, I think that the _size field in the vtable should contain the kind information and that the _len field should carry the length of the string in bytes. I think that it is better to optimise array access this way than to avoid the division in evaluating LEN (). I am happy to accept contrary opinions from the others. I do not believe that the bind_c issue is an issue. Your patch correctly deals with it IMHO. Subject to the above change in the value of _len, I think that your patch is OK for trunk. With best regards Paul On 4 January 2015 at 13:40, Andre Vehreschild ve...@gmx.de wrote: Hi Janus, hi Paul, hi Tobias, Janus: During code review, I found that I had the code in gfc_get_len_component() duplicated. So I now reintroduced and documented the routine making is more commonly usable and added more documentation. The call sites are now simplify.c (gfc_simplify_len) and trans-expr.c (gfc_trans_pointer_assignment). Attached is the reworked version of the patch. Paul, Tobias: Can one of you have a look at line 253 of the patch? I need some expertise on the bind_c behavior. My patch needs the check for is_bind_c added in trans_expr.c (gfc_conv_expr) to prevent mistyping an associated variable in a select type() during the conv. Background: This code fragment taken from the testcase in the patch: MODULE m contains subroutine bar (arg, res) class(*) :: arg character(100) :: res select type (w = arg) type is (character(*)) write (res, '(I2)') len(w) end select end subroutine END MODULE has the conditions required for line trans-expr.c:6630 of gfc_conv_expr when the associate variable w is converted. This transforms the type of the associate variable to something unexpected in the further processing leading to some issues during fortraning. Janus told me, that the f90_type has been abused for some other things (unlimited polymorphic treatment). Although I believe that reading the comments above the if in question, the check I had to enhance is treating bind_c stuff (see the threads content for more). I would feel safer when one of you gfortran gurus can have a look and given an opinion, whether the change is problematic. I couldn't figure why w is resolved to meet the criteria (any ideas). Btw, all regtest are ok reporting no issues at all. Bootstraps and regtests ok on x86_64-linux-gnu Regards, Andre On Sat, 3 Jan 2015 16:45:07 +0100 Janus Weil ja...@gcc.gnu.org wrote: Hi Andre, For the second one (in gfc_conv_expr), I don't directly see how it's related to deferred char-len. Why is this change needed? That change is needed, because in some rare case where an associated variable in a select type () is used, then the type and f90_type match the condition while them not really being in a bind_c context. Therefore I have added the check for bind_c. Btw, I now have removed the TODO, because that case is covered by the regression tests. I don't understand how f90_type can be BT_VOID without being in a BIND_C context, but I'm not really a ISO_C_BINDING expert. Which test case is the one that triggered this? This case is triggered by the test-case in the patch, where in the select type (w = arg) in module m routine bar the w meets the criteria to make the condition become true. The type of w is then fixed and gfortran would terminate, because the type of w would be set be and BT_INTEGER. I tried to backtrace where this is coming from, but to no success. In the resolve () of the select type it looks all quite ok, but in the trans stage the criteria are met. Most intriguing to me is, that in the condition we are talking about the type of w and f90_type of the derived class' ts (expr-ts.u.derived-ts.f90_type) of w is examined. But expr-ts.u.derived-ts does not describe the type of w, but of the class w is associate with __STAR... So I am not quite sure how to fix this, if this really needs fixing. When I understand you right, then f90_type should only be set in a bind_c context, so adding that check wouldn't hurt, right? Yes, in principle adding the check for attr.bind_c looks ok to me (alternatively one could also check for attr.unlimited_polymorphic). I think originally BT_VOID was indeed only used in a bind_c context, but recently it has also been 'hijacked' for unlimited polymorphism, e.g. for the STAR symbol and some of the components of the intrinsic vtabs. What I don't really understand is why these problems are triggered by your patch now and have not crept up earlier in other use-cases of CLASS(*). 3) The function 'gfc_get_len_component' that you're introducing is only called in a single place. Do you expect this to be useful in other places in the future, or could one remove the function and insert the code inline? In one of the first versions it
Re: [Patch, Fortran] Add CO_BROADCAST
Hi Tobias, In the check.c error messages, you use 'A argument'. Should you not use 'SOURCE argument', following CO BROADCAST (SOURCE, SOURCE IMAGE [, STAT, ERRMSG]) ? I am looking at WG5/N1983 - is there some more recent proposal? When do you intend to implement a _gfortran_caf_co_broadcast that does something? Anway, the patch is OK for trunk. Thanks Paul On 20 September 2014 16:09, Tobias Burnus bur...@net-b.de wrote: This patch adds a CO_BROADCAST and prepares a bit for CO_REDUCE. Both functions permit arguments with allocatable components (nonpolymophic or polymorphic), CO_BROADCAST also permits polymorphic arguments. This patch doesn't support allocatable/polymorphic arguments but otherwise CO_BROADCAST should work. For CO_REDUCE only some parsing/argument checking is done but no actual implementation. The allocatables make life harder for general coarray communication, broadcast and reduction and have to be implemented at some point in a clever way. I am thinking of some call-back-able function - which could also be used for OpenMP 4.x/5.0 to handle copying to threadprivate variables and for copyin/out to accelerators; the current spec handles allocatable components by creating the copying code in the middle end, but that won't work for polymorphic allocatables. For CO_REDUCE, it becomes even harder as currently any pure function works (elemental or not, passing arguments with array descriptor, as pointer or as value, having a hidden string length argument or [with C binding] not etc. Requiring packed array arguments or not, whether gfortran returns the result as value or as argument - and possibly more). There is some J3 discussion if one could narrow down the possibilities a bit. In any case, implementing co_reduce requires some thinking. The attached patch was build and regtested on x86-64-gnu-linux. OK for the trunk? Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, Fortran] CAF dep (1/3): PR62278 - improve dependency.c's gfc_check_dependency's check (missed-optimization)
Dear Tobias, Given Dominques news that this fixes a golden oldie that drove me to madness and PR60593 - OK for trunk, 4.8 and 4.9 Many thanks for the patch Paul On 27 August 2014 22:59, Tobias Burnus bur...@net-b.de wrote: The current gfc_check_dependency check always looked at the pointer attribute - and assumed the worst, if either the LHS or the RHS was true. Thus, it claimed that a and b alias for the following definition: integer, pointer :: p; integer :: a. However, as a has no target (or pointer) attribute, that's not possible. Additionally, class(t) :: a has internally the pointer attribute (but CLASS_DATA(sym)-attr.class_pointer == 0), however, in the Fortran sense, a is not a pointer and cannot alias. I do not have a good example for the test case, except for a similar one as above using a[i] = p and looking at the dump; but that requires patch 3/3 of this series. Build and regtested on x86-64-gnu-linux. (I do get a failure for gfortran.dg/graphite/pr42393.f90, but only with -O1 -fgraphite-identity and also without the patch.) OK for the trunk? Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, Fortran] CAF dep (2/3): Move code around, prepare for more locking support
Dear Tobias, Obviously there is no problem with this - OK for trunk Cheers Paul On 27 August 2014 23:37, Tobias Burnus bur...@net-b.de wrote: I claim that it is part 2 of 3 of the CAF dep series, but the patch has nothing to do with it, except that it is in the way. Technically, it just moves code from trans-intrinsic.c to trans-expr.c and makes it available. Additionally, I support the case offset == NULL_TREE, which is supposed to be used with lock variables, where we know that the coarray offset is always zero. That's used by my incomplete local lock patch. Build and regtested (as part of the series) on x86-64-gnu-linux. OK for the trunk? Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, Fortran] CAF dep (3/3): coarrays - pass may_require_tmp informtion for CAF_get/send/sendget to the library
Dear Tobias, This looks fine to me - OK for trunk. Thanks for this massive effort! Paul On 28 August 2014 08:13, Tobias Burnus bur...@net-b.de wrote: This patch is based on 1/2 and 2/2 on the series. When the patch is approved, OpenCoarrays needs to be adapted; however, as surplus arguments of the callee are ignored, no immediate action is required. (And some delay avoids issues with compilers being older than the library.) The issue comes up in the context of having a coarray access on the same image, e.g. a[this_image()] = a, where alias questions play a role. While one can leave the general handling to the library - such as switching to memmove in case of local memory access, this patch tries to help the library to decide whether it has to create a temporary variable or not. For that reason, it passes an may_require_temporary argument to the library. may_require_temporary is false if the source and target variables are disjunct, or if they are such overlapping that walking them in element order will not require a temporary (special case: identical). If the compiler cannot tell at compile time, the value is always one. Of course, if the memory access is for a different image than the current image (or for sendget: when the two image indexes are for different images), the library can ignore the argument may_require_temporary and use the normal remote memory access. Build and regtested on x86-64-gnu-linux. OK for the trunk? Tobias PS: I image code like the following in the library: if (image_index == this_image) { if (contiguous LHS and RHS): use memmove // With special case: LHS and RHS identical if (!may_require_temporary) for-loop assigning LHS = RHS in element order else { tmp = malloc() if (RHS contiguous or scalar) tmp = memcpy(RHS) else for loop assigning RHS to tmp if (LHS contiguous) LHS = memcpy(tmp) else for loop assigning tmp to LHS } } else { do normal remote-image assignment } -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [patch] PR fortran/61669
Dear Steven, I am constantly amazed that data statement bugs keep turning up:-) Anyway, your fix is fine for trunk and, if you feel so inclined, 4.8 and 4.9. Thanks Paul On 23 August 2014 16:52, Steven Bosscher stevenb@gmail.com wrote: Hello, This bug is an error recovery issue. A data declaration is parsed and accepted, and added to gfc_current_ns-data, but the statement is rejected. The rejected data decl is not rolled back, causing memory corruption later on. Proposed fix is to roll back DATA for rejected statements. Bootstrappedtested on powerpc64-unknown-linux-gnu. OK for trunk? Ciao! Steven -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [PATCH, Fortran] PR fortran/60289 First try on: Fixing character array allocation for class(*) type variable
Dear All, There are known issues with unlimited polymorphic variables representing characters : see https://groups.google.com/forum/#!topic/comp.lang.fortran/aRz3HMpblTs and https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55901 One way or another, the variable itself needs to carry the string length and the kind. With fixed length and kind=1, the vtable 'size' does the job. Tobias has suggested more than once that we use the new array descriptor as the class container as well. We either do this or add fields to the class container. Cheers Paul On 17 August 2014 18:39, Dominique Dhumieres domi...@lps.ens.fr wrote: Here is a failing testcase. I was about to post the same test. The test fails with two counts: (1) len(P) == 80, (2) deallocate(P) fails with a.out(882,0x7fff75e1d310) malloc: *** error for object 0x7fc801c04978: incorrect checksum for freed object - object was probably modified after being freed. ... Dominique -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [PATCH] Fix PR61950
Dear Richie and Tobias, I OK'd this patch on 9th August but I now see that the posting bounced because of mime content emanating from my phone mail reader :-( I also thought that the patch is obvious. Cheers Paul On 13 August 2014 23:22, Tobias Burnus bur...@net-b.de wrote: Hi Richard, sorry for the belated reply due to spending lazily my time at the sea ... The patch is okay - and I would even claim that it is obvious. Regarding the data type, I have no idea why it uses a hard-coded 32bit integer instead of a size-type one. I have added it as item to https://gcc.gnu.org/wiki/LibgfortranAbiCleanup to change it once we break the ABI. Tobias PS: I wonder where all the other patch reviewers are, given that quite a few patches have accumulated. In particular, I am lacking a patch reviewer myself. Richard Biener wrote: The following fixes PR61950 by properly initializing the _size field of the static constructor for the vtable init member. Previously gfortran would use a 64bit integer to initialize the 32bit size field (not sure why larger aggregates are not considered). This breaks more sophisticated constant-folding and previously inhibited constant folding (which would have worked in this case been there no mismatch between initializer and field). Bootstrap and regtest ongoing on powerpc64-linux-gnu, ok? I'm not considering a backport as it is only a missed optimization there. Thanks, Richard. 2014-07-31 Richard Biener rguent...@suse.de * trans-expr.c (gfc_conv_structure): Initialize _size with a value of proper type. Index: gcc/fortran/trans-expr.c === --- gcc/fortran/trans-expr.c(revision 213342) +++ gcc/fortran/trans-expr.c(working copy) @@ -6260,7 +6260,9 @@ gfc_conv_structure (gfc_se * se, gfc_exp else if (cm-ts.u.derived strcmp (cm-name, _size) == 0) { val = TYPE_SIZE_UNIT (gfc_get_derived_type (cm-ts.u.derived)); - CONSTRUCTOR_APPEND_ELT (v, cm-backend_decl, val); + CONSTRUCTOR_APPEND_ELT (v, cm-backend_decl, + fold_convert (TREE_TYPE (cm-backend_decl), + val)); } else { -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: PING – Re: [Patch, Fortran] -fcoarray=lib - support CRITICAL, prepare for locking support
Dear Tobias, dear all, This patch and the documentation patch are OK for trunk. Many thanks Paul On 6 August 2014 08:46, Tobias Burnus bur...@net-b.de wrote: * PING * – of the patch with the obvious change mentioned by Alessandro (i.e. using if(is_lock_type))? Tobias On 1 August 2014 21:57, Alessandro Fanfarillo wrote: Hello, I was implementing lock/unlock on the library side when I found a possible problem in the patch: if (is_lock_type == GFC_CAF_CRITICAL) +reg_type = sym-attr.artificial ? GFC_CAF_CRITICAL : GFC_CAF_LOCK_STATIC; + else +reg_type = GFC_CAF_COARRAY_STATIC; the if statement cannot be true since is_lock_type is a boolean and GFC_CAF_CRITICAL is 4. Using if (is_lock_type) it produces the right result for the lock registration. Regards Alessandro 2014-07-28 14:37 GMT-06:00 Tobias Burnus bur...@net-b.de: This patch implements -fcoarray=lib support for CRITICAL blocks and includes some preparatory work for locking. In particular: * Updated the documentation for locking/critical, minor cleanup. The patch goes on top of the unreviewed patch https://gcc.gnu.org/ml/fortran/2014-07/msg00155.html * Add libcaf_single implementation for lock/unlock * Add lock/unlock calls for CRITICAL * Register static/SAVEd locking variables and locking variables for critical sections. Build and currently regtesting on x86-64-gnu-linux. OK when it regtested successfully? * * * Still to be done as follow up: * Handling the registering of lock-type components in statically allocated derived types * Handling the registering of lock-type variables and components with allocate and with implicit/explicit deallocate * Calling lock/unlock function for those * Test case for locking and critical blocks Other coarray to-do items: * Type-conversion test case missing * Vector subscript library implementation + test cases * Extending the documentation * Issues with striding for coarray components of derived types * Nonallocatable polymophic coarrays and select type/associated * Allocatable/pointer components of coarrays; co_reduce and co_broadcast Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, Fortran] PRs 61881/61888 - Fix issues with SIZEOF, CLASS(*) and assumed-rank
Dear Tobias, Whilst I am aware that we can now use the single line C++ comment, would it perhaps be a better idea to stick with the C style comments just for uniformity? + if (arg-ts.type == BT_CLASS) +{ + tmp = gfc_vtable_size_get (TREE_OPERAND (argse.expr, 0)); + tmp = fold_convert (result_type, tmp); + goto done; +} Is there any possibility that the class object will be adorned by any kind of reference here? In which case, you should drill down through the TREE_OPERANDS to find it. Otherwise, this is OK for trunk. Thanks for the patch Paul On 26 July 2014 01:47, Tobias Burnus bur...@net-b.de wrote: The patch has been motivated by the needs for implementing the openacc module. In particular, it does: - Fix passing intrinsic types to CLASS(*) [F2003] - Fix STORAGE_SIZE for polymorphic arrays [F2003] - Permit the vendor intrinsic SIZEOF also for TYPE(*) if and only if an array descriptor has been used [extend GNU extension] - Fix SIZEOF with assumed-rank [fix GNU extension] - Document that SIZEOF's result are not well defined for noncontiguous arrays. [fix GNU extension] Build and regtested on x86-64-gnu-linux. OK for the trunk? Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, Fortran] Fix atomic_ref with -fcoarray=lib
Dear Tobias, It looks OK to me - good for trunk Thanks for the patch Paul On 14 July 2014 19:19, Tobias Burnus bur...@net-b.de wrote: If the atomic_ref VALUE argument is of a different kind than the ATOM argument, the result was wrong with -fcoarray=lib. That showed up with gfortran.dg/coarray/atomic_1.f90 - but for some reasons only with -m32. The fix is to create a temporary variable in this case, what the patch does. OK for the trunk? Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, Fortran] Add library support for coarray's atomic intrinsics
Dear Tobias, That's good for trunk. Thanks for the patch. Paul On 12 July 2014 17:05, Tobias Burnus bur...@net-b.de wrote: This patch is relative to the still unreviewed patch https://gcc.gnu.org/ml/gcc-patches/2014-07/msg00864.html With this patch, all of Fortran 2008's and TS18508's atomics should be supported with both -fcoarray=single and =lib (with libcaf_single). Still missing is the support in the MPI and GASNet multi-image libraries, which is supposed to get released soon. However, I think adding atomics support to libcaf_mpi should be very simple. I haven't included a -fdump-tree-original test case, but in the pending patch is coarray/atomic_2.f90, which a run-time test which is also run with -fcoarray=lib and -lcaf_single. The patch has been build and regtested on x86-64-gnu-linux. OK for the trunk? Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, Fortran] Update atomics support for TS18508
Dear Tobias, OK for trunk. Thanks for the patch. Paul On 11 July 2014 23:30, Tobias Burnus bur...@net-b.de wrote: This patch updates the atomic support for TS18508, namely: – The atomic intrinsics now take a STAT= argument – Add the new atomics atomic_{add,and,or,xor} and their fetch variants atomic_fetch_{add,and,or,xor} – and compare and swap (atomic_cas) In addition, the previous implementation (for -fcoarray=single) used simple assignments; the patch changes those to using real atomic operations, which makes the atomic operations thread safe. Build and regtested on x86-64-gnu-linux. OK for the trunk? As next step, library calls have to be added for -fcoarray=lib. For coarrays, I also a have still on my to do list: Adding a type-conversion test case for -fcorray=lib; implementing in the library the support for vector subscripts; and fixing an issue with nonallocatable polymorphic dummy coarrays and select type. For full F2008 support, the following is needed as well: locking and critical blocks – and allocatable/pointer components of derived types. For TS18508 much more is needed, in particular co_reduce/co_broadcast and team support. Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Bug fortran/61780] [4.8/4.9/4.10 Regression] Wrong code when shifting elements of a multidimensional array
Dear All, Committed to trunk as revision 212486. Cheers Paul On 12 July 2014 20:52, Tobias Burnus bur...@net-b.de wrote: Dear Paul, the patch attached to the PR looks good to me for GCC 4.8, 4.9 and 4.10. For 4.9.1 vs. 4.9.2, it's Jakub's call. Tobias Paul Richard Thomas wrote: Dear Jakub, PR61780 came at us out of the blue on the 11th. It's quite a nasty wrong code bug but it was planted slightly more than three years ago. I don't know if that is because the bug is rarely encountered or is going into code unnoticed. I posted a fix for it today that is pretty bomb-proof and bootstraps and regtests OK. Do you think that I should slip it into 4.9 before release of 4.9.1 or should I hold off? I am happy either way but could do the business right now. I realise that on the scale of blocking regressions it doesn't even appear on the meter. Best regards Paul On 12 July 2014 17:18, paul.richard.thomas at gmail dot com gcc-bugzi...@gcc.gnu.org wrote: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61780 --- Comment #3 from paul.richard.thomas at gmail dot com paul.richard.thomas at gmail dot com --- Dear Mikael, I didn't see your posting, which was about an hour before mine. At least we came to the same conclusion! Thanks Paul On 12 July 2014 13:43, mikael at gcc dot gnu.org gcc-bugzi...@gcc.gnu.org wrote: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61780 Mikael Morin mikael at gcc dot gnu.org changed: What|Removed |Added CC||mikael at gcc dot gnu.org --- Comment #1 from Mikael Morin mikael at gcc dot gnu.org --- This sets loop reversal in dependency.c: /* Set reverse if backward dependence and not inhibited. */ if (reverse reverse[n] == GFC_ENABLE_REVERSE) reverse[n] = (this_dep == GFC_DEP_BACKWARD) ? GFC_REVERSE_SET : reverse[n]; However, the 'n' used indexes over array ref dimension, so in the case at hand the second element is flagged as GFC_REVERSE_SET. But the 'reverse' array is used later on using scalarizer dimensions, and as y(k,4:n) and y(k,3:n-1) are one-dimension arrays, only the first element of 'reverse' is ever looked at. -- You are receiving this mail because: You are on the CC list for the bug. -- You are receiving this mail because: You are on the CC list for the bug. You are the assignee for the bug. -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
[Patch, fortran] PR61459 - [4.8/4.9/4.10 Regression] segfault when assigning to allocatable function result from matmul result
Dear All, This PR is caused by a rather unfortunate cast for the pointer to the return value from matmul: struct array1_real(kind=4) D.2357; D.2357 = *(struct array1_real(kind=4) *) __result; D.2357.data = 0B; snip _gfortran_matmul_r4 (*(struct array1_real(kind=4) * *) D.2357, D.2384, D.2391, 0, 0, 0B); Unsurprisingly, matmul_r4 segfaults because it is passed a NULL for the descriptor. The fix is obvious (ie use its natural kind) and results in: _gfortran_matmul_r4 (D.2357, D.2384, D.2391, 0, 0, 0B); Dominique points out that the patch fixes PR58883 as well. I'll enhance the testcase appropriately. Unless anybody has any objections, I'll commit to trunk tonight and to 4.8 and 4.9 next weekend. Cheers Paul 2014-07-06 Paul Thomas pa...@gcc.gnu.org PR fortran/61459 * trans-expr.c (fcncall_realloc_result): Use the natural type for the address expression of 'res_desc'. 2014-07-06 Paul Thomas pa...@gcc.gnu.org PR fortran/61459 * gfortran.dg/allocatable_function_8.f90 : New test Index: gcc/fortran/trans-expr.c === *** gcc/fortran/trans-expr.c (revision 211867) --- gcc/fortran/trans-expr.c (working copy) *** fcncall_realloc_result (gfc_se *se, int *** 7299,7305 res_desc = gfc_evaluate_now (desc, se-pre); gfc_conv_descriptor_data_set (se-pre, res_desc, null_pointer_node); ! se-expr = gfc_build_addr_expr (TREE_TYPE (se-expr), res_desc); /* Free the lhs after the function call and copy the result data to the lhs descriptor. */ --- 7299,7305 res_desc = gfc_evaluate_now (desc, se-pre); gfc_conv_descriptor_data_set (se-pre, res_desc, null_pointer_node); ! se-expr = gfc_build_addr_expr (NULL_TREE, res_desc); /* Free the lhs after the function call and copy the result data to the lhs descriptor. */ Index: gcc/testsuite/gfortran.dg/allocatable_function_8.f90 === *** gcc/testsuite/gfortran.dg/allocatable_function_8.f90 (revision 0) --- gcc/testsuite/gfortran.dg/allocatable_function_8.f90 (working copy) *** *** 0 --- 1,47 + ! { dg-do run } + ! Test the fix for PR61459. + ! + ! Contributed by John Wingate joh...@tds.net + ! + module a + +implicit none +private +public :: f_segfault, f_segfault_plus, f_workaround +integer, dimension(2,2) :: b = reshape([1,-1,1,1],[2,2]) + + contains + +function f_segfault(x) + real, dimension(:), allocatable :: f_segfault + real, dimension(:), intent(in) :: x + allocate(f_segfault(2)) + f_segfault = matmul(b,x) +end function f_segfault + + ! Sefaulted without the ALLOCATE as well. +function f_segfault_plus(x) + real, dimension(:), allocatable :: f_segfault_plus + real, dimension(:), intent(in) :: x + f_segfault_plus = matmul(b,x) +end function f_segfault_plus + +function f_workaround(x) + real, dimension(:), allocatable :: f_workaround + real, dimension(:), intent(in) :: x + real, dimension(:), allocatable :: tmp + allocate(f_workaround(2),tmp(2)) + tmp = matmul(b,x) + f_workaround = tmp +end function f_workaround + + end module a + + program main +use a +implicit none +real, dimension(2) :: x = 1.0, y +y = f_workaround (x) +if (any (f_segfault (x) .ne. y)) call abort +if (any (f_segfault_plus (x) .ne. y)) call abort + end program main
Re: [Patch, fortran] PR61459 - [4.8/4.9/4.10 Regression] segfault when assigning to allocatable function result from matmul result
Dear Tobias, The patch is so safe and, as you say, has no side effects that I will commit to 4.9 first. Cheers Paul On 7 July 2014 10:05, Tobias Burnus tobias.bur...@physik.fu-berlin.de wrote: Paul Richard Thomas wrote: This PR is caused by a rather unfortunate cast for the pointer to the return value from matmul: [...] The fix is obvious (ie use its natural kind) [...] Dominique points out that the patch fixes PR58883 as well. I'll enhance the testcase appropriately. Nice! Unless anybody has any objections, I'll commit to trunk tonight and to 4.8 and 4.9 next weekend. Looks good to me. As the patch shouldn't have side effects and as Jakub intents to do a 4.9.1 RC early this week, I'd prefer it the 4.9 patch would be committed earlier. Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: Ping [PATCH, fortran] PR 41936 Memory leakage with allocatables and user-defined operators
Dear Dominique, On condition that your commit the right patch :-), OK for 4.9.1 Cheers Paul On 7 July 2014 13:48, Dominique Dhumieres domi...@lps.ens.fr wrote: In https://gcc.gnu.org/ml/fortran/2014-06/msg00150.html I wrote I am now trying to do the back port to 4.9 and I got two failures: ... Actually I did not applied the right patch, with it the testsuite pass without regression. Still OK for 4.9.1? TIA Dominique -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [PATCH, gfortran testsuite] Minor cleanup for the gfortran test suite, v2.
Dear Dominique, OK for trunk. Thanks for the patch Paul On 5 July 2014 18:09, Dominique Dhumieres domi...@lps.ens.fr wrote: On Mon, 05 May 2014, I have posted https://gcc.gnu.org/ml/fortran/2014-05/msg00012.html. On IRC Tobias Burnus remarked that the *.mod file in gfortran.dg/vect/fast-math-real8-pr40801.f90 should be cleaned automatically. This is not done because the cleaning is done in gfortran-dg-runtest and not in dg-runtest, hense the submitted patch. OK for trunk? Dominique 2014-07-05 Dominique d'Humieres domi...@lps.ens.fr * gfortran.dg/list_read_12.f90: Delete the file. * gfortran.dg/vect/vect.exp: Use gfortran-dg-runtest instead of dg-runtest. --- ../_clean/gcc/testsuite/gfortran.dg/list_read_12.f902014-03-15 16:19:35.0 +0100 +++ gcc/testsuite/gfortran.dg/list_read_12.f90 2014-04-17 10:27:58.0 +0200 @@ -7,5 +7,6 @@ close(99) open(99, access='sequential', form='formatted') read(99, *, iostat=ios) i +close(99, status=delete) if (ios /= 0) call abort end --- ../_clean/gcc/testsuite/gfortran.dg/vect/vect.exp 2014-07-04 10:51:07.0 +0200 +++ gcc/testsuite/gfortran.dg/vect/vect.exp 2014-07-05 14:18:33.0 +0200 @@ -57,43 +57,43 @@ set SAVED_DEFAULT_VECTCFLAGS $DEFAULT_VE # -ffast-math tests set DEFAULT_VECTCFLAGS $SAVED_DEFAULT_VECTCFLAGS lappend DEFAULT_VECTCFLAGS -ffast-math -dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/fast-math-*.\[fF\]{,90,95,03,08} ]] \ - $DEFAULT_VECTCFLAGS +gfortran-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/fast-math-*.\[fF\]{,90,95,03,08} ]] \ + $DEFAULT_VECTCFLAGS # -ffast-math tests set DEFAULT_VECTCFLAGS $SAVED_DEFAULT_VECTCFLAGS lappend DEFAULT_VECTCFLAGS -ffast-math -fdefault-real-8 -dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/fast-math-real8*.\[fF\]{,90,95,03,08} ]] \ +gfortran-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/fast-math-real8*.\[fF\]{,90,95,03,08} ]] \ $DEFAULT_VECTCFLAGS # -fvect-cost-model tests set DEFAULT_VECTCFLAGS $SAVED_DEFAULT_VECTCFLAGS lappend DEFAULT_VECTCFLAGS -fvect-cost-model=dynamic -dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/cost-model-*.\[fF\]{,90,95,03,08} ]] \ +gfortran-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/cost-model-*.\[fF\]{,90,95,03,08} ]] \ $DEFAULT_VECTCFLAGS # --param vect-max-version-for-alias-checks=0 tests set DEFAULT_VECTCFLAGS $SAVED_DEFAULT_VECTCFLAGS lappend DEFAULT_VECTCFLAGS --param vect-max-version-for-alias-checks=0 -dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/no-vfa-*.\[fF\]{,90,95,03,08} ]] \ +gfortran-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/no-vfa-*.\[fF\]{,90,95,03,08} ]] \ $DEFAULT_VECTCFLAGS # With -O3 set DEFAULT_VECTCFLAGS $SAVED_DEFAULT_VECTCFLAGS lappend DEFAULT_VECTCFLAGS -O3 -dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/O3-*.\[fF\]{,90,95,03,08} ]] \ +gfortran-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/O3-*.\[fF\]{,90,95,03,08} ]] \ $DEFAULT_VECTCFLAGS # With -Ofast set DEFAULT_VECTCFLAGS $SAVED_DEFAULT_VECTCFLAGS lappend DEFAULT_VECTCFLAGS -Ofast -dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/Ofast-*.\[fF\]{,90,95,03,08} ]] \ +gfortran-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/Ofast-*.\[fF\]{,90,95,03,08} ]] \ $DEFAULT_VECTCFLAGS # With -fno-tree-copy-prop -fno-tree-fre -O3 set DEFAULT_VECTCFLAGS $SAVED_DEFAULT_VECTCFLAGS lappend DEFAULT_VECTCFLAGS -fno-tree-copy-prop -fno-tree-fre -O3 -dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/no-fre-no-copy-prop-O3-*.\[fF\]{,90,95,03,08} ]] \ +gfortran-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/no-fre-no-copy-prop-O3-*.\[fF\]{,90,95,03,08} ]] \ $DEFAULT_VECTCFLAGS # Clean up. -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, Fortran] Update gfortran.texi's 2003/2008 status
To quote William Shakespeare: FALSTAFF ‘Tis not due yet; I would be loath to pay him before his day. What need I be so forward with him that calls not on me? Well, ’tis no matter; honour pricks me on. Yea, but how if honour prick me off when I come on? how then? Can honour set to a leg? no: or an arm? no: or take away the grief of a wound? no. Honour hath no skill in surgery, then? no. What is honour? a word. What is in that word honour? what is that honour? air. A trim reckoning! Who hath it? he that died o’ Wednesday. Doth he feel it? no. Doth he hear it? no. ‘Tis insensible, then. Yea, to the dead. But will it not live with the living? no. Why? detraction will not suffer it. Therefore I’ll none of it. Honour is a mere scutcheon: and so ends my catechism. What can I say more :-) ? Paul On 7 July 2014 00:20, Gerald Pfeifer ger...@pfeifer.com wrote: On Sat, 8 Mar 2014, Tobias Burnus wrote: An update the gfortran.texi's F2003/F2008 status. I just made the following change on top. Also, I noticed that you used British English for honour. There are now five uses of honor in gcc/fortran/* and five uses of honour. In gcc/doc, and in general, we use American English, and per https://gcc.gnu.org/codingconventions.html we should use that. I did not change this yet, to see whether there is any strong opposition making things consistent. Gerald 2014-07-07 Gerald Pfeifer ger...@pfeifer.com * gfortran.texi (Fortran 2003 status): Fix grammar. Index: gfortran.texi === --- gfortran.texi (revision 212307) +++ gfortran.texi (working copy) @@ -926,7 +926,7 @@ @item Minor I/O features: Rounding during formatted output, using of a decimal comma instead of a decimal point, setting whether a plus sign -should appear for positive numbers. On system where @code{strtod} honours +should appear for positive numbers. On systems where @code{strtod} honours the rounding mode, the rounding mode is also supported for input. @item -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: *ping* - [Patch, Fortran] Coarray fixes for select type/associate and type of derived components
Dear Tobias, This looks absolutely fine to me - good for trunk. Thanks for the patch and for the overall co-array effort. Paul On 4 July 2014 08:02, Tobias Burnus bur...@net-b.de wrote: Another somewhat early PING**2 On June 29, 2014, Tobias Burnus wrote: This patch fixes some issues with polymorphic coarrays. I still have to fix at least one issue. Fixed by the patch: a) The temporary pointer generated with SELECT TYPE has to be a coarray. That's fixed with the resolve.c patch. The comment is also bogus: The comment is correct – and gfortran correctly detects coindexed variables as selector. However, in the code in question, the selector is not coindexed but the variable in the coindexed section is. b) It doesn't make sense to try to initialize the temporary pointer of SELECT TYPE (or ASSOCIATE), thus we have to exclude it also in trans-decl.c c) As the temporary variable is internally a pointer, the assert in trans-array.c also has to accept a pointer – even though coarrays with token in the descriptor can only be allocatable. But for code like a(1)[1]), a(1) is not longer a pointer – and one ends up having an akind of unknown. Instead of adding all kind of values, I simply removed the assert. d) In trans-intrinsic.c, one has a similar issue. We now avoid an ICE by checking whether the variable is set before accessing it. e) For caf(:)[i]%a, we had the dtype of the descriptor of caf instead of ...%a. That's now fixed. Build and regtested on x86-64-gnu-linux. OK for the trunk? Tobias PS: Still to be done for coarrays: Nonallocatable polymorphic coarray dummies. For those, the offset and the token is passed as additional argument – but that's not yet correctly handled with ASSOCIATE/SELECT TYPE. Also to be done are more type-conversion checks (beyond those which are implicitly checked by this patch) – and the handling of vector subscripts. -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [patch, libgfortran] [4.9/4.10 Regression] Internal read of negative integer broken
Hi Jerry, The patch looks to be OK for trunk. Did you check it with the NIST by any chance? Thanks a lot Paul On 26 June 2014 03:58, Jerry DeLisle jvdeli...@charter.net wrote: Hi, This bug has nothing to do with negative numbers as in the description. However, the problem is due to seeking when there are no spaces to skip. I restructured the loop so that the skipping is not done if there are no spaces. Regression tested on x86-64. New test case from the PR. OK for trunk and 4.9? Regards, Jerry 2014-06-25 Jerry DeLisle jvdeli...@gcc.gnu.org PR libgfortran/61499 * io/list_read.c (eat_spaces): Use a 'for' loop instead of 'while' loop to skip the loop if there are no bytes left in the string. Only seek if actual spaces can be skipped. -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Three pending co-array patches
Dear Tobias, I have taken a look through the following three patches: * https://gcc.gnu.org/ml/fortran/2014-06/msg00178.html * https://gcc.gnu.org/ml/fortran/2014-06/msg00183.html * https://gcc.gnu.org/ml/fortran/2014-06/msg00187.html I can find nothing to object to in any of them; on the contrary, I can see every good reason to applaud them! This is a brilliant achievement. OK for trunk Paul
Re: [fortran,patch] One-line fix to PR61454 (init expression simplification)
Dear FX, Not only is it 'obvious' but it can do no harm in any circumstances :-) OK to commit Thanks Paul On 19 June 2014 13:14, FX fxcoud...@gmail.com wrote: In expr.c:scalarize_intrinsic_call(), we don't deal correctly with intrinsics that have an optional kind argument, while simplifying initialization expressions. The attached one-line patch fixes it, and adds a testcase so we don’t regress. Bootstrapped and regtested on x86_64-apple-darwin13. OK to commit? FX -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, Fortran] Add coarray communication support to the trunk (coindex variables)
Dear Tobias and Alessandro, Well what can I say? The patch is something of a tour de force! Sandro, questo è assolutamente meraviglioso. Molte grazie da tutti noi. I have done nothing to check the functionality of the patch. However, I have checked the conformance with coding standards and that it is well and truly insulated from the rest of gfortran by the coarray option. OK for trunk Once again many thanks for the patch. Paul On 17 June 2014 08:28, Tobias Burnus bur...@net-b.de wrote: This patch add the first coarray communication support to the trunk (ignoring the co_sum/co_min/co_max support, which was recently merged). [Note: In terms of the library this is still libcaf_single, but see below.] The patch is based on my work on the fortran-caf branch, but has a slightly modified ABI. The patch should support most communications, but it is not complete. I intent to submit soon a patch which irons some wrinkles. In particular, this patch adds three library calls to handle coindexed communication: Assignment to a coindex variable (caf_send), a coindexed expression (caf_expression) and assigning a coindexed variable to a coindexed variable (caf_sendget). The coarray is identified by a token (opaque object provided by the coarray library), an offset to that base address, an image index and an array descriptor for the coarray, which is also used for scalars – and which has the value of the whole array for vector subscripts. Additionally, one passes a kind variable as extra argument as the current array descriptor cannot destinguish a len=1 kind=4 from a len=4 kind=1 character string. And for vector subscripts, the subscripts are passed as additional argument. For assignments, the library is supposed to handle padding/trimming of strings and type conversion (e.g. cmplx_caf(:)[i] = int_array) as well as array = scalar assignments. The following is left to be done as follow up: * Support of vector subscripts with assumed-size variables: To be tested; might need the new array descriptor or some similar work around – or just a test case. * The library libcaf_single supports padding/trimming of strings but still lacks the support for type conversion and vector subscripts. * Adding an ABI documentation * There are still some issues with regards to polymorphic coarrays, in particular with passing them as dummy arguments and in ASSOCIATE/SELECT TYPE, but presumably also with using them in coindexed expressions. And as bigger item: Allocatable components of coarrays are not supported – not is the access to pointer or allocatable components (part refs); currently, there is no compile time diagnostic for it. Additionally, I have remove the vector subscript preparations from the co_sum/min/max as it does not make much sense for those. And I added a collective test case, which I found on my hard disk. Build and regtested. OK for the trunk? Tobias PS: Additional missing bits, not listed above: Locking and CRITICAL and atomics for Fortran 2008. And for TS18508 co_broadcast and co_reduce, the atomics extensions, teams, events and error recovery. -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
[Patch, fortran-dev] Fix regression in transpose_4.f90
2014-06-14 Paul Thomas pa...@gcc.gnu.org * trans-array.c (gfc_conv_array_parameter): Assign 'old_desc' to 'new_desc' rather than some of the components. Bootstraps and regtests OK Committed at revision 211664. Cheers Paul
Re: [fortran,patch] Fix Cray pointers in modules
Dear Francois-Xavier, The patch and the logic behind it, including the name dependence, is fine. OK for trunk and, I suggest, 4.9 and 4.8 after a suitable delay. Thanks! Paul On 8 June 2014 18:40, FX fxcoud...@gmail.com wrote: The attached one-line patch fixes PR45187 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=45187), where we would sometimes try to create the backend_decl of Cray-pointees twice. We should simply bail out early of the procedure, following what is done in the similar situation in gfc_finish_var_decl(). Bootstrapped and regtested on x86_64-apple-darwin13, includes a testcase. OK to commit? FX -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Patch, fortran] PR61406 - [4.9/4.10 Regression] ICE on ASSOCIATE construct to literal array expression
Dear All, This was a slip up on my part. The fix is obvious - OK for trunk and 4.9? Cheers Paul 2014-06-09 Paul Thomas pa...@gcc.gnu.org PR fortran/61406 * trans-stmt.c (trans_associate_var): Check that array constructors are constant for direct reference. 2014-06-09 Paul Thomas pa...@gcc.gnu.org PR fortran/61406 * gfortran.dg/associate_17.f90 : New test Index: gcc/fortran/trans-stmt.c === *** gcc/fortran/trans-stmt.c(revision 211365) --- gcc/fortran/trans-stmt.c(working copy) *** trans_associate_var (gfc_symbol *sym, gf *** 1167,1179 { gfc_se se; tree desc; desc = sym-backend_decl; /* If association is to an expression, evaluate it and create temporary. Otherwise, get descriptor of target for pointer assignment. */ gfc_init_se (se, NULL); ! if (sym-assoc-variable || e-expr_type == EXPR_ARRAY) { se.direct_byref = 1; se.use_offset = 1; --- 1167,1182 { gfc_se se; tree desc; + bool cst_array_ctor; desc = sym-backend_decl; + cst_array_ctor = e-expr_type == EXPR_ARRAY + gfc_constant_array_constructor_p (e-value.constructor); /* If association is to an expression, evaluate it and create temporary. Otherwise, get descriptor of target for pointer assignment. */ gfc_init_se (se, NULL); ! if (sym-assoc-variable || cst_array_ctor) { se.direct_byref = 1; se.use_offset = 1; *** trans_associate_var (gfc_symbol *sym, gf *** 1184,1190 /* If we didn't already do the pointer assignment, set associate-name descriptor to the one generated for the temporary. */ ! if (!sym-assoc-variable e-expr_type != EXPR_ARRAY) { int dim; --- 1187,1193 /* If we didn't already do the pointer assignment, set associate-name descriptor to the one generated for the temporary. */ ! if (!sym-assoc-variable !cst_array_ctor) { int dim; Index: gcc/testsuite/gfortran.dg/associate_17.f90 === *** gcc/testsuite/gfortran.dg/associate_17.f90 (revision 0) --- gcc/testsuite/gfortran.dg/associate_17.f90 (working copy) *** *** 0 --- 1,12 + ! { dg-do run } + ! Test the fix for PR61406 + ! Contributed by Adam Hirst a...@aphirst.karoo.co.uk + program test + implicit none + real :: theta = 1.0 + + associate (n = [cos(theta), sin(theta)]) + if (abs (norm2(n) - 1.0) .gt. 1.0e-4) call abort + end associate + + end program test
Re: [Patch, fortran] PR61406 - [4.9/4.10 Regression] ICE on ASSOCIATE construct to literal array expression
Dear Dominique, Committed to trunk as revision 211374. I decided that getting rid of the double temporary was delving too deep into the workings of trans-array.c to be worthwhile at the moment. Cheers Paul On 9 June 2014 12:05, Dominique Dhumieres domi...@lps.ens.fr wrote: Dear Paul, This was a slip up on my part. The fix is obvious - OK for trunk and 4.9? The patch fixes the ICE. Any progress about the double temporary? [Book15] f90/bug% gfc pr61406.f90 -Warray-temporaries pr61406.f90:6.18: associate (n = [cos(theta), sin(theta)]) 1 Warning: Creating array temporary at (1) pr61406.f90:6.18: associate (n = [cos(theta), sin(theta)]) 1 Warning: Creating array temporary at (1) Thanks for the patch. Dominique -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: Ping [PATCH, fortran] PR 41936 Memory leakage with allocatables and user-defined operators
Dear Dominique, Without the patch applied, trunk shows 360 bytes definitely missing in 3 blocks for the original testcase and 9 out of 92 allocs did not have corresponding frees. WIth the patch, I get: ==20527== ==20527== HEAP SUMMARY: ==20527== in use at exit: 0 bytes in 0 blocks ==20527== total heap usage: 92 allocs, 92 frees, 54,502 bytes allocated ==20527== ==20527== All heap blocks were freed -- no leaks are possible ==20527== ==20527== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2) --20527-- --20527-- used_suppression: 2 glibc-2.5.x-on-SUSE-10.2-(PPC)-2a /usr/lib64/valgrind/default.supp:1286 ==20527== ==20527== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2) Not only that, it bootstraps and regtests OK. So it definitely does what it is advertised to do :-) OK for trunk and, I would suggest 4.9. Cheers Paul PS ifort 13.01 has exactly the same fault On 9 June 2014 14:59, Dominique Dhumieres domi...@lps.ens.fr wrote: Patch posted at https://gcc.gnu.org/ml/fortran/2014-05/msg00155.html. TIA Dominique -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, Fortran] Add support for TS18508's CO_SUM/MAX/MIN (part 1/2)
Dear Tobias, OK for trunk with one slight quibble: I would rather see + /* Generate the function call. */ + if (code-resolved_isym-id == GFC_ISYM_CO_MAX) +fndecl = gfor_fndecl_co_max; + else if (code-resolved_isym-id == GFC_ISYM_CO_MIN) +fndecl = gfor_fndecl_co_min; + else if (code-resolved_isym-id == GFC_ISYM_CO_SUM) + fndecl = gfor_fndecl_co_sum; + else + gcc_unreachable (); + but your version is obviously functionally OK. Thanks for the patch Paul
Re: [Fortran, Patch] Some prep patches for coarray communication
Dear Tobias, This one is fine for trunk. Thanks for the patch. Paul
Re: [Patch, Fortran] Fix an issue with CLASS and -fcoarray=lib on the trunk
Dear Tobias, On 27 April 2014 20:56, Tobias Burnus bur...@net-b.de wrote: First, I would be really delighted if someone could review my coarray patches for the trunk as it makes simpler to develop patches on top of it: * http://gcc.gnu.org/ml/fortran/2014-04/msg00087.html This is OK for trunk. * http://gcc.gnu.org/ml/fortran/2014-04/msg00091.html dg2final Surely this is a typo? Although I note that getting it wrong on a German keyboard should produce a 4. Therefore it might well be a command that i do not know about. Otherwise, OK for trunk. * http://gcc.gnu.org/ml/fortran/2014-04/msg00092.html This is OK for trunk Secondly, attached is a patch which fixes an ICE - and prepares for some additional class-related coarray patches. In particular, the patch ensures that for nonallocatable *polymorphic* coarrays, the coarray token and offset are passed. This is also OK for trunk. Build and regtested on x86-64-gnu-linux. OK for the trunk? Tobias PS: There is still something wrong (for both -fcoarray=single and -fcoarray=lib) with lcobound/ucobounds and polymorphic coarrays and with using them with select type and associated. That's something I would like to tackle next. If that's done, I probably should really concentrate on reviewing a few patches and doing some other bug fixes before continue working on coarrays. Thank you truly for the effort that you are putting into co-arrays. It is highly gratifying that gfortran seems to perform so well compared with another product. Frankly, if I were you I would continue working on co-arrays, whilst you have the wind in your sails :-) Best regards Paul -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, Fortran] PR60881 - fix ICE with allocatable scalar coarrays
Dear Tobias, As you say, this of a rather obvious nature and is OK for trunk. Cheers Paul On 21 April 2014 22:52, Tobias Burnus bur...@net-b.de wrote: Dear all, for a change, a patch for the trunk and not for the fortran-caf branch. The following is a rather obvious patch which fixes the ICE. Built and regtested on x86-64-gnu-linux. OK for the trunk? As it is of rather obvious nature, I will commit it to the trunk in the next days unless there are objections. Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: Fwd: [Patch, fortran] PR60717 - Wrong code with recursive procedure with unlimited polymorphic dummy argument
Dear All, I have upgraded this patch slightly to fix PR58085 as well. I would judge this to be completely safe because the fixes depend on the new bit flag for both PRs. Bootstrapped and regtested on FC17/x86_64 - OK for 4.9 immediately and trunk? Paul 2014-04-12 Paul Thomas pa...@gcc.gnu.org PR fortran/60717 PR fortran/58085 * trans.h: Add 'use_offset' bitfield to gfc_se. * trans-array.c (gfc_conv_expr_descriptor): Use 'use_offset' as a trigger to unconditionally recalculate the offset for array slices and constant arrays. trans-expr.c (gfc_conv_intrinsic_to_class): Use it. trans-stmt.c (trans_associate_var): Ditto. (gfc_conv_procedure_call): Ditto. 2014-04-12 Paul Thomas pa...@gcc.gnu.org PR fortran/60717 * gfortran.dg/unlimited_polymorphic_17.f90: New test. PR fortran/58085 * gfortran.dg/associate_15.f90: New test. On 12 April 2014 10:30, Jakub Jelinek ja...@redhat.com wrote: On Sat, Apr 12, 2014 at 07:27:00AM +0200, Paul Richard Thomas wrote: I know that you are probably snowed under with requests like this! I was away on a business trip when Mikael's approval below came and had intended to apply it to 4.9 aka trunk today. Is it OK with you if I slip it into 4.9 or should I let it go until after the release? I'd prefer to put it in after the release, especially for non-regressions or regressions that aren't regressions from 4.8.2. 4.9.1 will be probably 2 months away from 4.9.0, and many people use release branch snapshots anyway, but I'd prefer to avoid any risks of slipping the release further. Jakub -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy Index: gcc/fortran/trans-array.c === *** gcc/fortran/trans-array.c (revision 209322) --- gcc/fortran/trans-array.c (working copy) *** gfc_conv_expr_descriptor (gfc_se *se, gf *** 6807,6814 /* Set offset for assignments to pointer only to zero if it is not the full array. */ ! if (se-direct_byref ! info-ref info-ref-u.ar.type != AR_FULL) base = gfc_index_zero_node; else if (GFC_ARRAY_TYPE_P (TREE_TYPE (desc))) base = gfc_evaluate_now (gfc_conv_array_offset (desc), loop.pre); --- 6807,6815 /* Set offset for assignments to pointer only to zero if it is not the full array. */ ! if ((se-direct_byref || se-use_offset) ! ((info-ref info-ref-u.ar.type != AR_FULL) ! || (expr-expr_type == EXPR_ARRAY se-use_offset))) base = gfc_index_zero_node; else if (GFC_ARRAY_TYPE_P (TREE_TYPE (desc))) base = gfc_evaluate_now (gfc_conv_array_offset (desc), loop.pre); *** gfc_conv_expr_descriptor (gfc_se *se, gf *** 6893,6905 stride, info-stride[n]); if (se-direct_byref ! info-ref ! info-ref-u.ar.type != AR_FULL) { base = fold_build2_loc (input_location, MINUS_EXPR, TREE_TYPE (base), base, stride); } ! else if (GFC_ARRAY_TYPE_P (TREE_TYPE (desc))) { tmp = gfc_conv_array_lbound (desc, n); tmp = fold_build2_loc (input_location, MINUS_EXPR, --- 6894,6906 stride, info-stride[n]); if (se-direct_byref ! ((info-ref info-ref-u.ar.type != AR_FULL) ! || (expr-expr_type == EXPR_ARRAY se-use_offset))) { base = fold_build2_loc (input_location, MINUS_EXPR, TREE_TYPE (base), base, stride); } ! else if (GFC_ARRAY_TYPE_P (TREE_TYPE (desc)) || se-use_offset) { tmp = gfc_conv_array_lbound (desc, n); tmp = fold_build2_loc (input_location, MINUS_EXPR, *** gfc_conv_expr_descriptor (gfc_se *se, gf *** 6935,6942 gfc_get_dataptr_offset (loop.pre, parm, desc, offset, subref_array_target, expr); ! if ((se-direct_byref || GFC_ARRAY_TYPE_P (TREE_TYPE (desc))) !se-data_not_needed) { /* Set the offset. */ gfc_conv_descriptor_offset_set (loop.pre, parm, base); --- 6936,6944 gfc_get_dataptr_offset (loop.pre, parm, desc, offset, subref_array_target, expr); ! if (((se-direct_byref || GFC_ARRAY_TYPE_P (TREE_TYPE (desc))) !se-data_not_needed) + || (se-use_offset base != NULL_TREE)) { /* Set the offset. */ gfc_conv_descriptor_offset_set (loop.pre, parm, base); Index: gcc/fortran/trans-expr.c === *** gcc/fortran/trans-expr.c(revision 209322
[Patch, fortran] PR60717 - Wrong code with recursive procedure with unlimited polymorphic dummy argument
Dear All, This fix, of itself, is quite obvious. The offset was being set to zero for array segments, rather than that required for unity valued lvalues. I think that the fix could be used to clean up: trans-expr.c(gfc_trans_alloc_subarray_assign) trans-expr.c(gfc_trans_pointer_assign) trans-expr.c(fncall_realloc_result) trans-array.c(trans_associate_var) each of which contains calculation of the offset. However, I do not think that this is the stage to fix things that are not broken! I propose to keep the PR open as a reminder to look into this. Bootstrapped and regtested on X86_64/FC17 - OK for trunk and backporting to 4.8? Paul 2014-04-12 Paul Thomas pa...@gcc.gnu.org PR fortran/58771 * trans.h : Add 'use_offset' bitfield to gfc_se. * trans-array.c (gfc_conv_expr_descriptor) : Use 'use_offset' as a trigger to unconditionally recalculate the offset. trans-expr.c (gfc_conv_intrinsic_to_class) : Use it. (gfc_conv_procedure_call) : Ditto. 2014-04-02 Paul Thomas pa...@gcc.gnu.org PR fortran/58771 * gfortran.dg/unlimited_polymorphic_17.f90 : New test Index: gcc/fortran/trans-array.c === *** gcc/fortran/trans-array.c (revision 208997) --- gcc/fortran/trans-array.c (working copy) *** gfc_conv_expr_descriptor (gfc_se *se, gf *** 6807,6813 /* Set offset for assignments to pointer only to zero if it is not the full array. */ ! if (se-direct_byref info-ref info-ref-u.ar.type != AR_FULL) base = gfc_index_zero_node; else if (GFC_ARRAY_TYPE_P (TREE_TYPE (desc))) --- 6807,6813 /* Set offset for assignments to pointer only to zero if it is not the full array. */ ! if ((se-direct_byref || se-use_offset) info-ref info-ref-u.ar.type != AR_FULL) base = gfc_index_zero_node; else if (GFC_ARRAY_TYPE_P (TREE_TYPE (desc))) *** gfc_conv_expr_descriptor (gfc_se *se, gf *** 6899,6905 base = fold_build2_loc (input_location, MINUS_EXPR, TREE_TYPE (base), base, stride); } ! else if (GFC_ARRAY_TYPE_P (TREE_TYPE (desc))) { tmp = gfc_conv_array_lbound (desc, n); tmp = fold_build2_loc (input_location, MINUS_EXPR, --- 6899,6905 base = fold_build2_loc (input_location, MINUS_EXPR, TREE_TYPE (base), base, stride); } ! else if (GFC_ARRAY_TYPE_P (TREE_TYPE (desc)) || se-use_offset) { tmp = gfc_conv_array_lbound (desc, n); tmp = fold_build2_loc (input_location, MINUS_EXPR, *** gfc_conv_expr_descriptor (gfc_se *se, gf *** 6935,6942 gfc_get_dataptr_offset (loop.pre, parm, desc, offset, subref_array_target, expr); ! if ((se-direct_byref || GFC_ARRAY_TYPE_P (TREE_TYPE (desc))) !se-data_not_needed) { /* Set the offset. */ gfc_conv_descriptor_offset_set (loop.pre, parm, base); --- 6935,6943 gfc_get_dataptr_offset (loop.pre, parm, desc, offset, subref_array_target, expr); ! if (((se-direct_byref || GFC_ARRAY_TYPE_P (TREE_TYPE (desc))) !se-data_not_needed) + || (se-use_offset base != NULL_TREE)) { /* Set the offset. */ gfc_conv_descriptor_offset_set (loop.pre, parm, base); Index: gcc/fortran/trans-expr.c === *** gcc/fortran/trans-expr.c(revision 208997) --- gcc/fortran/trans-expr.c(working copy) *** gfc_conv_intrinsic_to_class (gfc_se *par *** 593,598 --- 593,599 else { parmse-ss = ss; + parmse-use_offset = 1; gfc_conv_expr_descriptor (parmse, e); gfc_add_modify (parmse-pre, ctree, parmse-expr); } *** gfc_conv_procedure_call (gfc_se * se, gf *** 4378,4383 --- 4379,4385 || CLASS_DATA (fsym)-attr.codimension)) { /* Pass a class array. */ + parmse.use_offset = 1; gfc_conv_expr_descriptor (parmse, e); /* If an ALLOCATABLE dummy argument has INTENT(OUT) and is Index: gcc/fortran/trans.h === *** gcc/fortran/trans.h (revision 208997) --- gcc/fortran/trans.h (working copy) *** typedef struct gfc_se *** 87,92 --- 87,96 args alias. */ unsigned force_tmp:1; + / * Unconditionally calculate offset for array segments in + gfc_conv_expr_descriptor. */ + unsigned use_offset:1; + unsigned want_coarray:1; /* Scalarization parameters. */ Index:
Re: [Patch, Fortran] PR60576 Fix out-of-bounds problem
Dear Tobias, This is, of course, fine since it is 'obvious' (in my opinion at least). Thanks for the patch Paul On 27 March 2014 21:05, Tobias Burnus bur...@net-b.de wrote: An early * PING* for this wrong-code issue. Tobias Burnus wrote: This patch fixes part of the problems of the PR. The problem is that one assigns an array descriptor to an assumed-rank array descriptor. The latter has for BT_CLASS the size of max_dim (reason: we have first the data array and than vtab). With true, one takes the TREE_TYPE from the LHS (i.e. the assumed-rank variable) and as the type determines how many bytes the range assignment copies, one reads max_dimension elements from the RHS array - which can be too much. Testcase: Already in the testsuite, even if it only fails under special conditions. Build and regtested on x86-64-gnu-linux. OK for the trunk and 4.8? Tobias PS: I haven't investigated the issues Jakub is seeing. With valgrind, they do not pop up and my attempt to build with all checking enabled, failed with configure or compile errors. -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, Fortran] PRs 60283/60543: Fix two wrong-code bugs related for implicit pure
Dear Tobias, The patch looks OK to me. If nothing else, it offers a rationalisation of all the lines of code that unset the attribute! I am somewhat puzzled by Note: I failed to create a test case, wheras I find one at the end of the patch. Can you explain what you mean? Cheers Paul On 19 March 2014 21:21, Tobias Burnus bur...@net-b.de wrote: Early *ping* - I think this wrong-code GCC 4.7/4.8/4.9 issue is pretty severe. Tobias Burnus wrote: This patch fixes two issues, where gfortran claims that a function is implicit pure, but it is not. That will cause a wrong-code optimization in the middle end. First problem, cf. PR60543, is that implicit pure was not set to 0 for calls to impure intrinsic subroutines. (BTW: There are no impure intrinsic functions.) Example: module m contains REAL(8) FUNCTION random() CALL RANDOM_NUMBER(random) END FUNCTION random end module m The second problem pops up if one adds a BLOCK ... END BLOCK around the random_number call after applying the patch of the PR, which just does: gfc_current_ns-proc_name-attr.implicit_pure = 0. The problem is that one sets only the implicit_pure of the block to 0 and not of the function. That's the reason that the patch became much longer and that I added gfc_unset_implicit_pure as new function. Thus, the suspicion I had when reviewing the OpenACC patches turned out to be founded. Cf. PR60283. Build and regtested on x86-64-gnu-linux. OK for the trunk and for the 4.7 and 4.8 branches? Note: I failed to create a test case. Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
[PATCH] __builtin_expect with alternate predictors for Fortran (PR ipa/58721)
Dear All, The fortran part looks fine to me. Thanks for the patch,gents Paul
[Patch, fortran] PR59198 - [4.7/4.8/4.9 Regression] ICE on cyclically dependent polymorphic types
Dear All, The attached patch allows the original testcase to compile but attempting to use it runs into segfaults at runtime. The initializer for 'decay_t' has a bad FIELD_DECL for 'decay_gen_t' since it lacks the DECL_SIZE(_UNIT) fields but the TYPE_SIZE(_UNIT) fields are OK. The patch runs through the constructor and restores the missing fields, thus preventing the ICE in varasm.c. Taking the reduced testcase of comment #4 and adding a main program: module decays implicit none interface real elemental function iface (arg) real, intent(in) :: arg end function end interface type :: decay_term_t type(decay_t), pointer :: unstable_product end type type :: decay_gen_t procedure(iface), nopass, pointer :: obs1_int type(decay_term_t), allocatable :: term end type type :: rng_t integer :: i end type type, extends (decay_gen_t) :: decay_t class(rng_t), allocatable :: rng end type class(decay_t), allocatable :: object end use decays type(decay_t), pointer :: template allocate (template) allocate (template%rng) template%obs1_int = cos print *, template%obs1_int (3.14159/2.) allocate (object, source = template) ! print *, object%obs1_int (3.14159/2.) ! This should be OK but it segfaults at runtime. object%obs1_int = sin print *, object%obs1_int (3.14159/2.) end This runs correctly. However, the commented out line causes a segfault in runtime. I haven't attempted yet to understand why this should be the case but it is obvious that the _copy procedure for decay_t has something wrong with it. The patch, therefore is neither ready in principle to be committed and uncouvers other nasties. I have two questions that I hope that somebody can answer: (i) How or why is it possible for build_constructor to produce incomplete FIELD_DECLS?; and (ii) What is wrong with the _copy procedure? I cannot return to the fray for slightly more than a week. Best regards Paul Index: gcc/fortran/trans-expr.c === *** gcc/fortran/trans-expr.c(revision 208048) --- gcc/fortran/trans-expr.c(working copy) *** gfc_conv_structure (gfc_se * se, gfc_exp *** 6175,6180 --- 6175,6201 se-expr = build_constructor (type, v); if (init) TREE_CONSTANT (se-expr) = 1; + + /* Verify that the DECL_SIZE fields have been set so that the + constructor is ready for varasm.c(output_constructor_regular_field). + TODO: Find out why this is very occasionally necessary. + See PR59198. */ + type = TREE_TYPE (se-expr); + if (TREE_CODE (type) == RECORD_TYPE) + { + tmp = TYPE_FIELDS (type); + while (tmp) + { + if (TREE_CODE (tmp) == FIELD_DECL + DECL_SIZE_UNIT (tmp) == NULL_TREE) + { + DECL_SIZE (tmp) = TYPE_SIZE (TREE_TYPE (tmp)); + DECL_SIZE_UNIT (tmp) = TYPE_SIZE_UNIT (TREE_TYPE (tmp)); + } + tmp = TREE_CHAIN (tmp); + } + } + }
Re: [Patch, Fortran] PR602864 - fix INQUIRE for write= with stdout/stdin/stderr
Dear Uros, Tobias's patch is designed to be future-proof! Cheers Paul On 20 February 2014 14:26, Uros Bizjak ubiz...@gmail.com wrote: Hello! A rather simple patch. Build and regtested on x86-64-gnu-linux. OK for the trunk? 2014-02-20 Tobias Burnus bur...@net-b.de PR fortran/602864 ... we are not there yet with PR numbers ;) Uros. -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, fortran] PR51976 - [F2003] Support deferred-length character components of derived types (allocatable string length)
Dear Janus, I had completely forgotten about this patch... I even thought that it had been applied :-) I'll have time, either tomorrow evening or Saturday to take a look. After nearly 11 months, a couple more days will not hurt! Thanks for bringing it to my attention. Paul On 19 February 2014 16:51, Janus Weil ja...@gcc.gnu.org wrote: The patch was not applying cleanly any more, so here is a re-diffed version for current trunk. It works nicely on the included test case as well as the one provided by Walter Spector in comment 12 of the PR. Since, also in the current state, character(:) works only in a subset of all cases, I think it cannot hurt to add more cases that work for 4.9 (even if still not all possible cases work). Please let me know what you think ... Cheers, Janus 2014-02-19 16:16 GMT+01:00 Janus Weil ja...@gcc.gnu.org: Hi all, the patch below has been posted a long time ago, but was never actually committed (although it seems close to being finished). Could it still be considered for trunk? I think it is a rather popular feature, which would be helpful for many users ... Cheers, Janus 2013-03-19 22:17 GMT+01:00 Tobias Burnus bur...@net-b.de: Dear Paul, dear all, On February 24, 2013 Paul Richard Thomas wrote: The attached patch represents progress to date. It fixes the original problem in this PR and allows John Reid's version of iso_varying_string/vocabulary_word_count.f90 to compile and run correctly. It even bootstraps and regtests! Attached is a re-diffed patch; I have additionally fixed some indenting issues. Additionally, I have tested the patch - and it fails with deferred-length *array* character components. See attached test case. Also, the following line of the included test case leaks memory: allocate (array(2), source = [t(abcedefg,hi), t(jkl,mnop)]) I think at least the array bug should be fixed prior committal. (Fixing the memory leak and some of the below-mentioned issues would be nice, too.) Otherwise, I think the patch looks fine. For completeness, I have some naming remarks, which I would also like to considered: http://thread.gmane.org/gmane.comp.gcc.fortran/40393/focus=281580 Tobias However, it doe not fix: PR51976 comment #6 and PR51550 - allocate with typespec ICEs PR51976 comment #6 FORALL assignment is messed up and ICEs.. PR47545 the compiler complains about the lack of an initializer for the hidden character length field. PR45170 will need going through from one end to the other - there is a lot of stuff here! Of these, I consider the fix of the PR47545 problem to be a must and the allocate with typespec desirable. -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, Fortran, Regression] PR 55907: ICE with -fno-automatic -finit-local-zero
Dear Janus, This is OK for trunk, 4.8 and 4.7. Thanks for the patch. Paul On 16 February 2014 23:11, Janus Weil ja...@gcc.gnu.org wrote: Hi all, here is a small patch for a ICE-on-valid regression. Regtested on x86_64-unknown-linux-gnu. Ok for trunk/4.8/4.7? Cheers, Janus 2014-02-16 Janus Weil ja...@gcc.gnu.org PR fortran/55907 * resolve.c (build_default_init_expr): Don't initialize character variable if -fno-automatic is given. 2014-02-16 Janus Weil ja...@gcc.gnu.org PR fortran/55907 * gfortran.dg/init_flag_12.f90: New. -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, fortran] PR59026 - ELEMENTAL procedure with VALUE arguments emits wrong code
Dear All, Committed as revision 207645 - thanks for review and RM OK. Cheers Paul On 9 February 2014 13:55, Richard Biener rguent...@suse.de wrote: On February 8, 2014 6:48:08 PM GMT+01:00, Paul Richard Thomas paul.richard.tho...@gmail.com wrote: Dear All, This is another corner of a corner case that is not a regression but fixes a wrong-code-on-valid. At present, an actual argument of an elemental procedure with the VALUE attribute is not passed by value. The fix is obvious. Bootstraps and regtests on FC17/x86_64 - OK for trunk, 4.8 and 4.7? Ok from a RM perspective. Richard. Cheers Paul 2014-02-08 Paul Thomas pa...@gcc.gnu.org PR fortran/59026 * trans-expr.c (gfc_conv_procedure_call): Pass the value of the actual argument to a formal argument with the value attribute in an elemental procedure. 2014-02-08 Paul Thomas pa...@gcc.gnu.org PR fortran/59026 * gfortran.dg/elemental_by_value_1.f90 : New test -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, fortran] PR57522 - [F03] ASSOCIATE construct creates array descriptor with incorrect stride for derived type array component
Dear All, Committed as revision 207646. Thanks for the review and RM OK. Cheers Paul On 9 February 2014 13:56, Richard Biener rguent...@suse.de wrote: On February 8, 2014 5:08:52 PM GMT+01:00, Paul Richard Thomas paul.richard.tho...@gmail.com wrote: Dear All, I am aware that we are in stage 4 but this wrong-code-on-valid PR is confined to a corner of a corner and so I am certain that it can be safely applied - OK, Richie? Ok with me. Richard. This patch follows the same route as has been used for pointer array assignment to components of derived type arrays by rolling out the auxilliary 'span' variable for the associate-name to give it the correct extent. Once the array descriptor reform has been completed, this fix should be removed. All such code can be found by grepping on 'subref_array', so the excision will be swift and painless :-) Note that this is not needed for SELECT_TYPE since it is inaccessible there: component to the right of an array reference may not have ALLOCATABLE or POINTER attribute class component must be ALLOCATABLE or POINTER = false for any selector that I have been able to construct. Bootstraps and regtests on FC17/x86_64 - OK for trunk and 4.8? Cheers Paul 2014-02-08 Paul Thomas pa...@gcc.gnu.org PR fortran/57522 * resolve.c (resolve_assoc_var): Set the subref_array_pointer attribute for the 'associate-name' if necessary. * trans-stmt.c (trans_associate_var): If the 'associate-name' is a subref_array_pointer, assign the element size of the associate variable to 'span'. 2014-02-08 Paul Thomas pa...@gcc.gnu.org PR fortran/57522 * gfortran.dg/associated_target_5.f03 : New test -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
[Patch, fortran] PR57522 - [F03] ASSOCIATE construct creates array descriptor with incorrect stride for derived type array component
Dear All, I am aware that we are in stage 4 but this wrong-code-on-valid PR is confined to a corner of a corner and so I am certain that it can be safely applied - OK, Richie? This patch follows the same route as has been used for pointer array assignment to components of derived type arrays by rolling out the auxilliary 'span' variable for the associate-name to give it the correct extent. Once the array descriptor reform has been completed, this fix should be removed. All such code can be found by grepping on 'subref_array', so the excision will be swift and painless :-) Note that this is not needed for SELECT_TYPE since it is inaccessible there: component to the right of an array reference may not have ALLOCATABLE or POINTER attribute class component must be ALLOCATABLE or POINTER = false for any selector that I have been able to construct. Bootstraps and regtests on FC17/x86_64 - OK for trunk and 4.8? Cheers Paul 2014-02-08 Paul Thomas pa...@gcc.gnu.org PR fortran/57522 * resolve.c (resolve_assoc_var): Set the subref_array_pointer attribute for the 'associate-name' if necessary. * trans-stmt.c (trans_associate_var): If the 'associate-name' is a subref_array_pointer, assign the element size of the associate variable to 'span'. 2014-02-08 Paul Thomas pa...@gcc.gnu.org PR fortran/57522 * gfortran.dg/associated_target_5.f03 : New test Index: gcc/fortran/resolve.c === *** gcc/fortran/resolve.c (revision 207612) --- gcc/fortran/resolve.c (working copy) *** resolve_assoc_var (gfc_symbol* sym, bool *** 8269,8274 --- 8269,8276 sym-attr.target = tsym-attr.target || gfc_expr_attr (target).pointer; + if (is_subref_array (target)) + sym-attr.subref_array_pointer = 1; } /* Get type if this was not already set. Note that it can be Index: gcc/fortran/trans-stmt.c === *** gcc/fortran/trans-stmt.c(revision 207612) --- gcc/fortran/trans-stmt.c(working copy) *** trans_associate_var (gfc_symbol *sym, gf *** 1190,1195 --- 1190,1206 dim, gfc_index_one_node); } + /* If this is a subreference array pointer associate name use the +associate variable element size for the value of 'span'. */ + if (sym-attr.subref_array_pointer) + { + gcc_assert (e-expr_type == EXPR_VARIABLE); + tmp = e-symtree-n.sym-backend_decl; + tmp = gfc_get_element_type (TREE_TYPE (tmp)); + tmp = fold_convert (gfc_array_index_type, size_in_bytes (tmp)); + gfc_add_modify (se.pre, GFC_DECL_SPAN(desc), tmp); + } + /* Done, register stuff as init / cleanup code. */ gfc_add_init_cleanup (block, gfc_finish_block (se.pre), gfc_finish_block (se.post)); Index: gcc/testsuite/gfortran.dg/associated_target_5.f03 === *** gcc/testsuite/gfortran.dg/associated_target_5.f03 (revision 0) --- gcc/testsuite/gfortran.dg/associated_target_5.f03 (working copy) *** *** 0 --- 1,32 + ! { dg-do run } + ! Test the fix for PR57522, in which the associate name had a + ! 'span' of an INTEGER rather than that of 'mytype'. + ! + ! Contributed by A Briolat alan.brio...@gmail.com + ! + program test_associate + type mytype + integer :: a, b + end type + type(mytype) :: t(4) + integer :: c(4) + t%a = [0, 1, 2, 3] + t%b = [4, 5, 6, 7] + associate (a = t%a) + ! Test 'a' is OK on lhs and/or rhs of assignments + c = a - 1 + if (any (c .ne. [-1,0,1,2])) call abort + a = a + 1 + if (any (a .ne. [1,2,3,4])) call abort + a = t%b + if (any (a .ne. t%b)) call abort + ! Test 'a' is OK as an actual argument + c = foo(a) + if (any (c .ne. t%b + 10)) call abort + end associate + contains + function foo(arg) result(res) + integer :: arg(4), res(4) + res = arg + 10 + end function + end program
[Patch, fortran] PR59026 - ELEMENTAL procedure with VALUE arguments emits wrong code
Dear All, This is another corner of a corner case that is not a regression but fixes a wrong-code-on-valid. At present, an actual argument of an elemental procedure with the VALUE attribute is not passed by value. The fix is obvious. Bootstraps and regtests on FC17/x86_64 - OK for trunk, 4.8 and 4.7? Cheers Paul 2014-02-08 Paul Thomas pa...@gcc.gnu.org PR fortran/59026 * trans-expr.c (gfc_conv_procedure_call): Pass the value of the actual argument to a formal argument with the value attribute in an elemental procedure. 2014-02-08 Paul Thomas pa...@gcc.gnu.org PR fortran/59026 * gfortran.dg/elemental_by_value_1.f90 : New test Index: gcc/fortran/trans-expr.c === *** gcc/fortran/trans-expr.c(revision 205031) --- gcc/fortran/trans-expr.c(working copy) *** gfc_conv_procedure_call (gfc_se * se, gf *** 4050,4056 --- 4050,4060 gfc_init_se (parmse, se); parm_kind = ELEMENTAL; + if (fsym fsym-attr.value) + gfc_conv_expr (parmse, e); + else gfc_conv_expr_reference (parmse, e); + if (e-ts.type == BT_CHARACTER !e-rank e-expr_type == EXPR_FUNCTION) parmse.expr = build_fold_indirect_ref_loc (input_location, Index: gcc/testsuite/gfortran.dg/elemental_by_value_1.f90 === *** gcc/testsuite/gfortran.dg/elemental_by_value_1.f90 (revision 0) --- gcc/testsuite/gfortran.dg/elemental_by_value_1.f90 (working copy) *** *** 0 --- 1,22 + ! { dg-do run } + ! + ! PR fortran/59026 + ! + ! Contributed by F-X Coudert fxcoud...@gcc.gnu.org + ! + ! Failed to dereference the argument in scalarized loop. + ! + elemental integer function foo(x) + integer, value :: x + foo = x + 1 + end function + + interface + elemental integer function foo(x) + integer, value :: x + end function + end interface + + if (foo(42) .ne. 43) call abort + if (any (foo([0,1]) .ne. [1,2])) call abort + end
Re: [Bug fortran/60066] Bad elemental invocation of non-scalar base object
Dear All, I propose to add the attached to the testsuite. It is the testcase from PR60066, which was fixed by the patch for PR59066. OK for trunk, 4.8 and 4.7? On 5 February 2014 12:38, pault at gcc dot gnu.org gcc-bugzi...@gcc.gnu.org wrote: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60066 Paul Thomas pault at gcc dot gnu.org changed: What|Removed |Added Resolution|DUPLICATE |FIXED --- Comment #8 from Paul Thomas pault at gcc dot gnu.org --- (In reply to Dominique d'Humieres from comment #5) I have applied the patch at http://gcc.gnu.org/ml/fortran/2014-02/txtX3eVILZEGw.txt on top of 4.8.3 r206497 and the test runs successfully ... Marking as duplicate of pr49906. Paul, For the record, no regression when testing with make -k -j8 check-gfortran RUNTESTFLAGS=--target_board=unix'{-m32,-m64}' *** This bug has been marked as a duplicate of bug 49906 *** I will, however, add this testcase to that of PR59906 - it is different yet again from the verification tests although it is fixed by the patch. Cheers Pau -- You are receiving this mail because: You are on the CC list for the bug. -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy Index: gcc/testsuite/gfortran.dg/elemental_subroutine_10.f90 === *** gcc/testsuite/gfortran.dg/elemental_subroutine_10.f90 (revision 0) --- gcc/testsuite/gfortran.dg/elemental_subroutine_10.f90 (working copy) *** *** 0 --- 1,68 + ! { dg-do run } + ! + ! PR fortran/60066 + ! + ! Contributed by F Martinez Fadrique fmarti...@gmv.com + ! + ! Fixed by the patch for PR59906 but adds another, different test. + ! + module m_assertion_character + implicit none + type :: t_assertion_character + character(len=8) :: name + contains + procedure :: assertion_character + procedure :: write = assertion_array_write + end type t_assertion_character + contains + elemental subroutine assertion_character( ast, name ) + class(t_assertion_character), intent(out) :: ast + character(len=*), intent(in) :: name + ast%name = name + end subroutine assertion_character + subroutine assertion_array_write( ast, unit ) + class(t_assertion_character), intent(in) :: ast + character(*), intent(inOUT) :: unit + write(unit,*) trim (unit(2:len(unit)))//trim (ast%name) + end subroutine assertion_array_write + end module m_assertion_character + + module m_assertion_array_character + use m_assertion_character + implicit none + type :: t_assertion_array_character + type(t_assertion_character), dimension(:), allocatable :: rast + contains + procedure :: assertion_array_character + procedure :: write = assertion_array_character_write + end type t_assertion_array_character + contains + pure subroutine assertion_array_character( ast, name, nast ) + class(t_assertion_array_character), intent(out) :: ast + character(len=*), intent(in) :: name + integer, intent(in) :: nast + integer :: i + allocate ( ast%rast(nast) ) + call ast%rast%assertion_character ( name ) + end subroutine assertion_array_character + subroutine assertion_array_character_write( ast, unit ) + class(t_assertion_array_character), intent(in) :: ast + CHARACTER(*), intent(inOUT) :: unit + integer :: i + do i = 1, size (ast%rast) + call ast%rast(i)%write (unit) + end do + end subroutine assertion_array_character_write + end module m_assertion_array_character + + program main + use m_assertion_array_character + implicit none + type(t_assertion_array_character) :: ast + character(len=8) :: name + character (26) :: line = '' + name = 'test' + call ast%assertion_array_character ( name, 5 ) + call ast%write (line) + if (line(2:len (line)) .ne. testtesttesttesttest) call abort + end program main
[Patch, fortran] PR49906 [4.7/4.8/4.9 Regression] error: size of variable 'anonymous' is too large
Dear All, This regression was flagged by Harald and the trigger, r158683, was identified by HJ. Many thanks to both. It surprises me that the bug has lain dormant for so long. The fix is fortunately relatively simple. CHARACTER scalars are, in fact arrays in one shape or form and so using them as an SS_REFERENCE is bound to fail unless a pointer to the array is stored in the outer loop and passed to the ELEMENTAL subroutine/function. gfc_conv_string_parameter is equipped to convert CHARACTERs in all their manifestations into a pointer, so I have used this. It does not work correctly for function results, so this case has been excluded. Not only does this patch bootstrap and regtest on FC17/x86_64 but all the tests in ISO_VARYING_STRING run correctly. This latter is important because the suite makes heavy use of elemental functions with character arguments. OK for trunk and, with a decent delay, 4.7 and 4.8? Cheers Paul 2014-02-01 Paul Thomas pa...@gcc.gnu.org PR fortran/59906 * trans-stmt.c (gfc_add_loop_ss_code): In the case of character SS_REFERENCE, use gfc_conv_string_parameter to ensure that a pointer to the string is stored. * trans-expr.c (gfc_conv_expr_reference): Likewise, use gfc_conv_string_parameter to ensure that a pointer to is passed to the elemental function. 2014-02-01 Paul Thomas pa...@gcc.gnu.org PR fortran/59906 * gfortran.dg/elemental_subroutine_9.f90 : New test Index: gcc/fortran/trans-array.c === *** gcc/fortran/trans-array.c (revision 207203) --- gcc/fortran/trans-array.c (working copy) *** gfc_add_loop_ss_code (gfc_loopinfo * loo *** 2491,2496 --- 2491,2501 a reference to the value. */ gfc_conv_expr (se, expr); } + + /* Ensure that a pointer to the string is stored. */ + if (expr-ts.type == BT_CHARACTER) + gfc_conv_string_parameter (se); + gfc_add_block_to_block (outer_loop-pre, se.pre); gfc_add_block_to_block (outer_loop-post, se.post); if (gfc_is_class_scalar_expr (expr)) Index: gcc/fortran/trans-expr.c === *** gcc/fortran/trans-expr.c(revision 207203) --- gcc/fortran/trans-expr.c(working copy) *** gfc_conv_expr_reference (gfc_se * se, gf *** 6350,6356 --- 6350,6362 /* Returns a reference to the scalar evaluated outside the loop for this case. */ gfc_conv_expr (se, expr); + + if (expr-ts.type == BT_CHARACTER + expr-expr_type != EXPR_FUNCTION) + gfc_conv_string_parameter (se); + else se-expr = gfc_build_addr_expr (NULL_TREE, se-expr); + return; } Index: gcc/testsuite/gfortran.dg/elemental_subroutine_9.f90 === *** gcc/testsuite/gfortran.dg/elemental_subroutine_9.f90(revision 0) --- gcc/testsuite/gfortran.dg/elemental_subroutine_9.f90(working copy) *** *** 0 --- 1,39 + ! { dg-do run } + ! + ! PR fortran/59906 + ! + ! Contributed by H Anlauf anl...@gmx.de + ! + ! Failed generate character scalar for scalarized loop for elemantal call. + ! + program x + implicit none + call y('bbb') + contains + + subroutine y(str) + character(len=*), intent(in) :: str + character(len=len_trim(str)) :: str_aux + character(len=3) :: str3 = 'abc' + + str_aux = str + + ! Compiled but did not give correct result + if (any (str_cmp((/'aaa','bbb'/), str) .neqv. [.FALSE.,.TRUE.])) call abort + + ! Did not compile + if (any (str_cmp((/'bbb', 'aaa'/), str_aux) .neqv. [.TRUE.,.FALSE.])) call abort + + ! Verify patch + if (any (str_cmp((/'bbb', 'aaa'/), str3) .neqv. [.FALSE.,.FALSE.])) call abort + if (any (str_cmp((/'bbb', 'aaa'/), 'aaa') .neqv. [.FALSE.,.TRUE.])) call abort + + end subroutine y + + elemental logical function str_cmp(str1, str2) + character(len=*), intent(in) :: str1 + character(len=*), intent(in) :: str2 + str_cmp = (str1 == str2) + end function str_cmp + + end program x
Re: [Patch, fortran] PR49906 [4.7/4.8/4.9 Regression] error: size of variable 'anonymous' is too large
Mikael, Je t'en remercie! That must be one of the fastest reviews on record! Committed as revision 207389 4.7 and 4.8 to follow next weekend. Paul On 1 February 2014 19:28, Mikael Morin mikael.mo...@sfr.fr wrote: Le 01/02/2014 18:57, Paul Richard Thomas a écrit : Dear All, This regression was flagged by Harald and the trigger, r158683, was identified by HJ. Many thanks to both. It surprises me that the bug has lain dormant for so long. The fix is fortunately relatively simple. CHARACTER scalars are, in fact arrays in one shape or form and so using them as an SS_REFERENCE is bound to fail unless a pointer to the array is stored in the outer loop and passed to the ELEMENTAL subroutine/function. gfc_conv_string_parameter is equipped to convert CHARACTERs in all their manifestations into a pointer, so I have used this. It does not work correctly for function results, so this case has been excluded. Not only does this patch bootstrap and regtest on FC17/x86_64 but all the tests in ISO_VARYING_STRING run correctly. This latter is important because the suite makes heavy use of elemental functions with character arguments. OK for trunk and, with a decent delay, 4.7 and 4.8? Yes, thanks. Mikael -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, fortran] PR57033 ICE on extended derived type and default initialization
Dear Mikael, We seem to be breaking records tonight :-) OK by me for trunk, 4.7 and 4.8. Thanks! Paul On 1 February 2014 19:49, Mikael Morin mikael.mo...@sfr.fr wrote: Hello, here is a fix for PR57033. The problem was gfc_convert_to_structure_constructor calling itself recursively and changing `actual' behind its back without going through the loop condition. The fix is pretty obvious; I thought there was something missing (see the PR) but on second thought, it seems to be correct. regression tested on x86_64-unknown-linux-gnu; OK for trunk and then 4.8/4.7? Mikael -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, fortran] PR59414 [4.8/4.9 Regression] [OOP] ICE in in gfc_conv_expr_descriptor on ALLOCATE inside SELECT TYPE
Dear Janus, snip well, actually the selector in the test case is not an array. And polymorphic arrays as such have been supported since 4.7. duuh! Shows how time flies when you're having fun! Bootstrapped and regtested on FC17/x86_64 - OK for trunk and, after a decent delay, 4.8? Yes, looks good to me. Thanks for the patch! Thanks for the review. PS I know of at least one other place where this manoeuvre had to be done. If I find a third, I will turn it into a function in class.c. It might be worth doing anyway? I tend to agree. Up to you ... I am debugging such a function but, for reasons that I have not yet found, it recovers the original ICE :-( Elsewhere, I have implemented the same removal of references in the TREE_SSA representation for which there is a helper function. I must check if that is not possible here. Cheers Paul Cheers, Janus 2014-01-20 Paul Thomas pa...@gcc.gnu.org PR fortran/59414 * trans-stmt.c (gfc_trans_allocate): Before the pointer assignment to transfer the source _vptr to a class allocate expression, the final class reference should be exposed. The tail that includes the _data and array references is stored. This reduced expression is transferred to 'lhs' and the _vptr added. Then the tail is restored to the allocate expression. 2014-01-20 Paul Thomas pa...@gcc.gnu.org PR fortran/59414 * gfortran.dg/allocate_class_3.f90 : New test -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy