Re: [PATCH] PR fortran/89943 -- Duplicate BIND(c) allowed (?)
Hi Steve, In the F2018 standard: C1550 (R1526) If MODULE appears in the prefix of a module subprogram and a binding label is specified, it shall be the same as the binding label specified in the corresponding module procedure interface body. While it does not say explicitly that a repeat binding label is allowed, I think that the implication is clear enough. The patch is OK as it is but it would be nice if C1550 is or would be implemented. Thanks Paul On Fri, 11 Oct 2019 at 19:31, Steve Kargl wrote: > > PING. > > On Fri, Oct 04, 2019 at 03:26:53PM -0700, Steve Kargl wrote: > > The attached patch allows the declaration of a BIND(C) > > module function or module subroutine to appear in a > > submodule (see testcases). Regression test was clean. > > OK to commit? > > > > Before a rubber stamped 'OK'. I do NOT use submodules. > > A submodule user needs to pipe up on the validity of > > the patch. > > > > > > 2019-10-04 Steven G. Kargl > > > > PR fortran/89943 > > decl.c (gfc_match_function_decl): Ignore duplicate BIND(C) for > > function > > declaration in submodule. > > (gfc_match_entry): Use temporary for locus, which allows removal of > > one gfc_error_now(). > > (gfc_match_subroutine): Ignore duplicate BIND(C) for subroutine > > declaration in submodule. > > > > 2019-10-04 Steven G. Kargl > > > > PR fortran/89943 > > * gfortran.dg/pr89943_1.f90: New test. > > * gfortran.dg/pr89943_2.f90: Ditto. > > > > -- > > Steve > > > Index: gcc/fortran/decl.c > > === > > --- gcc/fortran/decl.c(revision 276601) > > +++ gcc/fortran/decl.c(working copy) > > @@ -7259,13 +7259,16 @@ gfc_match_function_decl (void) > >if (sym->attr.is_bind_c == 1) > > { > >sym->attr.is_bind_c = 0; > > - if (sym->old_symbol != NULL) > > -gfc_error_now ("BIND(C) attribute at %L can only be used for " > > - "variables or common blocks", > > - &(sym->old_symbol->declared_at)); > > - else > > -gfc_error_now ("BIND(C) attribute at %L can only be used for " > > - "variables or common blocks", &gfc_current_locus); > > + > > + if (gfc_state_stack->previous > > + && gfc_state_stack->previous->state != COMP_SUBMODULE) > > + { > > + locus loc; > > + loc = sym->old_symbol != NULL > > + ? sym->old_symbol->declared_at : gfc_current_locus; > > + gfc_error_now ("BIND(C) attribute at %L can only be used for " > > + "variables or common blocks", &loc); > > + } > > } > > > >if (found_match != MATCH_YES) > > @@ -7517,16 +7520,16 @@ gfc_match_entry (void) > > not allowed for procedures. */ > >if (entry->attr.is_bind_c == 1) > > { > > + locus loc; > > + > >entry->attr.is_bind_c = 0; > > - if (entry->old_symbol != NULL) > > -gfc_error_now ("BIND(C) attribute at %L can only be used for " > > - "variables or common blocks", > > - &(entry->old_symbol->declared_at)); > > - else > > -gfc_error_now ("BIND(C) attribute at %L can only be used for " > > - "variables or common blocks", &gfc_current_locus); > > -} > > > > + loc = entry->old_symbol != NULL > > + ? entry->old_symbol->declared_at : gfc_current_locus; > > + gfc_error_now ("BIND(C) attribute at %L can only be used for " > > + "variables or common blocks", &loc); > > + } > > + > >/* Check what next non-whitespace character is so we can tell if there > > is the required parens if we have a BIND(C). */ > >old_loc = gfc_current_locus; > > @@ -7725,13 +7728,16 @@ gfc_match_subroutine (void) > >if (sym->attr.is_bind_c == 1) > > { > >sym->attr.is_bind_c = 0; > > - if (sym->old_symbol != NULL) > > -gfc_error_now ("BIND(C) attribute at %L can only be used for " > > - "variables or common blocks", > > - &(sym->old_symbol->declared_at)); > > - else > > -gfc_error_now ("BIND(C) attribute at %L can only be used for " > > - "variables or common blocks", &gfc_current_locus); > > + > > + if (gfc_state_stack->previous > > + && gfc_state_stack->previous->state != COMP_SUBMODULE) > > + { > > + locus loc; > > + loc = sym->old_symbol != NULL > > + ? sym->old_symbol->declared_at : gfc_current_locus; > > + gfc_error_now ("BIND(C) attribute at %L can only be used for " > > + "variables or common blocks", &loc); > > + } > > } > > > >/* C binding names are not allowed for internal procedures. */ > > Index: gcc/testsuite/gfortran.dg/pr89943_1.f90 > > === > > --- gcc/testsuite/gfortran.dg/pr89
Re: [PATCH] PR fortran/90297 -- Remove code with no functional effect
Hi Steve, As I remarked on the PR thread, it is one of the less harmful bits of code that I introduced :-) OK to commit. Thanks Paul On Sat, 12 Oct 2019 at 01:17, Steve Kargl wrote: > > The patch is fairly self-explanatory. OK to commit? > > 2019-10-11 Steven G. Kargl > > PR fortran/90297 > * resolve.c (resolve_typebound_function): Remove code with no > functional effect. > > -- > Steve -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran] PR91926 - assumed rank optional
Hi Christophe, Thanks for flagging this up - I am back at base on Saturday and will take it up then. Regards Paul On Wed, 9 Oct 2019 at 11:13, Christophe Lyon wrote: > > Hi, > > > On Sat, 5 Oct 2019 at 20:31, Paul Richard Thomas > wrote: >> >> I must apologise not posting this before committing. I left for a >> vacation this morning and I thought that this problem and the one >> posted by Gilles were best fixed before departing. The patch only >> touches the new ISO_Fortran binding feature and so I thought that I >> would be safe to do this. >> >> It was fully regtested and only applies to trunk. >> >> Paul >> >> Author: pault >> Date: Sat Oct 5 08:17:55 2019 >> New Revision: 276624 >> >> URL: https://gcc.gnu.org/viewcvs?rev=276624&root=gcc&view=rev >> Log: >> 2019-10-05 Paul Thomas >> >> PR fortran/91926 >> * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Correct the >> assignment of the attribute field to account correctly for an >> assumed shape dummy. Assign separately to the gfc and cfi >> descriptors since the atribute can be different. Add btanch to >> correctly handle missing optional dummies. >> >> 2019-10-05 Paul Thomas >> >> PR fortran/91926 >> * gfortran.dg/ISO_Fortran_binding_13.f90 : New test. >> * gfortran.dg/ISO_Fortran_binding_13.c : Additional source. >> * gfortran.dg/ISO_Fortran_binding_14.f90 : New test. >> >> 2019-10-05 Paul Thomas >> >> PR fortran/91926 >> * runtime/ISO_Fortran_binding.c (cfi_desc_to_gfc_desc): Do not >> modify the bounds and offset for CFI_other. >> >> Added: >> trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.c >> trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.f90 >> trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_14.f90 >> Modified: >> trunk/gcc/fortran/ChangeLog >> trunk/gcc/fortran/trans-expr.c >> trunk/gcc/testsuite/ChangeLog >> trunk/libgfortran/ChangeLog >> trunk/libgfortran/runtime/ISO_Fortran_binding.c > > > > Since this was committed (r276624), I have noticed regressions on > arm-linux-gnueabihf: > FAIL: gfortran.dg/ISO_Fortran_binding_11.f90 -O3 -g execution test > I've seen other reports on gcc-testresults too. > > Christophe > -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
[Patch, fortran] PR91926 - assumed rank optional
I must apologise not posting this before committing. I left for a vacation this morning and I thought that this problem and the one posted by Gilles were best fixed before departing. The patch only touches the new ISO_Fortran binding feature and so I thought that I would be safe to do this. It was fully regtested and only applies to trunk. Paul Author: pault Date: Sat Oct 5 08:17:55 2019 New Revision: 276624 URL: https://gcc.gnu.org/viewcvs?rev=276624&root=gcc&view=rev Log: 2019-10-05 Paul Thomas PR fortran/91926 * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Correct the assignment of the attribute field to account correctly for an assumed shape dummy. Assign separately to the gfc and cfi descriptors since the atribute can be different. Add btanch to correctly handle missing optional dummies. 2019-10-05 Paul Thomas PR fortran/91926 * gfortran.dg/ISO_Fortran_binding_13.f90 : New test. * gfortran.dg/ISO_Fortran_binding_13.c : Additional source. * gfortran.dg/ISO_Fortran_binding_14.f90 : New test. 2019-10-05 Paul Thomas PR fortran/91926 * runtime/ISO_Fortran_binding.c (cfi_desc_to_gfc_desc): Do not modify the bounds and offset for CFI_other. Added: trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.c trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.f90 trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_14.f90 Modified: trunk/gcc/fortran/ChangeLog trunk/gcc/fortran/trans-expr.c trunk/gcc/testsuite/ChangeLog trunk/libgfortran/ChangeLog trunk/libgfortran/runtime/ISO_Fortran_binding.c
[Patch, fortran] PR91729 - ICE in gfc_match_select_rank, at fortran/match.c:6586
Fixed as obvious in revision: 276051. The patch is largely due to Steve, for which thanks. Paul 2019-09-23 Paul Thomas PR fortran/91729 * match.c (gfc_match_select_rank): Initialise 'as' to NULL. Check for a symtree in the selector expression before trying to assign a value to 'as'. Revert to gfc_error and go to cleanup after setting a MATCH_ERROR. 2019-09-23 Paul Thomas PR fortran/91729 * gfortran.dg/select_rank_2.f90 : Add two more errors in foo2. * gfortran.dg/select_rank_3.f90 : New test. -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
[Patch, fortran] PR91726 - [7/8/9/10 Regression] ICE in gfc_conv_array_ref, at fortran/trans-array.c:3612
Fixing the original problem in the module took a few minutes. Making the module do something useful took rather longer! The testcase in the patch compiles with 6-branch but segfaults in runtime. Bootstrapped and regtested on FC30/x86_64 - OK to commit and go steadily back through the branches over some weeks? Regards Paul 2019-09-22 Paul Thomas PR fortran/91726 * resolve.c (gfc_expr_to_initialize): Bail out with a copy of the original expression if the array ref is a scalar and the array_spec has corank. * trans-array.c (gfc_conv_array_ref): Such expressions are OK even if the array ref codimen is zero. * trans-expr.c (gfc_get_class_from_expr): New function taken from gfc_get_vptr_from_expr. (gfc_get_vptr_from_expr): Call new function. * trans-stmt.c (trans_associate_var): If one of these is a target expression, extract the class expression from the target and copy its fields to a new target variable. * trans.h : Add prototype for gfc_get_class_from_expr. 2019-09-22 Paul Thomas PR fortran/91726 * gfortran.dg/coarray_poly_9.f90 : New test. Index: gcc/fortran/resolve.c === *** gcc/fortran/resolve.c (revision 275799) --- gcc/fortran/resolve.c (working copy) *** gfc_expr_to_initialize (gfc_expr *e) *** 7433,7438 --- 7433,7442 for (ref = result->ref; ref; ref = ref->next) if (ref->type == REF_ARRAY && ref->next == NULL) { + if (ref->u.ar.dimen == 0 + && ref->u.ar.as && ref->u.ar.as->corank) + return result; + ref->u.ar.type = AR_FULL; for (i = 0; i < ref->u.ar.dimen; i++) Index: gcc/fortran/trans-array.c === *** gcc/fortran/trans-array.c (revision 275799) --- gcc/fortran/trans-array.c (working copy) *** gfc_conv_array_ref (gfc_se * se, gfc_arr *** 3609,3615 if (ar->dimen == 0) { ! gcc_assert (ar->codimen || sym->attr.select_rank_temporary); if (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (se->expr))) se->expr = build_fold_indirect_ref (gfc_conv_array_data (se->expr)); --- 3609,3616 if (ar->dimen == 0) { ! gcc_assert (ar->codimen || sym->attr.select_rank_temporary ! || (ar->as && ar->as->corank)); if (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (se->expr))) se->expr = build_fold_indirect_ref (gfc_conv_array_data (se->expr)); Index: gcc/fortran/trans-expr.c === *** gcc/fortran/trans-expr.c (revision 275799) --- gcc/fortran/trans-expr.c (working copy) *** gfc_reset_len (stmtblock_t *block, gfc_e *** 472,482 } ! /* Obtain the vptr of the last class reference in an expression. Return NULL_TREE if no class reference is found. */ tree ! gfc_get_vptr_from_expr (tree expr) { tree tmp; tree type; --- 472,482 } ! /* Obtain the last class reference in an expression. Return NULL_TREE if no class reference is found. */ tree ! gfc_get_class_from_expr (tree expr) { tree tmp; tree type; *** gfc_get_vptr_from_expr (tree expr) *** 487,493 while (type) { if (GFC_CLASS_TYPE_P (type)) ! return gfc_class_vptr_get (tmp); if (type != TYPE_CANONICAL (type)) type = TYPE_CANONICAL (type); else --- 487,493 while (type) { if (GFC_CLASS_TYPE_P (type)) ! return tmp; if (type != TYPE_CANONICAL (type)) type = TYPE_CANONICAL (type); else *** gfc_get_vptr_from_expr (tree expr) *** 501,506 --- 501,523 tmp = build_fold_indirect_ref_loc (input_location, tmp); if (GFC_CLASS_TYPE_P (TREE_TYPE (tmp))) + return tmp; + + return NULL_TREE; + } + + + /* Obtain the vptr of the last class reference in an expression. +Return NULL_TREE if no class reference is found. */ + + tree + gfc_get_vptr_from_expr (tree expr) + { + tree tmp; + + tmp = gfc_get_class_from_expr (expr); + + if (tmp != NULL_TREE) return gfc_class_vptr_get (tmp); return NULL_TREE; Index: gcc/fortran/trans-stmt.c === *** gcc/fortran/trans-stmt.c (revision 275799) --- gcc/fortran/trans-stmt.c (working copy) *** trans_associate_var (gfc_symbol *sym, gf *** 2099,2105 --- 2099,2141 } else { + tree ctree = gfc_get_class_from_expr (se.expr); tmp = TREE_TYPE (sym->backend_decl); + + /* Coarray scalar component expressions can emerge from + the front end as array elements of the _data field. */ + if (sym->ts.type == BT_CLASS + && e->ts.type == BT_CLASS && e->rank == 0 + && !GFC_CLASS_TYPE_P (TREE_TYPE (se.expr)) && ctree) + { + tree stmp; + tree dtmp; + + se.expr = ctree; + dtmp = TREE_TYPE (TREE_TYPE (sym->backend_decl)); + ctree =
Re: [Patch, Fortran] CO_BROADCAST for derived types with allocatable components
Hi Sandro, This patch looks fine to me. I have a question about the comment: "This code is obviously added because the finalizer is not trusted to free all memory." 'obviously'? Not to me :-( Maybe you can expand on this? As to the stat and errmsg: Leave them for the time being. However, an attempt will have to be made to implement F2018 "16.6 Collective subroutines". I don't know enough about the coarrays implementation to be able to help you about detecting these conditions. Maybe Tobias Burnus can help? OK to commit. Paul PS Sometime before very long, something will have to be done about the exponential code bloat that structure_alloc_comps. The more users that there are for it the tougher it is going to get! On Thu, 22 Aug 2019 at 18:41, Alessandro Fanfarillo wrote: > > Dear all, > please find in attachment a preliminary patch that adds support to > co_broadcast for allocatable components of derived types. > The patch is currently ignoring the stat and errmsg arguments, mostly > because I am not sure how to handle them properly. I have created a > new data structure called used to pass those argument to the > preexisting structure_alloc_comps. > Suggestions on how to handle them are more than welcome :-) > > The patch builds correctly on x86_64 and it has been tested with > OpenCoarrays and the following test cases: > > https://github.com/sourceryinstitute/OpenCoarrays/blob/co_broadcast-derived-type/src/tests/unit/collectives/co_broadcast_allocatable_components.f90 > > https://github.com/sourceryinstitute/OpenCoarrays/blob/co_broadcast-derived-type/src/tests/unit/collectives/co_broadcast_allocatable_components_array.f90 > > Regards, -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
[Patch, fortran] PR91588 - ICE in check_inquiry, at fortran/expr.c:2673
The attached bootstraps and regtests on FC30/x86_64 - OK for trunk? It strikes me that this should be backported since the bug is rather fundamental and ispresent all the way back to version 4.8. An obvious question is how far back? To 8-branch? Cheers Paul 2019-09-15 Paul Thomas PR fortran/91588 * expr.c (check_inquiry): Remove extended component refs by using symbol pointers. If a function argument is an associate variable with a constant target, copy the target expression in place of the argument expression. Check that the charlen is not NULL before using the string length. (gfc_check_assign): Remove extraneous space. 2019-09-15 Paul Thomas PR fortran/91588 * gfortran.dg/associate_49.f90 : New test. Index: gcc/fortran/expr.c === *** gcc/fortran/expr.c (revision 275695) --- gcc/fortran/expr.c (working copy) *** check_inquiry (gfc_expr *e, int not_rest *** 2610,2615 --- 2610,2617 int i = 0; gfc_actual_arglist *ap; + gfc_symbol *sym; + gfc_symbol *asym; if (!e->value.function.isym || !e->value.function.isym->inquiry) *** check_inquiry (gfc_expr *e, int not_rest *** 2619,2638 if (e->symtree == NULL) return MATCH_NO; ! if (e->symtree->n.sym->from_intmod) { ! if (e->symtree->n.sym->from_intmod == INTMOD_ISO_FORTRAN_ENV ! && e->symtree->n.sym->intmod_sym_id != ISOFORTRAN_COMPILER_OPTIONS ! && e->symtree->n.sym->intmod_sym_id != ISOFORTRAN_COMPILER_VERSION) return MATCH_NO; ! if (e->symtree->n.sym->from_intmod == INTMOD_ISO_C_BINDING ! && e->symtree->n.sym->intmod_sym_id != ISOCBINDING_C_SIZEOF) return MATCH_NO; } else { ! name = e->symtree->n.sym->name; functions = inquiry_func_gnu; if (gfc_option.warn_std & GFC_STD_F2003) --- 2621,2642 if (e->symtree == NULL) return MATCH_NO; ! sym = e->symtree->n.sym; ! ! if (sym->from_intmod) { ! if (sym->from_intmod == INTMOD_ISO_FORTRAN_ENV ! && sym->intmod_sym_id != ISOFORTRAN_COMPILER_OPTIONS ! && sym->intmod_sym_id != ISOFORTRAN_COMPILER_VERSION) return MATCH_NO; ! if (sym->from_intmod == INTMOD_ISO_C_BINDING ! && sym->intmod_sym_id != ISOCBINDING_C_SIZEOF) return MATCH_NO; } else { ! name = sym->name; functions = inquiry_func_gnu; if (gfc_option.warn_std & GFC_STD_F2003) *** check_inquiry (gfc_expr *e, int not_rest *** 2657,2697 if (!ap->expr) continue; if (ap->expr->ts.type == BT_UNKNOWN) { ! if (ap->expr->symtree->n.sym->ts.type == BT_UNKNOWN ! && !gfc_set_default_type (ap->expr->symtree->n.sym, 0, gfc_current_ns)) return MATCH_NO; ! ap->expr->ts = ap->expr->symtree->n.sym->ts; } ! /* Assumed character length will not reduce to a constant expression ! with LEN, as required by the standard. */ ! if (i == 5 && not_restricted && ap->expr->symtree ! && ap->expr->symtree->n.sym->ts.type == BT_CHARACTER ! && (ap->expr->symtree->n.sym->ts.u.cl->length == NULL ! || ap->expr->symtree->n.sym->ts.deferred)) ! { ! gfc_error ("Assumed or deferred character length variable %qs " ! "in constant expression at %L", ! ap->expr->symtree->n.sym->name, ! &ap->expr->where); ! return MATCH_ERROR; ! } ! else if (not_restricted && !gfc_check_init_expr (ap->expr)) ! return MATCH_ERROR; ! if (not_restricted == 0 ! && ap->expr->expr_type != EXPR_VARIABLE ! && !check_restricted (ap->expr)) return MATCH_ERROR; ! if (not_restricted == 0 ! && ap->expr->expr_type == EXPR_VARIABLE ! && ap->expr->symtree->n.sym->attr.dummy ! && ap->expr->symtree->n.sym->attr.optional) ! return MATCH_NO; } return MATCH_YES; --- 2661,2708 if (!ap->expr) continue; + asym = ap->expr->symtree ? ap->expr->symtree->n.sym : NULL; + if (ap->expr->ts.type == BT_UNKNOWN) { ! if (asym && asym->ts.type == BT_UNKNOWN ! && !gfc_set_default_type (asym, 0, gfc_current_ns)) return MATCH_NO; ! ap->expr->ts = asym->ts; } ! if (asym && asym->assoc && asym->assoc->target ! && asym->assoc->target->expr_type == EXPR_CONSTANT) ! { ! gfc_free_expr (ap->expr); ! ap->expr = gfc_copy_expr (asym->assoc->target); ! } ! /* Assumed character length will not reduce to a constant expression ! with LEN, as required by the standard. */ ! if (i == 5 && not_restricted && asym ! && asym->ts.type == BT_CHARACTER ! && ((asym->ts.u.cl && asym->ts.u.cl->length == NULL) ! || asym->ts.deferred)) ! { ! gfc_error ("Assumed or deferred character length variable %qs " ! "in constant expression at %L", ! asym->name, &ap->expr->where); return MATCH_ERROR; + } + else if (not_restricte
Re: [Patch, fortran] PR91717 - ICE on concatenating deferred-length character and character literal
Thanks - committed as r275696. Paul On Thu, 12 Sep 2019 at 15:09, Steve Kargl wrote: > > On Thu, Sep 12, 2019 at 09:55:20AM +0100, Paul Richard Thomas wrote: > > > > OK to commit? > > > > Yes. > > -- > Steve -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
[Patch, fortran] PR91686 - ICE in gimplify:2554
Committed as 'obvious' in revision 275681: 2019-09-12 Paul Thomas PR fortran/91686 Backport from mainline * trans-expr.c (gfc_trans_assignment_1): Copy and paste section handling the rse.pre block from mainline. 2019-09-12 Paul Thomas PR fortran/91686 * gfortran.dg/pr91686.f90 : New test. The rse.pre block was being added outside the loop so that a temporary index was being used before it was declared and the loop index was not defined. Paul
Re: [Patch, fortran] PR91717 - ICE on concatenating deferred-length character and character literal
Hi Steve, I have inserted a my_core%msg = '"" so that it is initialised. The reallocation on assignment explicitly deals with an unallocated entity or one of its allocatable components by allocation, rather than reallocation. I realise that my explanation of the patch might be a bit sparse. The ultimate caller is frontend_passes.c(realloc_string_callback), which looks for overlap even for scalar cases. The ICE came about because there are no array references in the expressions being compared. Flagging that there are identical component chains in this case and returning 1 from gfc_dep_resolver covers this case. OK to commit? Cheers Paul realloc_string_callback) On Thu, 12 Sep 2019 at 00:05, Steve Kargl wrote: > > On Wed, Sep 11, 2019 at 11:27:56PM +0100, Paul Richard Thomas wrote: > > Hi Steve, > > > > Being an allocatable component, this code appears on entry into scope: > > > > my_core.msg = 0B; > > my_core._msg_length = 0; > > { > > > > Thus, according to the standard, my_core%msg is defined. > > > > I'll politely defer to the Fortran standard. > > 5.4.10 Allocatable variables > > The allocation status of an allocatable variable is either allocated > or unallocated. An allocatable variable becomes allocated as described > in 9.7.1.3. It becomes unallocated as described in 9.7.3.2. > > An unallocated allocatable variable shall not be referenced or defined. > > 7.5.4.6 Default initialization for components > > Allocatable components are always initialized to unallocated. > > In your testcase, you have an unallocatable allocatable entity > referenced on the RHS in the first line that involves my_core%msg. > From the Fortran standard's point of view, I believe the code is > nonconforming. > > type core > character (len=:), allocatable :: msg > end type > > type(core) :: my_core > > print *, allocated(my_core%msg) > > ! Comment out to avoid ICE. > ! my_core%msg on RHS is unallocated in next line. > ! my_core%msg = my_core%msg//"my message is: > ! my_core%msg = my_core%msg//"Hello!" > ! > ! if (my_core%msg .ne. "my message is: Hello!") stop 1 > end > > % gfcx -o z a.f90 && ./z > F > > > detected for this assignment even though no array references are > > involved. There is certainly the need for a temporary, which the patch > > generates. > > > > Your patch may fix the ICE, but if the code compiles and > execute. You are getting the "right" answer fortuitiouly. > > Of course, I could be wrong. > > -- > Steve -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran] PR91717 - ICE on concatenating deferred-length character and character literal
Hi Steve, Being an allocatable component, this code appears on entry into scope: my_core.msg = 0B; my_core._msg_length = 0; { Thus, according to the standard, my_core%msg is defined. If you follow the backtrace from the ICE, you will see that something like the patch is required. It is important that the dependency is detected for this assignment even though no array references are involved. There is certainly the need for a temporary, which the patch generates. Cheers Paul On Wed, 11 Sep 2019 at 06:50, Steve Kargl wrote: > > On Wed, Sep 11, 2019 at 06:44:50AM +0100, Paul Richard Thomas wrote: > > === > > *** gcc/testsuite/gfortran.dg/dependency_55.f90 (nonexistent) > > --- gcc/testsuite/gfortran.dg/dependency_55.f90 (working copy) > > *** > > *** 0 > > --- 1,17 > > + ! { dg-do run } > > + ! > > + ! Test the fix for PR91717 in which the concatenation operation ICEd. > > + ! > > + ! Contributed by Damian Rouson > > + ! > > + type core > > + character (len=:), allocatable :: msg > > + end type > > + > > + type(core) :: my_core > > + > > + my_core%msg = my_core%msg//"my message is: " > > my_core%msg is undefined on the RHS. This is invalid > Fortran. Not sure whether your patch is correct or not. > > -- > Steve -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
[Patch, fortran] PR91717 - ICE on concatenating deferred-length character and character literal
Hi All, I nearly committed this patch as 'obvious' but noticed a fair number of changes in 10-branch in dependency analysis. This is in fact a 10-regression. I'll wait for an OK. Bootstrapped and regtested on FC30/x86_64 - OK to commit? Paul 2019-09-11 Paul Thomas PR fortran/91717 * dependency.c (gfc_dep_resolver): Flag identical components and exit with return value 1 if set and no array refs. 2019-09-11 Paul Thomas PR fortran/91717 * gfortran.dg/dependency_55.f90 : New test. Index: gcc/fortran/dependency.c === *** gcc/fortran/dependency.c (revision 275323) --- gcc/fortran/dependency.c (working copy) *** gfc_dep_resolver (gfc_ref *lref, gfc_ref *** 2096,2101 --- 2096,2102 int m; gfc_dependency fin_dep; gfc_dependency this_dep; + bool same_component = false; this_dep = GFC_DEP_ERROR; fin_dep = GFC_DEP_ERROR; *** gfc_dep_resolver (gfc_ref *lref, gfc_ref *** 2115,2120 --- 2116,2123 components. */ if (lref->u.c.component != rref->u.c.component) return 0; + + same_component = true; break; case REF_SUBSTRING: *** gfc_dep_resolver (gfc_ref *lref, gfc_ref *** 2280,2285 --- 2283,2292 if (lref || rref) return 1; + /* This can result from concatenation of assumed length string components. */ + if (same_component && fin_dep == GFC_DEP_ERROR) + return 1; + /* If we haven't seen any array refs then something went wrong. */ gcc_assert (fin_dep != GFC_DEP_ERROR); Index: gcc/testsuite/gfortran.dg/dependency_55.f90 === *** gcc/testsuite/gfortran.dg/dependency_55.f90 (nonexistent) --- gcc/testsuite/gfortran.dg/dependency_55.f90 (working copy) *** *** 0 --- 1,17 + ! { dg-do run } + ! + ! Test the fix for PR91717 in which the concatenation operation ICEd. + ! + ! Contributed by Damian Rouson + ! + type core + character (len=:), allocatable :: msg + end type + + type(core) :: my_core + + my_core%msg = my_core%msg//"my message is: " + my_core%msg = my_core%msg//"Hello!" + + if (my_core%msg .ne. "my message is: Hello!") stop 1 + end
[Patch-fortran] PR91589 - [9/10 Regression] ICE in gfc_conv_component_ref, at fortran/trans-expr.c:2447
This was caused by my inquiry ref patch. I will commit to both branches tomorrow night unless there is an objection. Bootstraps and regtests on FC29/x86_64. Paul 2019-09-02 Paul Thomas PR fortran/91589 * primary.c (gfc_match_varspec): Return MATCH_NO on an apparent component ref, when the primary type is not derived, class or unknown. 2019-09-02 Paul Thomas PR fortran/91589 * gfortran.dg/pr91589.f90 : New test. Index: gcc/fortran/primary.c === *** gcc/fortran/primary.c (revision 275268) --- gcc/fortran/primary.c (working copy) *** gfc_match_varspec (gfc_expr *primary, in *** 2237,2242 --- 2237,2244 inquiry = is_inquiry_ref (name, &tmp); if (inquiry) sym = NULL; + else if (primary->ts.type != BT_UNKNOWN) + return MATCH_NO; } else inquiry = false; *** gfc_variable_attr (gfc_expr *expr, gfc_t *** 2598,2604 case AR_UNKNOWN: /* For standard conforming code, AR_UNKNOWN should not happen. ! For nonconforming code, gfortran can end up here. Treat it as a no-op. */ break; } --- 2600,2606 case AR_UNKNOWN: /* For standard conforming code, AR_UNKNOWN should not happen. ! For nonconforming code, gfortran can end up here. Treat it as a no-op. */ break; } Index: gcc/testsuite/gfortran.dg/pr91589.f90 === *** gcc/testsuite/gfortran.dg/pr91589.f90 (nonexistent) --- gcc/testsuite/gfortran.dg/pr91589.f90 (working copy) *** *** 0 --- 1,14 + ! { dg-do compile } + ! + ! Check the fix for PR91589, in which the invalid expression caused an ICE. + ! + ! Contributed by Gerhardt Steinmetz + ! + program p +type t + integer :: a +end type +type(t) :: x = t(1) +call sub (x%a%a) ! { dg-error "Syntax error in argument list" } + end +
Re: [PATCH] PR fortran/91552 -- Walk array constructor to do type conversion
Hi Steve, OK to commit. Thanks Paul On Sat, 31 Aug 2019 at 21:16, Steve Kargl wrote: > > The attached patch has been built and tested on i586-*-freebsd > and x86_64-*-freebsd. OK to commit? > > The patch fixes an ICE during type conversion where an array > constructor contains another array. The new function recursively > walks the constructor, and tries to do the type conversion. > The testcase demonstrates the recursion. > > 2019-08-31 Steven G. Kargl > > PR fortran/91552 > * array.c (walk_array_constructor): New function. > (gfc_match_array_constructor): Use it. > > 2019-08-31 Steven G. Kargl > > PR fortran/91552 > * gfortran.dg/pr91552.f90: New test. > > -- > Steve -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran] Implementation of F2018 SELECT RANK
Hi Steve, Thanks for the remarks and the OK. Committed as revision 275269. Onward to PDTs! Cheers Paul On Sat, 31 Aug 2019 at 18:46, Steve Kargl wrote: > > On Sat, Aug 31, 2019 at 04:59:03PM +0100, Paul Richard Thomas wrote: > > > > Bootstraps and regtests on FC29/x86_64 - OK for trunk? > > Well, I've made it through to the trans-* files. Reading > that part of the patch will take a awhile. I'm think you > should commit. > > -- > steve -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran] Implementation of F2018 SELECT RANK
Hi Steve, > > > If an error occurs, should this set m = MATCH_ERROR and goto cleanup? > > > OR, set m = MATCH_ERROR, free expr1 and expr2 and return m? > > > IOW, if an error occurs, why should gfortran continue to match select > > > rank? This prevents or reduces the deluge of errors that come from the cases and the case blocks. > > > > > > Still looking at the remaining part of patch. > > > > > > > There's another gfc_error_now several lines down. Does the > > same early return apply there as well. Same reason > > > > Also, found > > > > Index: gcc/fortran/parse.c > > === > > *** gcc/fortran/parse.c (revision 275242) > > --- gcc/fortran/parse.c (working copy) > > + > > + /* At this point, we're got a nonempty select block. */ > > > > s/we're/we've > > > > Copy-n-paste may corrupt this, but the change is removing > 1 trailing whitespace space and leaving a remaining trailing > whitespace. OK I'll check. Thanks Paul
[Patch, fortran] Implementation of F2018 SELECT RANK
When I started on this about three months ago, I thought that with SELECT TYPE as a template this was going to be low hanging fruit. Not so :-) Among the issues on the way were: class selectors; unlimited polymorphic selectors even more so; and error recovery. It's a good starting point but I am not holding my breath for it being PR-free. It will be noted that the select rank temporaries are also select type temporaries. This simplifies some of the logic, especially in resolution, and allows RANK DEFAULT to be handled in the same way as CLASS DEFAULT in trans-stmt.c. In the new function, trans-stmt.c(copy_descriptor), memcpy is used to copy-in/copy-out sections of the dim[] array between the case descriptor and the assumed rank selector. This avoids explicitly copying the bounds and strides or messing around trying to convert the types. There is maybe a better way to do this but I didn't find it. While I was about it, I made the code in some of the SELECT TYPE matching much more readable by using a gfc_symbol *selector = select_type_stack->selector. Bootstraps and regtests on FC29/x86_64 - OK for trunk? Now on to PDTs Paul 2019-08-31 Paul Thomas * array.c (spec_dimen_size): Check for the presence of expressions for the bounds. * decl.c (gfc_match_end): Add case COMP_SELECT_RANK. * dump-parse-tree.c(show_symbol): Show the arrayspec of class entities. (show_code_node): Show the code for SELECT_RANK. * expr.c (gfc_check_vardef_context): Omit the context of variable definition for select rank associate names since the ASSUMED RANK throws. * gfortran.h : Add ST_SELECT_RANK and ST_RANK to enum gfc_statement. Add select_rank_temporary to symbol attribute structure. Add EXEC_SELECT_RANK to enum gfc_exec_op. * match.c (match_exit_cycle): Add COMP_SELECT_RANK. (copy_ts_from_selector_to_associate): Add as special case for assumed rank class variables. (select_intrinsic_set_tmp): Clean up the code by using symbols for references to the temporary and the selector. (select_type_set_tmp): Ditto. (select_rank_set_tmp): New function. (gfc_match_select_rank): New function. (gfc_match_rank_is): New function. * match.h : Add prototypes for gfc_match_select_rank and gfc_match_rank_is. * parse.c (decode_statement): Attempt to match select_rank and rank statements. (next_statement, gfc_ascii_statement): Add ST_SELECT_RANK. (parse_select_rank_block): New function. (parse_executable): Parse select rank block for ST_SELECT_RANK. * parse.h : Add COMP_SELECT_RANK to enum gfc_compile_state. * resolve.c (resolve_variable): Exclude select_rank_temporaries from the check on use of ASSUMED RANK. (gfc_resolve_expr): Make sure that unlimited polymorphic select rank temporaries expressions are not resolved again after being successfully resolved. (resolve_assoc_var): Do not do the rank check for select rank temporaries. (resolve_select_rank): New function. (gfc_resolve_blocks): Deal with case EXEC_SELECT_RANK. (resolve_symbol): Exclude select rank temporaries for check on use of ASSUMED RANK. * st.c (gfc_free_statement): Include EXEC_SELECT_RANK. * trans-array.c (gfc_conv_array_ref): Select rank temporaries may have dimen == 0. (gfc_conv_expr_descriptor): Zero the offset of select rank temporaries. * trans-stmt.c (copy_descriptor): New function. (trans_associate_var): Add code to associate select rank temps. (gfc_trans_select_rank_cases): New function. (gfc_trans_select_rank): New function. * trans-stmt.h : Add prototype for gfc_trans_select_rank. trans.c (trans_code): Add select rank case. 2019-08-31 Paul Thomas * gfortran.dg/select_rank_1.f90 : New test. Index: gcc/fortran/array.c === *** gcc/fortran/array.c (revision 275242) --- gcc/fortran/array.c (working copy) *** spec_dimen_size (gfc_array_spec *as, int *** 2213,2219 gfc_internal_error ("spec_dimen_size(): Bad dimension"); if (as->type != AS_EXPLICIT ! || as->lower[dimen]->expr_type != EXPR_CONSTANT || as->upper[dimen]->expr_type != EXPR_CONSTANT || as->lower[dimen]->ts.type != BT_INTEGER || as->upper[dimen]->ts.type != BT_INTEGER) --- 2213,2223 gfc_internal_error ("spec_dimen_size(): Bad dimension"); if (as->type != AS_EXPLICIT ! || !as->lower[dimen] ! || !as->upper[dimen]) ! return false; ! ! if (as->lower[dimen]->expr_type != EXPR_CONSTANT || as->upper[dimen]->expr_type != EXPR_CONSTANT || as->lower[dimen]->ts.type != BT_INTEGER || as->upper[dimen]->ts.type != BT_INTEGER) Index: gcc/fortran/decl.c === *** gcc/fortran/decl.c (revision 275242) --- gcc/fortran/decl.c (working copy) *** gfc_match_end (gf
Re: [PR fortran/91496] !GCC$ directives error if mistyped or unknown
Hi Harald, This is OK for trunk. Thanks! Paul On Mon, 26 Aug 2019 at 22:13, Harald Anlauf wrote: > > Dear all, > > the attached patch adds Fortran support for the following pragmas > (loop annotations): IVDEP (ignore vector dependencies), VECTOR, and > NOVECTOR. Furthermore, it downgrades unsupported directives from > error to warning (by default, it stays an error with -pedantic), > thus fixing the PR. > > It has no effect on existing code (thus regtested cleanly on > x86_64-pc-linux-gnu), but gives users an option for fine-grained > control of optimization. The above pragmas are supported by other > compilers (with different sentinels, e.g. !DIR$ for Intel, Cray, > sometimes with slightly different keywords). > > OK for trunk, and backport to 9? > > Thanks, > Harald > > > 2019-08-26 Harald Anlauf > > PR fortran/91496 > * gfortran.h: Extend struct gfc_iterator for loop annotations. > * array.c (gfc_copy_iterator): Copy loop annotations by IVDEP, > VECTOR, and NOVECTOR pragmas. > * decl.c (gfc_match_gcc_ivdep, gfc_match_gcc_vector) > (gfc_match_gcc_novector): New matcher functions handling IVDEP, > VECTOR, and NOVECTOR pragmas. > * match.h: Declare prototypes of matcher functions handling IVDEP, > VECTOR, and NOVECTOR pragmas. > * parse.c (decode_gcc_attribute, parse_do_block) > (parse_executable): Decode IVDEP, VECTOR, and NOVECTOR pragmas; > emit warning for unrecognized pragmas instead of error. > * trans-stmt.c (gfc_trans_simple_do, gfc_trans_do): Add code to > emit annotations for IVDEP, VECTOR, and NOVECTOR pragmas. > * gfortran.texi: Document IVDEP, VECTOR, and NOVECTOR pragmas. > > 2019-08-26 Harald Anlauf > > PR fortran/91496 > * gfortran.dg/pr91496.f90: New testcase. > -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [patch, fortran] Fix PR 90561
Hi Thomas, Yes that's good to apply to 9 and 10 branches. I am certain that there are other places where the new function will be very helpful. Thanks for the patch. Paul On Tue, 13 Aug 2019 at 13:59, Thomas König wrote: > > Hello world, > > this patch fixes a 9/10 regression by placing the variable used to > hold a string length at function scope. > > I chose to implement this version of gfc_evaluate_now as a separate > function because I have a sneaking suspicion this may not be the > last time we are going to encounter something like that - better > have the function there for future use. > > Regression-tested. OK for trunk and gcc-9? > > Regards > > Thomas -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [PATCH] PR fortran/42546 -- ALLOCATED has 2 mutually exclusive keywords
Hi Steve, Who thought of that one in the standard? Uuugh! The solution looks good to commit - again as far back as you feel inclined to do. Regards Paul On Tue, 6 Aug 2019 at 19:27, Steve Kargl wrote: > > Ping. > > On Thu, Aug 01, 2019 at 02:11:39PM -0700, Steve Kargl wrote: > > The attached patch fixed the issues raised in the > > PR fortran/42546. Namely, ALLOCATED has two possible > > keywords: ALLOCATE(ARRAY=...) or ALLOCATED(SCALAR=...) > > > > In Tobias' original patch (attached to the PR), he > > tried to make both ARRAY and SCALAR options, then > > in gfc_check_allocated() appropriate checking was > > added. I started down that road, but intrinsic.c( > > sort_actual) got in the way. Fortunately, the > > checking for ARRAY or SCALAR can be special-cased > > in sort_actual. See the patch. > > > > Regression tested on x86_64-*-freebsd. OK to commit? > > > > 2019-08-01 Steven G. Kargl > > > > PR fortran/42546 > > * check.c(gfc_check_allocated): Add comment pointing to ... > > * intrinsic.c(sort_actual): ... the checking done here. > > > > 2019-08-01 Steven G. Kargl > > > > PR fortran/42546 > > * gfortran.dg/allocated_1.f90: New test. > > * gfortran.dg/allocated_2.f90: Ditto. > > > > -- > > Steve > > > Index: gcc/fortran/check.c > > === > > --- gcc/fortran/check.c (revision 273950) > > +++ gcc/fortran/check.c (working copy) > > @@ -1168,6 +1168,10 @@ gfc_check_all_any (gfc_expr *mask, gfc_expr *dim) > > } > > > > > > +/* Limited checking for ALLOCATED intrinsic. Additional checking > > + is performed in intrinsic.c(sort_actual), because ALLOCATED > > + has two mutually exclusive non-optional arguments. */ > > + > > bool > > gfc_check_allocated (gfc_expr *array) > > { > > Index: gcc/fortran/intrinsic.c > > === > > --- gcc/fortran/intrinsic.c (revision 273950) > > +++ gcc/fortran/intrinsic.c (working copy) > > @@ -4180,6 +4180,40 @@ sort_actual (const char *name, gfc_actual_arglist > > **ap > >if (f == NULL && a == NULL)/* No arguments */ > > return true; > > > > + /* ALLOCATED has two mutually exclusive keywords, but only one > > + can be present at time and neither is optional. */ > > + if (strcmp (name, "allocated") == 0 && a->name) > > +{ > > + if (strcmp (a->name, "scalar") == 0) > > + { > > + if (a->next) > > + goto whoops; > > + if (a->expr->rank != 0) > > + { > > + gfc_error ("Scalar entity required at %L", &a->expr->where); > > + return false; > > + } > > + return true; > > + } > > + else if (strcmp (a->name, "array") == 0) > > + { > > + if (a->next) > > + goto whoops; > > + if (a->expr->rank == 0) > > + { > > + gfc_error ("Array entity required at %L", &a->expr->where); > > + return false; > > + } > > + return true; > > + } > > + else > > + { > > + gfc_error ("Invalid keyword %qs in %qs intrinsic function at %L", > > + a->name, name, @a->expr->where); > > + return false; > > + } > > +} > > + > >for (;;) > > {/* Put the nonkeyword arguments in a 1:1 > > correspondence */ > >if (f == NULL) > > @@ -4199,6 +4233,7 @@ sort_actual (const char *name, gfc_actual_arglist **ap > >if (a == NULL) > > goto do_sort; > > > > +whoops: > >gfc_error ("Too many arguments in call to %qs at %L", name, where); > >return false; > > > > Index: gcc/testsuite/gfortran.dg/allocated_1.f90 > > === > > --- gcc/testsuite/gfortran.dg/allocated_1.f90 (nonexistent) > > +++ gcc/testsuite/gfortran.dg/allocated_1.f90 (working copy) > > @@ -0,0 +1,24 @@ > > +! { dg-do run } > > +program foo > > + > > + implicit none > > + > > + integer, allocatable :: x > > + integer, allocatable :: a(:) > > + > > + logical a1, a2 > > + > > + a1 = allocated(scalar=x) > > + if (a1 .neqv. .false.) stop 1 > > + a2 = allocated(array=a) > > + if (a2 .neqv. .false.) stop 2 > > + > > + allocate(x) > > + allocate(a(2)) > > + > > + a1 = allocated(scalar=x) > > + if (a1 .neqv. .true.) stop 3 > > + a2 = allocated(array=a) > > + if (a2 .neqv. .true.) stop 4 > > + > > +end program foo > > Index: gcc/testsuite/gfortran.dg/allocated_2.f90 > > === > > --- gcc/testsuite/gfortran.dg/allocated_2.f90 (nonexistent) > > +++ gcc/testsuite/gfortran.dg/allocated_2.f90 (working copy) > > @@ -0,0 +1,16 @@ > > +! { dg-do compile } > > +program foo > > + > > + implicit none > > + > > + integer, allocatable :: x > > + integer, allocatable :: a(:) > > + > > + logical a1, a2 > > + > > + a1 = allocated(scalar=a) ! { dg-error "Scalar
Re: [PATCH] PR fortran/91359 -- A function should return something
Hi Steve, That certainly does the trick! OK to commit as far back as you have the intestinal fortitude for. Thanks Paul On Tue, 6 Aug 2019 at 19:24, Steve Kargl wrote: > > The spaghetti code in PR fortran/91359 has a few gotos > to jump to different places. In doing so, gfortran > with generate a function that has a naked 'return'. > Namely, > > foo () > { > logical(kind=4) __result_foo; > > goto __label_02; > __label_01:; > return; /* <-- THIS IS BAD. */ > __label_02:; > __result_foo = 0; > if (!__result_foo) goto __label_01; > L.1:; > return __result_foo; > return __result_foo; > } > > The attached patch avoids the naked 'return' by > adding the variable that is constructed from the > the function name. Namely, > > foo () > { > logical(kind=4) __result_foo; > > goto __label_02; > __label_01:; > return __result_foo;/* <-- THIS IS GOOD. */ > __label_02:; > __result_foo = 0; > if (!__result_foo) goto __label_01; > L.1:; > return __result_foo; > return __result_foo; > } > > Regression tested on x86_64-*-freebsd. OK to commit? > > 2019-08-06 Steven G. Kargl > > PR fortran/91359 > * trans-decl.c (gfc_generate_return): Ensure something is returned > from a function. > > 2019-08-06 Steven G. Kargl > > PR fortran/91359 > * gfortran.dg/pr91359_1.f: New test. > * gfortran.dg/pr91359_2.f: Ditto. > > -- > Steve -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [PATCH] PR fortran/88227 -- Revenge of the BOZ
Hi Steve, This is OK. Thanks for working on it. Paul On Thu, 1 Aug 2019 at 22:13, Steve Kargl wrote: > > Ping. > > -- > steve > > On Sun, Jul 28, 2019 at 04:41:02PM -0700, Steve Kargl wrote: > > The attach patch fixes a problem with the conversion of a > > BOZ literal constant to a REAL where the size of the REAL > > exceeds the size of the largest INTEGER. The problem can > > be seen on 32-bit targets that provide support for REAL(10) > > and/or REAL(16), or it can be seen with a multilib target > > when using -m32 and REAL(10) and/or REAL(16). > > > > If needed, the patch converts an octal or hexidecimal string > > to the equivalent binary string, and then converts the binary > > string to a REAL. In principle, bin2real() can convert to > > REAL(4), REAL(8), REAL(10), and REAL(16), but I have elected > > to use the old conversion method if the size of the largest > > INTEGER exceeds the size the REAL(XXX) of interest. A future > > patch may remove the old method and make this new approach the > > only way to convert a BOZ. > > > > I have attached a short test program. There is no testcase > > for testsuite. > > > > PLEASE TEST. > > > > 2019-07-28 Steven G. Kargl > > > > PR fortran/88227 > > * check.c (oct2bin): New function. Convert octal string to binary. > > (hex2bin): New function. Convert hexidecimal string to binary. > > (bin2real): New function. Convert binary string to REAL. Use > > oct2bin and hex2bin. > > (gfc_boz2real): Use fallback conversion bin2real. > > > > -- > > Steve > > > Index: gcc/fortran/check.c > > === > > --- gcc/fortran/check.c (revision 273766) > > +++ gcc/fortran/check.c (working copy) > > @@ -55,6 +55,7 @@ gfc_invalid_boz (const char *msg, locus *loc) > > > > > > /* Issue an error for an illegal BOZ argument. */ > > + > > static bool > > illegal_boz_arg (gfc_expr *x) > > { > > @@ -101,6 +102,167 @@ is_boz_constant (gfc_expr *a) > > } > > > > > > +/* Convert a octal string into a binary string. This is used in the > > + fallback conversion of an octal string to a REAL. */ > > + > > +static char * > > +oct2bin(int nbits, char *oct) > > +{ > > + const char bits[8][5] = { > > +"000", "001", "010", "011", "100", "101", "110", "111"}; > > + > > + char *buf, *bufp; > > + int i, j, n; > > + > > + j = nbits + 1; > > + if (nbits == 64) j++; > > + > > + bufp = buf = XCNEWVEC (char, j + 1); > > + memset (bufp, 0, j + 1); > > + > > + n = strlen (oct); > > + for (i = 0; i < n; i++, oct++) > > +{ > > + j = *oct - 48; > > + strcpy (bufp, &bits[j][0]); > > + bufp += 3; > > +} > > + > > + bufp = XCNEWVEC (char, nbits + 1); > > + if (nbits == 64) > > +strcpy (bufp, buf + 2); > > + else > > +strcpy (bufp, buf + 1); > > + > > + free (buf); > > + > > + return bufp; > > +} > > + > > + > > +/* Convert a hexidecimal string into a binary string. This is used in the > > + fallback conversion of a hexidecimal string to a REAL. */ > > + > > +static char * > > +hex2bin(int nbits, char *hex) > > +{ > > + const char bits[16][5] = { > > +"", "0001", "0010", "0011", "0100", "0101", "0110", "0111", > > +"1000", "1001", "1010", "1011", "1100", "1101", "1110", ""}; > > + > > + char *buf, *bufp; > > + int i, j, n; > > + > > + bufp = buf = XCNEWVEC (char, nbits + 1); > > + memset (bufp, 0, nbits + 1); > > + > > + n = strlen (hex); > > + for (i = 0; i < n; i++, hex++) > > +{ > > + j = *hex; > > + if (j > 47 && j < 58) > > + j -= 48; > > + else if (j > 64 && j < 71) > > + j -= 55; > > + else if (j > 96 && j < 103) > > + j -= 87; > > + else > > + gcc_unreachable (); > > + > > + strcpy (bufp, &bits[j][0]); > > + bufp += 4; > > + } > > + > > + return buf; > > +} > > + > > + > > +/* Fallback conversion of a BOZ string to REAL. */ > > + > > +static void > > +bin2real (gfc_expr *x, int kind) > > +{ > > + char buf[114], *sp; > > + int b, i, ie, t, w; > > + bool sgn; > > + mpz_t em; > > + > > + i = gfc_validate_kind (BT_REAL, kind, false); > > + t = gfc_real_kinds[i].digits - 1; > > + > > + /* Number of bits in the exponent. */ > > + if (gfc_real_kinds[i].max_exponent == 16384) > > +w = 15; > > + else if (gfc_real_kinds[i].max_exponent == 1024) > > +w = 11; > > + else > > +w = 8; > > + > > + if (x->boz.rdx == 16) > > +sp = hex2bin (gfc_real_kinds[i].mode_precision, x->boz.str); > > + else if (x->boz.rdx == 8) > > +sp = oct2bin (gfc_real_kinds[i].mode_precision, x->boz.str); > > + else > > +sp = x->boz.str; > > + > > + /* Extract sign bit. */ > > + sgn = *sp != '0'; > > + > > + /* Extract biased exponent. */ > > + memset (buf, 0, 114); > > + strncpy (buf, ++sp, w); > > + mpz_init (em); > > + mpz_set_str (em, buf, 2); > > + ie = mpz_get_si (em); > > + > > + mpfr_init2 (x->value.real, t + 1); > >
Re: [patch, fortran] Fix PR 90813
Hi Thomas, That is very well done. Thanks for picking it up and running with it. OK on both the fix and the dumping of the gsymbols. You might consider back porting both this patch and my fix for the original bug to 9-branch. Regards Paul On Sun, 28 Jul 2019 at 22:50, Thomas Koenig wrote: > > Hello world, > > the attached patch fixes PR 90813, a regression with proc pointers. > The problem was quite complex, and I'd like to thank the people > who helped debug this; the most important clue came from Richard. > > The problem was that, for a procedure pointer variable declared > in a module in the same file, we were using a different backend > decl in the module than in the main program. This led to the > later parts of the compiler to think that the procedure pointer > was actually two variables which could not alias. Optimization > on some architectures such as Aarch64 and POWER (but not > on x86_64) then led to reordering of stores, leading to a segfault. > > The solution is to put the mangled names into the global > variable table, and to look for it when getting its backend > declaration. > > While debugging it, I also put in an option to dump the global > symbol table to standard output. I have included this in this > patch because I think this may not be the last bug in that > area :-) > > Regression-tested on powerpc64le-unknown-linux-gnu, where the > segfault showed up. No test case because is is already > in the test suite. Doc changes checked with "make dvi" and > "make pdf". > > OK for trunk? > > Regards > > Thomas > 2019-07-28 Thomas Koenig > > PR fortran/90813 > * dump-parse-tree.c (show_global_symbol): New function. > (gfc_dump_global_symbols): New function. > * gfortran.h (gfc_traverse_gsymbol): Add prototype. > (gfc_dump_global_symbols): Likewise. > * invoke.texi: Document -fdump-fortran-global. > * lang.opt: Add -fdump-fortran-global. > * parse.c (gfc_parse_file): Handle flag_dump_fortran_global. > * symbol.c (gfc_traverse_gsymbol): New function. > * trans-decl.c (sym_identifier): New function. > (mangled_identifier): New function, doing most of the work > of gfc_sym_mangled_identifier. > (gfc_sym_mangled_identifier): Use mangled_identifier. Add mangled > identifier to global symbol table. > (get_proc_pointer_decl): Use backend decl from global identifier > if present. -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran] PR90903 - Implement runtime checks for bit manipulation intrinsics
Hi Harald and Steve, The patch looks fine to me - it's good be committed. Thanks Paul On Mon, 15 Jul 2019 at 03:34, Steve Kargl wrote: > > Harald, thanks for the patch. I'm that the best person > for reading the trans-* file, but your patch and changes > look good to me. If no one else speaks up, in the next > day or so, please commit. > > -- > steve > > On Sun, Jul 14, 2019 at 09:37:27PM +0200, Harald Anlauf wrote: > > Ping! > > > > On 06/23/19 23:36, Harald Anlauf wrote: > > > Dear all, > > > > > > the attached patch provides run-time checks for the bit manipulation > > > intrinsic functions (IBSET/IBCLR/BTEST/SHIFT[RLA]/ISHFT/ISHFTC). > > > I am using only one testcase whose purpose is mainly to verify that > > > there are no false positives, which I consider essential, and one > > > "failing" test at the end. > > > > > > What is still missing are run-time checks for the subroutine MVBITS. > > > I am not sure yet how to handle that case (frontend or library?), > > > and I am open to suggestions. For this purpose I intend to leave > > > the PR open until a good solution is found. > > > > > > Regtested on x86_64-pc-linux-gnu. OK for trunk? > > > > > > Harald > > > > > > 2019-06-23 Harald Anlauf > > > > > > PR fortran/90903 > > > * libgfortran.h: Add mask for -fcheck=bits option. > > > * options.c (gfc_handle_runtime_check_option): Add option "bits" > > > to run-time checks selectable via -fcheck. > > > * trans-intrinsic.c (gfc_conv_intrinsic_btest) > > > (gfc_conv_intrinsic_singlebitop, gfc_conv_intrinsic_ibits) > > > (gfc_conv_intrinsic_shift, gfc_conv_intrinsic_ishft) > > > (gfc_conv_intrinsic_ishftc): Implement run-time checks for the > > > POS, LEN, SHIFT, and SIZE arguments. > > > * gfortran.texi: Document run-time checks for bit manipulation > > > intrinsics. > > > * invoke.texi: Document new -fcheck=bits option. > > > > > > 2019-06-23 Harald Anlauf > > > > > > PR fortran/90903 > > > * gfortran.dg/check_bits_1.f90: New testcase. > > > > > > > -- > Steve > 20170425 https://www.youtube.com/watch?v=VWUpyCsUKR4 > 20161221 https://www.youtube.com/watch?v=IbCHE-hONow -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran] PR91077 - [8/9/10 Regression] Wrong indexing when using a pointer
Thanks, Steve. Fixed with revisions 273176, 7 & 8. Paul On Sat, 6 Jul 2019 at 18:05, Steve Kargl wrote: > > On Sat, Jul 06, 2019 at 02:29:06PM +0100, Paul Richard Thomas wrote: > > As anticipated, 8-branch required a different patch but the difference > > was much smaller than anticipated. > > > > Bootstrapped and regetested on FC29/x86_64 - OK for 8-branch? > > > > OK for both patches. > > -- > Steve -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran] PR91077 - [8/9/10 Regression] Wrong indexing when using a pointer
As anticipated, 8-branch required a different patch but the difference was much smaller than anticipated. Bootstrapped and regetested on FC29/x86_64 - OK for 8-branch? Paul 2019-07-06 Paul Thomas PR fortran/91077 * trans-array.c (gfc_conv_scalarized_array_ref) Delete code that gave symbol backend decl for subref arrays. 2019-07-06 Paul Thomas PR fortran/91077 * gfortran.dg/pointer_array_11.f90 : New test. On Sat, 6 Jul 2019 at 11:48, Paul Richard Thomas wrote: > > This problem was caused by the code for scalarized array references to > subref arrays and deferred length variables not obtaining the correct > array descriptor and so getting the array span wrong. As it happens, > the lines, following the deleted part, correctly identify when the > info descriptor is a pointer and provide the span as appropriate. > > Bootstrapped and regtested on FC29/x86_64 - OK for trunk and 9-branch? > 8-branch might be somewhat more difficult to fix but I will give it a > try. This will require a separate submission. > > Paul > > 2019-07-06 Paul Thomas > > PR fortran/91077 > * trans-array.c (gfc_conv_scalarized_array_ref) Delete code > that gave symbol backend decl for subref arrays and deferred > length variables. > > 2019-07-06 Paul Thomas > > PR fortran/91077 > * gfortran.dg/pointer_array_11.f90 : New test. -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein Index: gcc/fortran/trans-array.c === *** gcc/fortran/trans-array.c (revision 272102) --- gcc/fortran/trans-array.c (working copy) *** gfc_conv_scalarized_array_ref (gfc_se * *** 3422,3431 if (build_class_array_ref (se, base, index)) return; ! if (expr && ((is_subref_array (expr) ! && GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (info->descriptor))) ! || (expr->ts.deferred && (expr->expr_type == EXPR_VARIABLE ! || expr->expr_type == EXPR_FUNCTION decl = expr->symtree->n.sym->backend_decl; /* A pointer array component can be detected from its field decl. Fix --- 3422,3429 if (build_class_array_ref (se, base, index)) return; ! if (expr && (expr->ts.deferred && (expr->expr_type == EXPR_VARIABLE ! || expr->expr_type == EXPR_FUNCTION))) decl = expr->symtree->n.sym->backend_decl; /* A pointer array component can be detected from its field decl. Fix Index: gcc/testsuite/gfortran.dg/pointer_array_11.f90 === *** gcc/testsuite/gfortran.dg/pointer_array_11.f90 (nonexistent) --- gcc/testsuite/gfortran.dg/pointer_array_11.f90 (working copy) *** *** 0 --- 1,90 + ! { dg-do run } + ! + ! Test the fix for PR91077 - both the original test and that in comment #4 of the PR. + ! + ! Contribute by Ygal Klein + ! + program test + implicit none + call original + call comment_4 + contains + subroutine original + integer, parameter :: length = 9 + real(8), dimension(2) :: a, b + integer :: i + type point +real(8) :: x + end type point + + type stored +type(point), dimension(:), allocatable :: np + end type stored + type(stored), dimension(:), pointer :: std =>null() + allocate(std(1)) + allocate(std(1)%np(length)) + std(1)%np(1)%x = 0.3d0 + std(1)%np(2)%x = 0.3555d0 + std(1)%np(3)%x = 0.26782d0 + std(1)%np(4)%x = 0d0 + std(1)%np(5)%x = 1.555d0 + std(1)%np(6)%x = 7.3d0 + std(1)%np(7)%x = 7.8d0 + std(1)%np(8)%x = 6.3d0 + std(1)%np(9)%x = 5.5d0 + !do i = 1, 2 + ! write(*, "('std(1)%np(',i1,')%x = ',1e22.14)") i, std(1)%np(i)%x + !end do + !do i = 1, 2 + ! write(*, "('std(1)%np(1:',i1,') = ',9e22.14)") i, std(1)%np(1:i)%x + !end do + a = std(1)%np(1:2)%x + b = [std(1)%np(1)%x, std(1)%np(2)%x] + !print *,a + !print *,b + if (allocated (std(1)%np)) deallocate (std(1)%np) + if (associated (std)) deallocate (std) + if (norm2(a - b) .gt. 1d-3) stop 1 + end subroutine + + subroutine comment_4 + integer, parameter :: length = 2 + real(8), dimension(length) :: a, b + integer :: i + + type point +real(8) :: x + end type point + + type points +type(point), dimension(:), pointer :: np=>null() + end type points + + type stored +integer :: l +type(points), pointer :: nfpoint=>null() + end type stored + + type(stored), dimension(:), pointer :: std=>null() + + + allocate(std(1)) + allocate(std(1)%nfpoint) + allocate(std(1)%nfpoint%np(length)) + std(1)%nfpoint%np(1)%x = 0.3d0
[Patch, fortran] PR91077 - [8/9/10 Regression] Wrong indexing when using a pointer
This problem was caused by the code for scalarized array references to subref arrays and deferred length variables not obtaining the correct array descriptor and so getting the array span wrong. As it happens, the lines, following the deleted part, correctly identify when the info descriptor is a pointer and provide the span as appropriate. Bootstrapped and regtested on FC29/x86_64 - OK for trunk and 9-branch? 8-branch might be somewhat more difficult to fix but I will give it a try. This will require a separate submission. Paul 2019-07-06 Paul Thomas PR fortran/91077 * trans-array.c (gfc_conv_scalarized_array_ref) Delete code that gave symbol backend decl for subref arrays and deferred length variables. 2019-07-06 Paul Thomas PR fortran/91077 * gfortran.dg/pointer_array_11.f90 : New test. Index: gcc/fortran/trans-array.c === *** gcc/fortran/trans-array.c (revision 272089) --- gcc/fortran/trans-array.c (working copy) *** gfc_conv_scalarized_array_ref (gfc_se * *** 3502,3520 return; if (get_CFI_desc (NULL, expr, &decl, ar)) - { decl = build_fold_indirect_ref_loc (input_location, decl); - goto done; - } - - if (expr && ((is_subref_array (expr) - && GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (info->descriptor))) - || (expr->ts.deferred && (expr->expr_type == EXPR_VARIABLE - || expr->expr_type == EXPR_FUNCTION - decl = expr->symtree->n.sym->backend_decl; - - if (decl && GFC_DECL_PTR_ARRAY_P (decl)) - goto done; /* A pointer array component can be detected from its field decl. Fix the descriptor, mark the resulting variable decl and pass it to --- 3502,3508 *** gfc_conv_scalarized_array_ref (gfc_se * *** 3532,3538 decl = info->descriptor; } - done: se->expr = gfc_build_array_ref (base, index, decl); } --- 3520,3525 Index: gcc/testsuite/gfortran.dg/pointer_array_11.f90 === *** gcc/testsuite/gfortran.dg/pointer_array_11.f90 (nonexistent) --- gcc/testsuite/gfortran.dg/pointer_array_11.f90 (working copy) *** *** 0 --- 1,90 + ! { dg-do run } + ! + ! Test the fix for PR91077 - both the original test and that in comment #4 of the PR. + ! + ! Contribute by Ygal Klein + ! + program test + implicit none + call original + call comment_4 + contains + subroutine original + integer, parameter :: length = 9 + real(8), dimension(2) :: a, b + integer :: i + type point +real(8) :: x + end type point + + type stored +type(point), dimension(:), allocatable :: np + end type stored + type(stored), dimension(:), pointer :: std =>null() + allocate(std(1)) + allocate(std(1)%np(length)) + std(1)%np(1)%x = 0.3d0 + std(1)%np(2)%x = 0.3555d0 + std(1)%np(3)%x = 0.26782d0 + std(1)%np(4)%x = 0d0 + std(1)%np(5)%x = 1.555d0 + std(1)%np(6)%x = 7.3d0 + std(1)%np(7)%x = 7.8d0 + std(1)%np(8)%x = 6.3d0 + std(1)%np(9)%x = 5.5d0 + !do i = 1, 2 + ! write(*, "('std(1)%np(',i1,')%x = ',1e22.14)") i, std(1)%np(i)%x + !end do + !do i = 1, 2 + ! write(*, "('std(1)%np(1:',i1,') = ',9e22.14)") i, std(1)%np(1:i)%x + !end do + a = std(1)%np(1:2)%x + b = [std(1)%np(1)%x, std(1)%np(2)%x] + !print *,a + !print *,b + if (allocated (std(1)%np)) deallocate (std(1)%np) + if (associated (std)) deallocate (std) + if (norm2(a - b) .gt. 1d-3) stop 1 + end subroutine + + subroutine comment_4 + integer, parameter :: length = 2 + real(8), dimension(length) :: a, b + integer :: i + + type point +real(8) :: x + end type point + + type points +type(point), dimension(:), pointer :: np=>null() + end type points + + type stored +integer :: l +type(points), pointer :: nfpoint=>null() + end type stored + + type(stored), dimension(:), pointer :: std=>null() + + + allocate(std(1)) + allocate(std(1)%nfpoint) + allocate(std(1)%nfpoint%np(length)) + std(1)%nfpoint%np(1)%x = 0.3d0 + std(1)%nfpoint%np(2)%x = 0.3555d0 + + !do i = 1, length + ! write(*, "('std(1)%nfpoint%np(',i1,')%x = ',1e22.14)") i, std(1)%nfpoint%np(i)%x + !end do + !do i = 1, length + ! write(*, "('std(1)%nfpoint%np(1:',i1,')%x = ',2e22.14)") i, std(1)%nfpoint%np(1:i)%x + !end do + a = std(1)%nfpoint%np(1:2)%x + b = [std(1)%nfpoint%np(1)%x, std(1)%nfpoint%np(2)%x] + if (associated (std(1)%nfpoint%np)) deallocate (std(1)%nfpoint%np) + if (associated (std(1)%nfpoint)) deallocate (std(1)%nfpoint) + if (associated (std)) deallocate (std) + if (norm2(a - b) .gt. 1d-3) stop 2 + end subroutine + end program test
Re: [PATCH] PR fortran/87907 -- Don't dereference a NULL pointer
Hi Steve, I'm surprised that you didn't commit as obvious :-) OK for trunk. Thanks Paul On Tue, 18 Jun 2019 at 20:30, Steve Kargl wrote: > > The attach patch has been regression tested on x86_64-*-freebsd. > If the pointer is NULL, the function simply returns. It seems > that gfortran then does the Right Thing. OK to commit? > > 2019-06-18 Steven G. Kargl > > PR fortran/87907 > * resolve.c (resolve_contained_fntype): Do not dereference a NULL > pointer. > > 2019-06-18 Steven G. Kargl > > PR fortran/87907 > * gfortran.dg/pr87907.f90: New testcase. > > > -- > Steve -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [PATCH] PR fortran/68544 -- A derived type cannot be actual argument
Hi Steve, That's good to go - thanks Paul On Wed, 12 Jun 2019 at 23:44, Steve Kargl wrote: > > The attach patch has been sitting in my tree for a year. > It has been tested and updated as others have changed > the gfortran code. The patch has been compiled and > regression tested on x86_64-*-freebsd. OK to commit? > > Either testcase should provide sufficient information > about the problem that this patch fixes. > > 2019-06-12 Steven G. Kargl > > PR fortran/68544 > * resolve.c (is_dt_name): New function to compare symbol name against > list of derived types. > (resolve_actual_arglist): Use it to find wrong code. > > 2019-06-12 Steven G. Kargl > > PR fortran/68544 > * gfortran.dg/pr68544.f90: New test. > * gfortran.dg/pr85687.f90: Modify test for new error message. > > > -- > Steve -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [PATCH] PR fortran/88810 -- Code clean up
Hi Steve, As the original author of the confusing code, I want to thank you for de-confusing it! OK Paul On Wed, 12 Jun 2019 at 20:01, Steve Kargl wrote: > > This PR flags a section of confusing but correct code. > The attach patch rewrites the code to hopefully provide > some clarity. It has lived my tree for around 6 months, > so has sufferred through numerous regression tests. > OK to commit? > > 2019-06-12 Steven G. Kargl > > PR fortran/88810 > * dependency.c (gfc_dep_resolver): Re-arrange code to make the logic > a bit more transparent. Fix 2 nearby formatting issues. > > -- > Steve -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [PATCH] PR fortran/89344 -- INTENT(IN) CLASS(*) cannot be assigned to
Hi Steve, OK - thanks Paul On Wed, 12 Jun 2019 at 19:42, Steve Kargl wrote: > > The attach patch has lived in my tree for 4 months. > It's time to submit it. Passed regression testing > for a long time. > > An INTENT(in) entity that has CLASS(*) dummy argument > should not use SELECT TYPE to then try to assign to the > entity. OK to commit? > > 2019-06-12 Steven G. Kargl > > PR fortran/89344 > * expr.c (gfc_check_vardef_context): Check for INTENT(IN) variable > in SELECT TYPE construct. > > 2019-06-12 Steven G. Kargl > > PR fortran/89344 > * gfortran.dg/pr89344.f90: New test. > > -- > Steve -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [PATCH] PR fortran/90002 -- Free array-spec for assumed-shaped coarray
OK, Steve Thanks Paul On Wed, 12 Jun 2019 at 02:04, Steve Kargl wrote: > > The attached patch fixes an ICE when freeing an array-spec > that involves an assumed-shaped coarray (see testcase). The > patch regression tests cleanly. I don't use coarrays, so > cannot say whether the testcase is valid Fortran. I simply > fixed the ICE. OK to commit? > > 2019-06-11 Steven G. Kargl > > PR fortran/90002 > * array.c (gfc_free_array_spec): When freeing an array-spec, avoid > an ICE for assumed-shape coarrays > > 2019-06-11 Steven G. Kargl > > PR fortran/90002 > * gfortran.dg/pr90002.f90: New test. > > -- > Steve -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran] PR90786 - [7/8/9/10 Regression] ICE on procedure pointer assignment to function with class pointer result
Hi Christophe, I cannot see anything wrong with the optimized code and valgrind gives a clean bill of health on x86_64. We need help of somebody with access to an arm/aarch64 device. Cheers Paul On Mon, 10 Jun 2019 at 14:37, Christophe Lyon wrote: > > On Mon, 10 Jun 2019 at 13:05, Paul Richard Thomas > wrote: > > > > Hi Christophe, > > > > I'll have a think about this tonight. Is valgrind or some similar > > available for arm/aarch64? > > Yes, valgrind is available. I don't know if it's installed on the > machines in the GCC computer farm though. > > Christophe > > > > > > Many thanks for flagging it up. > > > > Paul > > > > On Mon, 10 Jun 2019 at 10:07, Christophe Lyon > > wrote: > > > > > > On Sat, 8 Jun 2019 at 18:25, Andrew Benson > > > wrote: > > > > > > > > Thanks Paul for the quick fix! > > > > > > > > On Saturday, June 8, 2019 4:56:46 PM PDT Paul Richard Thomas wrote: > > > > > Committed as obvious in revision 272084. > > > > > > > > > > The problem was that the lhs symbol itself was not being checked as a > > > > > proc_pointer - just the expression component. > > > > > > > > > > I will get on with backporting tomorrow. > > > > > > > > > > Cheers > > > > > > > > > > Paul > > > > > > > > > > 2019-06-08 Paul Thomas > > > > > > > > > > PR fortran/90786 > > > > > * trans-expr.c (pointer_assignment_is_proc_pointer) Remove as > > > > > it is very simple and only called from one place. > > > > > (gfc_trans_pointer_assignment): Rename non_proc_pointer_assign > > > > > as non_proc_ptr_assign. Assign to it directly, rather than call > > > > > to above, deleted function and use gfc_expr_attr instead of > > > > > only checking the reference chain. > > > > > > > > > > 2019-06-08 Paul Thomas > > > > > > > > > > PR fortran/90786 > > > > > * gfortran.dg/proc_ptr_51.f90 : New test. > > > > > > > > > > > > > > Hi, > > > > > > I've noticed that this new test fails on arm/aarch64: > > > FAIL:gfortran.dg/proc_ptr_51.f90 -O2 execution test > > > FAIL:gfortran.dg/proc_ptr_51.f90 -O3 -fomit-frame-pointer > > > -funroll-loops -fpeel-loops -ftracer -finline-functions execution > > > test > > > FAIL:gfortran.dg/proc_ptr_51.f90 -O3 -g execution test > > > > > > the logs say: > > > Program received signal SIGSEGV: Segmentation fault - invalid memory > > > reference. > > > > > > Backtrace for this error: > > > #0 0xa938f66b in ??? > > > #1 0x0 in ??? > > > > > > Christophe > > > > > > > -- > > > > > > > > * Andrew Benson: > > > > http://users.obs.carnegiescience.edu/abenson/contact.html > > > > > > > > * Galacticus: https://bitbucket.org/galacticusdev/galacticus > > > > > > > > > > > > -- > > "If you can't explain it simply, you don't understand it well enough" > > - Albert Einstein -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran] PR90786 - [7/8/9/10 Regression] ICE on procedure pointer assignment to function with class pointer result
Hi Christophe, I'll have a think about this tonight. Is valgrind or some similar available for arm/aarch64? Many thanks for flagging it up. Paul On Mon, 10 Jun 2019 at 10:07, Christophe Lyon wrote: > > On Sat, 8 Jun 2019 at 18:25, Andrew Benson > wrote: > > > > Thanks Paul for the quick fix! > > > > On Saturday, June 8, 2019 4:56:46 PM PDT Paul Richard Thomas wrote: > > > Committed as obvious in revision 272084. > > > > > > The problem was that the lhs symbol itself was not being checked as a > > > proc_pointer - just the expression component. > > > > > > I will get on with backporting tomorrow. > > > > > > Cheers > > > > > > Paul > > > > > > 2019-06-08 Paul Thomas > > > > > > PR fortran/90786 > > > * trans-expr.c (pointer_assignment_is_proc_pointer) Remove as > > > it is very simple and only called from one place. > > > (gfc_trans_pointer_assignment): Rename non_proc_pointer_assign > > > as non_proc_ptr_assign. Assign to it directly, rather than call > > > to above, deleted function and use gfc_expr_attr instead of > > > only checking the reference chain. > > > > > > 2019-06-08 Paul Thomas > > > > > > PR fortran/90786 > > > * gfortran.dg/proc_ptr_51.f90 : New test. > > > > > > Hi, > > I've noticed that this new test fails on arm/aarch64: > FAIL:gfortran.dg/proc_ptr_51.f90 -O2 execution test > FAIL:gfortran.dg/proc_ptr_51.f90 -O3 -fomit-frame-pointer > -funroll-loops -fpeel-loops -ftracer -finline-functions execution > test > FAIL:gfortran.dg/proc_ptr_51.f90 -O3 -g execution test > > the logs say: > Program received signal SIGSEGV: Segmentation fault - invalid memory > reference. > > Backtrace for this error: > #0 0xa938f66b in ??? > #1 0x0 in ??? > > Christophe > > > -- > > > > * Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html > > > > * Galacticus: https://bitbucket.org/galacticusdev/galacticus > > -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
[Patch, fortran] PR90786 - [7/8/9/10 Regression] ICE on procedure pointer assignment to function with class pointer result
Committed as obvious in revision 272084. The problem was that the lhs symbol itself was not being checked as a proc_pointer - just the expression component. I will get on with backporting tomorrow. Cheers Paul 2019-06-08 Paul Thomas PR fortran/90786 * trans-expr.c (pointer_assignment_is_proc_pointer) Remove as it is very simple and only called from one place. (gfc_trans_pointer_assignment): Rename non_proc_pointer_assign as non_proc_ptr_assign. Assign to it directly, rather than call to above, deleted function and use gfc_expr_attr instead of only checking the reference chain. 2019-06-08 Paul Thomas PR fortran/90786 * gfortran.dg/proc_ptr_51.f90 : New test. Index: gcc/fortran/trans-expr.c === *** gcc/fortran/trans-expr.c (revision 272076) --- gcc/fortran/trans-expr.c (working copy) *** class_array_fcn: *** 4881,4887 parmse->expr = gfc_build_addr_expr (NULL_TREE, parmse->expr); /* Basically make this into ! if (present) { if (contiguous) --- 4881,4887 parmse->expr = gfc_build_addr_expr (NULL_TREE, parmse->expr); /* Basically make this into ! if (present) { if (contiguous) *** trans_caf_token_assign (gfc_se *lse, gfc *** 8979,9001 } } - /* Indentify class valued proc_pointer assignments. */ - - static bool - pointer_assignment_is_proc_pointer (gfc_expr * expr1, gfc_expr * expr2) - { - gfc_ref * ref; - - ref = expr1->ref; - while (ref && ref->next) - ref = ref->next; - - return ref && ref->type == REF_COMPONENT - && ref->u.c.component->attr.proc_pointer - && expr2->expr_type == EXPR_VARIABLE - && expr2->symtree->n.sym->attr.flavor == FL_PROCEDURE; - } - /* Do everything that is needed for a CLASS function expr2. */ --- 8979,8984 *** gfc_trans_pointer_assignment (gfc_expr * *** 9048,9054 tree desc; tree tmp; tree expr1_vptr = NULL_TREE; ! bool scalar, non_proc_pointer_assign; gfc_ss *ss; gfc_start_block (&block); --- 9031,9037 tree desc; tree tmp; tree expr1_vptr = NULL_TREE; ! bool scalar, non_proc_ptr_assign; gfc_ss *ss; gfc_start_block (&block); *** gfc_trans_pointer_assignment (gfc_expr * *** 9056,9062 gfc_init_se (&lse, NULL); /* Usually testing whether this is not a proc pointer assignment. */ ! non_proc_pointer_assign = !pointer_assignment_is_proc_pointer (expr1, expr2); /* Check whether the expression is a scalar or not; we cannot use expr1->rank as it can be nonzero for proc pointers. */ --- 9039,9047 gfc_init_se (&lse, NULL); /* Usually testing whether this is not a proc pointer assignment. */ ! non_proc_ptr_assign = !(gfc_expr_attr (expr1).proc_pointer ! && expr2->expr_type == EXPR_VARIABLE ! && expr2->symtree->n.sym->attr.flavor == FL_PROCEDURE); /* Check whether the expression is a scalar or not; we cannot use expr1->rank as it can be nonzero for proc pointers. */ *** gfc_trans_pointer_assignment (gfc_expr * *** 9066,9072 gfc_free_ss_chain (ss); if (expr1->ts.type == BT_DERIVED && expr2->ts.type == BT_CLASS ! && expr2->expr_type != EXPR_FUNCTION && non_proc_pointer_assign) { gfc_add_data_component (expr2); /* The following is required as gfc_add_data_component doesn't --- 9051,9057 gfc_free_ss_chain (ss); if (expr1->ts.type == BT_DERIVED && expr2->ts.type == BT_CLASS ! && expr2->expr_type != EXPR_FUNCTION && non_proc_ptr_assign) { gfc_add_data_component (expr2); /* The following is required as gfc_add_data_component doesn't *** gfc_trans_pointer_assignment (gfc_expr * *** 9086,9092 else gfc_conv_expr (&rse, expr2); ! if (non_proc_pointer_assign && expr1->ts.type == BT_CLASS) { trans_class_vptr_len_assignment (&block, expr1, expr2, &rse, NULL, NULL); --- 9071,9077 else gfc_conv_expr (&rse, expr2); ! if (non_proc_ptr_assign && expr1->ts.type == BT_CLASS) { trans_class_vptr_len_assignment (&block, expr1, expr2, &rse, NULL, NULL); Index: gcc/testsuite/gfortran.dg/proc_ptr_51.f90 === *** gcc/testsuite/gfortran.dg/proc_ptr_51.f90 (nonexistent) --- gcc/testsuite/gfortran.dg/proc_ptr_51.f90 (working copy) *** *** 0 --- 1,38 + ! { dg-do run } + ! + ! Test the fix for PR90786. + ! + ! Contributed by Andrew benson + ! + module f + procedure(c), pointer :: c_ + + type :: s +integer :: i = 42 + end type s + class(s), pointer :: res, tgt + + contains + + function c() +implicit none +class(s), pointer :: c +c => tgt +return + end function c + + subroutine fs() +implicit none +c_ => c ! This used
Re: [patch, fortran] Two tweaks to argument repacking
Hi Thomas, It looks good to me. OK for trunk. Thanks Paul On Sun, 2 Jun 2019 at 14:34, Thomas Koenig wrote: > > Hello world, > > this patch adds two tweaks to the argument repacking. > > First, when the size of an argument is known to be one, as in a(n1:n1), > we can directly pass a pointer - the stride may not be one, but it > does not matter. > > Second, the case where the array passed is actually contiguous is > more likely in practice, so it should get the fast path. I have > done this by defining a new predictor and setting the estimated > likelyhood at 75%, which ensured a path without jumps when the > arguments passed to bar were contiguous: > > module y > contains >subroutine bar(a,b) > real, dimension(:) :: a,b > call foo(a,b,size(a)) >end subroutine bar > end module y > > Test case is only for the first part - making one for the second > part would have been a bit too much. > > Regression-tested. OK for trunk? > > Regards > > Thomas > > 2019-06-02 Thomas Koenig > > PR fortran/90539 > * trans-expr.c (gfc_conv_subref_array_arg): If the size of the > expression can be determined to be one, treat it as contiguous. > Set likelyhood of presence of an actual argument according to > PRED_FORTRAN_ABSENT_DUMMY and likelyhood of being contiguous > according to PRED_FORTRAN_CONTIGUOUS. > > 2019-06-02 Thomas Koenig > > PR fortran/90539 > * predict.def (PRED_FORTRAN_CONTIGUOUS): New predictor. -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
[Patch, fortran] PR90498 - [8/9/10 Regression] ICE with select type/associate and derived type argument containing class(*)
This turns out to be obvious. The component of the dummy argument cannot be replaced by the hidden descriptor (actually the class object) and so an ICE occurs. The fix is to add a guard against the expression being a COMPONENT_REF. I will commit to all three branches tomorrow unless there are any objections. Bootstrapped and regtested on FC29/x86_64 Paul 2019-05-18 Paul Thomas PR fortran/90948 * trans-stmt.c (trans_associate_var) Do not use the saved descriptor if the expression is a COMPONENT_REF. 2019-05-18 Paul Thomas PR fortran/90948 * gfortran.dg/associate_48.f90 : New test. Index: gcc/fortran/trans-stmt.c === *** gcc/fortran/trans-stmt.c (revision 271056) --- gcc/fortran/trans-stmt.c (working copy) *** trans_associate_var (gfc_symbol *sym, gf *** 1858,1864 { if (e->symtree && DECL_LANG_SPECIFIC (e->symtree->n.sym->backend_decl) ! && GFC_DECL_SAVED_DESCRIPTOR (e->symtree->n.sym->backend_decl)) /* Use the original class descriptor stored in the saved descriptor to get the target_expr. */ target_expr = --- 1858,1865 { if (e->symtree && DECL_LANG_SPECIFIC (e->symtree->n.sym->backend_decl) ! && GFC_DECL_SAVED_DESCRIPTOR (e->symtree->n.sym->backend_decl) ! && TREE_CODE (target_expr) != COMPONENT_REF) /* Use the original class descriptor stored in the saved descriptor to get the target_expr. */ target_expr = Index: gcc/testsuite/gfortran.dg/associate_48.f90 === *** gcc/testsuite/gfortran.dg/associate_48.f90 (nonexistent) --- gcc/testsuite/gfortran.dg/associate_48.f90 (working copy) *** *** 0 --- 1,41 + ! { dg=do run } + ! + ! Test the fix for PR90498. + ! + ! Contributed by Vladimir Fuka + ! + type field_names_a + class(*), pointer :: var(:) =>null() + end type + + type(field_names_a),pointer :: a(:) + allocate (a(2)) + + allocate (a(1)%var(2), source = ["hello"," vlad"]) + allocate (a(2)%var(2), source = ["HELLO"," VLAD"]) + call s(a) + deallocate (a(1)%var) + deallocate (a(2)%var) + deallocate (a) + contains + subroutine s(a) + + type(field_names_a) :: a(:) + + select type (var => a(1)%var) + type is (character(*)) + if (any (var .ne. ["hello"," vlad"])) stop 1 + class default + stop + end select + + associate (var => a(2)%var) + select type (var) + type is (character(*)) + if (any (var .ne. ["HELLO"," VLAD"])) stop 2 + class default + stop + end select + end associate + end + end
Re: [Patch, fortran] ISO_Fortran_binding PRs 90093, 90352 & 90355
Committed to 9-branch as revision 271089. Paul On Fri, 10 May 2019 at 09:00, Paul Richard Thomas wrote: > > Committed to trunk as revision 271057. > > Will do likewise with 9-branch asap. > > Cheers > > Paul > > On Wed, 8 May 2019 at 19:40, Paul Richard Thomas > wrote: > > > > Unless there are any objections to this patch, I plan to commit to > > trunk and 9-branch tomorrow night, with the change to the testcase > > pointed out by Dominique. > > > > I sincerely hope that will be the end of CFI PRs for a little while, > > at least. I have a load of pending patches and want to get on with > > fixing PDTs. > > > > Cheers > > > > Paul > > > > On Mon, 6 May 2019 at 19:59, Paul Richard Thomas > > wrote: > > > > > > It helps to attach the patch! > > > > > > On Mon, 6 May 2019 at 19:57, Paul Richard Thomas > > > wrote: > > > > > > > > Unfortunately, this patch was still in the making at the release of > > > > 9.1. It is more or less self explanatory with the ChangeLogs. > > > > > > > > It should be noted that gfc_conv_expr_present could not be used in the > > > > fix for PR90093 because the passed descriptor is a CFI type. Instead, > > > > the test is for a null pointer passed. > > > > > > > > The changes to trans-array.c(gfc_trans_create_temp_array) have an eye > > > > on the future, as well as PR90355. I am progressing towards the point > > > > where all descriptors have 'span' set correctly so that > > > > trans.c(get_array_span) can be eliminated and much of the code in the > > > > callers can be simplified. > > > > > > > > Bootstrapped and regtested on FC29/x86_64 - OK for trunk and 9-branch? > > > > > > > > Paul > > > > > > > > 2019-05-06 Paul Thomas > > > > > > > > PR fortran/90093 > > > > * trans-decl.c (convert_CFI_desc): Test that the dummy is > > > > present before doing any of the conversions. > > > > > > > > PR fortran/90352 > > > > * decl.c (gfc_verify_c_interop_param): Restore the error for > > > > charlen > 1 actual arguments passed to bind(C) procs. > > > > Clean up trailing white space. > > > > > > > > PR fortran/90355 > > > > * trans-array.c (gfc_trans_create_temp_array): Set the 'span' > > > > field to the element length for all types. > > > > (gfc_conv_expr_descriptor): The force_no_tmp flag is used to > > > > prevent temporary creation, especially for substrings. > > > > * trans-decl.c (gfc_trans_deferred_vars): Rather than assert > > > > that the backend decl for the string length is non-null, use it > > > > as a condition before calling gfc_trans_vla_type_sizes. > > > > * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): 'force_no_tmp' > > > > is set before calling gfc_conv_expr_descriptor. > > > > * trans.c (get_array_span): Move the code for extracting 'span' > > > > from gfc_build_array_ref to this function. This is specific to > > > > descriptors that are component and indirect references. > > > > * trans.h : Add the force_no_tmp flag bitfield to gfc_se. > > > > > > > > 2019-05-06 Paul Thomas > > > > > > > > PR fortran/90093 > > > > * gfortran.dg/ISO_Fortran_binding_12.f90: New test. > > > > * gfortran.dg/ISO_Fortran_binding_12.c: Supplementary code. > > > > > > > > PR fortran/90352 > > > > * gfortran.dg/iso_c_binding_char_1.f90: New test. > > > > > > > > PR fortran/90355 > > > > * gfortran.dg/ISO_Fortran_binding_4.f90: Add 'substr' to test > > > > the direct passing of substrings as descriptors to bind(C). > > > > * gfortran.dg/assign_10.f90: Increase the tree_dump count of > > > > 'atmp' to account for the setting of the 'span' field. > > > > * gfortran.dg/transpose_optimization_2.f90: Ditto. > > > > > > > > -- > > "If you can't explain it simply, you don't understand it well enough" > > - Albert Einstein > > > > -- > "If you can't explain it simply, you don't understand it well enough" > - Albert Einstein -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran] ISO_Fortran_binding PRs 90093, 90352 & 90355
Committed to trunk as revision 271057. Will do likewise with 9-branch asap. Cheers Paul On Wed, 8 May 2019 at 19:40, Paul Richard Thomas wrote: > > Unless there are any objections to this patch, I plan to commit to > trunk and 9-branch tomorrow night, with the change to the testcase > pointed out by Dominique. > > I sincerely hope that will be the end of CFI PRs for a little while, > at least. I have a load of pending patches and want to get on with > fixing PDTs. > > Cheers > > Paul > > On Mon, 6 May 2019 at 19:59, Paul Richard Thomas > wrote: > > > > It helps to attach the patch! > > > > On Mon, 6 May 2019 at 19:57, Paul Richard Thomas > > wrote: > > > > > > Unfortunately, this patch was still in the making at the release of > > > 9.1. It is more or less self explanatory with the ChangeLogs. > > > > > > It should be noted that gfc_conv_expr_present could not be used in the > > > fix for PR90093 because the passed descriptor is a CFI type. Instead, > > > the test is for a null pointer passed. > > > > > > The changes to trans-array.c(gfc_trans_create_temp_array) have an eye > > > on the future, as well as PR90355. I am progressing towards the point > > > where all descriptors have 'span' set correctly so that > > > trans.c(get_array_span) can be eliminated and much of the code in the > > > callers can be simplified. > > > > > > Bootstrapped and regtested on FC29/x86_64 - OK for trunk and 9-branch? > > > > > > Paul > > > > > > 2019-05-06 Paul Thomas > > > > > > PR fortran/90093 > > > * trans-decl.c (convert_CFI_desc): Test that the dummy is > > > present before doing any of the conversions. > > > > > > PR fortran/90352 > > > * decl.c (gfc_verify_c_interop_param): Restore the error for > > > charlen > 1 actual arguments passed to bind(C) procs. > > > Clean up trailing white space. > > > > > > PR fortran/90355 > > > * trans-array.c (gfc_trans_create_temp_array): Set the 'span' > > > field to the element length for all types. > > > (gfc_conv_expr_descriptor): The force_no_tmp flag is used to > > > prevent temporary creation, especially for substrings. > > > * trans-decl.c (gfc_trans_deferred_vars): Rather than assert > > > that the backend decl for the string length is non-null, use it > > > as a condition before calling gfc_trans_vla_type_sizes. > > > * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): 'force_no_tmp' > > > is set before calling gfc_conv_expr_descriptor. > > > * trans.c (get_array_span): Move the code for extracting 'span' > > > from gfc_build_array_ref to this function. This is specific to > > > descriptors that are component and indirect references. > > > * trans.h : Add the force_no_tmp flag bitfield to gfc_se. > > > > > > 2019-05-06 Paul Thomas > > > > > > PR fortran/90093 > > > * gfortran.dg/ISO_Fortran_binding_12.f90: New test. > > > * gfortran.dg/ISO_Fortran_binding_12.c: Supplementary code. > > > > > > PR fortran/90352 > > > * gfortran.dg/iso_c_binding_char_1.f90: New test. > > > > > > PR fortran/90355 > > > * gfortran.dg/ISO_Fortran_binding_4.f90: Add 'substr' to test > > > the direct passing of substrings as descriptors to bind(C). > > > * gfortran.dg/assign_10.f90: Increase the tree_dump count of > > > 'atmp' to account for the setting of the 'span' field. > > > * gfortran.dg/transpose_optimization_2.f90: Ditto. > > > > -- > "If you can't explain it simply, you don't understand it well enough" > - Albert Einstein -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran] ISO_Fortran_binding PRs 90093, 90352 & 90355
Unless there are any objections to this patch, I plan to commit to trunk and 9-branch tomorrow night, with the change to the testcase pointed out by Dominique. I sincerely hope that will be the end of CFI PRs for a little while, at least. I have a load of pending patches and want to get on with fixing PDTs. Cheers Paul On Mon, 6 May 2019 at 19:59, Paul Richard Thomas wrote: > > It helps to attach the patch! > > On Mon, 6 May 2019 at 19:57, Paul Richard Thomas > wrote: > > > > Unfortunately, this patch was still in the making at the release of > > 9.1. It is more or less self explanatory with the ChangeLogs. > > > > It should be noted that gfc_conv_expr_present could not be used in the > > fix for PR90093 because the passed descriptor is a CFI type. Instead, > > the test is for a null pointer passed. > > > > The changes to trans-array.c(gfc_trans_create_temp_array) have an eye > > on the future, as well as PR90355. I am progressing towards the point > > where all descriptors have 'span' set correctly so that > > trans.c(get_array_span) can be eliminated and much of the code in the > > callers can be simplified. > > > > Bootstrapped and regtested on FC29/x86_64 - OK for trunk and 9-branch? > > > > Paul > > > > 2019-05-06 Paul Thomas > > > > PR fortran/90093 > > * trans-decl.c (convert_CFI_desc): Test that the dummy is > > present before doing any of the conversions. > > > > PR fortran/90352 > > * decl.c (gfc_verify_c_interop_param): Restore the error for > > charlen > 1 actual arguments passed to bind(C) procs. > > Clean up trailing white space. > > > > PR fortran/90355 > > * trans-array.c (gfc_trans_create_temp_array): Set the 'span' > > field to the element length for all types. > > (gfc_conv_expr_descriptor): The force_no_tmp flag is used to > > prevent temporary creation, especially for substrings. > > * trans-decl.c (gfc_trans_deferred_vars): Rather than assert > > that the backend decl for the string length is non-null, use it > > as a condition before calling gfc_trans_vla_type_sizes. > > * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): 'force_no_tmp' > > is set before calling gfc_conv_expr_descriptor. > > * trans.c (get_array_span): Move the code for extracting 'span' > > from gfc_build_array_ref to this function. This is specific to > > descriptors that are component and indirect references. > > * trans.h : Add the force_no_tmp flag bitfield to gfc_se. > > > > 2019-05-06 Paul Thomas > > > > PR fortran/90093 > > * gfortran.dg/ISO_Fortran_binding_12.f90: New test. > > * gfortran.dg/ISO_Fortran_binding_12.c: Supplementary code. > > > > PR fortran/90352 > > * gfortran.dg/iso_c_binding_char_1.f90: New test. > > > > PR fortran/90355 > > * gfortran.dg/ISO_Fortran_binding_4.f90: Add 'substr' to test > > the direct passing of substrings as descriptors to bind(C). > > * gfortran.dg/assign_10.f90: Increase the tree_dump count of > > 'atmp' to account for the setting of the 'span' field. > > * gfortran.dg/transpose_optimization_2.f90: Ditto. -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran] ISO_Fortran_binding PRs 90093, 90352 & 90355
Hi Dominique, Many thanks - I had already found this after replenishing my tree and regtesting. I don't quite know how it escaped but the fix is obvious. Amicalement Paul On Tue, 7 May 2019 at 09:39, Dominique d'Humières wrote: > > Hi Paul, > > With your patch, I see > > FAIL: gfortran.dg/iso_c_binding_char_1.f90 -O (test for errors, line 8) > FAIL: gfortran.dg/iso_c_binding_char_1.f90 -O (test for errors, line 9) > FAIL: gfortran.dg/iso_c_binding_char_1.f90 -O (test for excess errors) > > This is due to a bad location of the errors: > > /opt/gcc/work/gcc/testsuite/gfortran.dg/iso_c_binding_char_1.f90:7:16: > > 7 | subroutine bar(c,d) BIND(C) > |1 > Error: Character argument 'c' at (1) must be length 1 because procedure 'bar' > is BIND(C) > /opt/gcc/work/gcc/testsuite/gfortran.dg/iso_c_binding_char_1.f90:7:18: > > 7 | subroutine bar(c,d) BIND(C) > | 1 > Error: Character argument 'd' at (1) must be length 1 because procedure 'bar' > is BIND(C) > > TIA > > Dominique -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran] ISO_Fortran_binding PRs 90093, 90352 & 90355
It helps to attach the patch! On Mon, 6 May 2019 at 19:57, Paul Richard Thomas wrote: > > Unfortunately, this patch was still in the making at the release of > 9.1. It is more or less self explanatory with the ChangeLogs. > > It should be noted that gfc_conv_expr_present could not be used in the > fix for PR90093 because the passed descriptor is a CFI type. Instead, > the test is for a null pointer passed. > > The changes to trans-array.c(gfc_trans_create_temp_array) have an eye > on the future, as well as PR90355. I am progressing towards the point > where all descriptors have 'span' set correctly so that > trans.c(get_array_span) can be eliminated and much of the code in the > callers can be simplified. > > Bootstrapped and regtested on FC29/x86_64 - OK for trunk and 9-branch? > > Paul > > 2019-05-06 Paul Thomas > > PR fortran/90093 > * trans-decl.c (convert_CFI_desc): Test that the dummy is > present before doing any of the conversions. > > PR fortran/90352 > * decl.c (gfc_verify_c_interop_param): Restore the error for > charlen > 1 actual arguments passed to bind(C) procs. > Clean up trailing white space. > > PR fortran/90355 > * trans-array.c (gfc_trans_create_temp_array): Set the 'span' > field to the element length for all types. > (gfc_conv_expr_descriptor): The force_no_tmp flag is used to > prevent temporary creation, especially for substrings. > * trans-decl.c (gfc_trans_deferred_vars): Rather than assert > that the backend decl for the string length is non-null, use it > as a condition before calling gfc_trans_vla_type_sizes. > * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): 'force_no_tmp' > is set before calling gfc_conv_expr_descriptor. > * trans.c (get_array_span): Move the code for extracting 'span' > from gfc_build_array_ref to this function. This is specific to > descriptors that are component and indirect references. > * trans.h : Add the force_no_tmp flag bitfield to gfc_se. > > 2019-05-06 Paul Thomas > > PR fortran/90093 > * gfortran.dg/ISO_Fortran_binding_12.f90: New test. > * gfortran.dg/ISO_Fortran_binding_12.c: Supplementary code. > > PR fortran/90352 > * gfortran.dg/iso_c_binding_char_1.f90: New test. > > PR fortran/90355 > * gfortran.dg/ISO_Fortran_binding_4.f90: Add 'substr' to test > the direct passing of substrings as descriptors to bind(C). > * gfortran.dg/assign_10.f90: Increase the tree_dump count of > 'atmp' to account for the setting of the 'span' field. > * gfortran.dg/transpose_optimization_2.f90: Ditto. Index: gcc/fortran/decl.c === *** gcc/fortran/decl.c (revision 270622) --- gcc/fortran/decl.c (working copy) *** match_data_constant (gfc_expr **result) *** 406,412 contains the right constant expression. Check here. */ if ((*result)->symtree == NULL && (*result)->expr_type == EXPR_CONSTANT ! && ((*result)->ts.type == BT_INTEGER || (*result)->ts.type == BT_REAL)) return m; --- 406,412 contains the right constant expression. Check here. */ if ((*result)->symtree == NULL && (*result)->expr_type == EXPR_CONSTANT ! && ((*result)->ts.type == BT_INTEGER || (*result)->ts.type == BT_REAL)) return m; *** gfc_verify_c_interop_param (gfc_symbol * *** 1493,1511 /* Character strings are only C interoperable if they have a length of 1. */ ! if (sym->ts.type == BT_CHARACTER) { gfc_charlen *cl = sym->ts.u.cl; if (!cl || !cl->length || cl->length->expr_type != EXPR_CONSTANT || mpz_cmp_si (cl->length->value.integer, 1) != 0) { ! if (!gfc_notify_std (GFC_STD_F2018, ! "Character argument %qs at %L " ! "must be length 1 because " ! "procedure %qs is BIND(C)", ! sym->name, &sym->declared_at, ! sym->ns->proc_name->name)) ! retval = false; } } --- 1493,1510 /* Character strings are only C interoperable if they have a length of 1. */ ! if (sym->ts.type == BT_CHARACTER && !sym->attr.dimension) { gfc_charlen *cl = sym->ts.u.cl; if (!cl || !cl->length || cl->length->expr_type != EXPR_CONSTANT || mpz_cmp_si (cl->length->value.integer, 1) != 0) { ! gfc_error ("Character argument %qs at %L " ! "must
[Patch, fortran] ISO_Fortran_binding PRs 90093, 90352 & 90355
Unfortunately, this patch was still in the making at the release of 9.1. It is more or less self explanatory with the ChangeLogs. It should be noted that gfc_conv_expr_present could not be used in the fix for PR90093 because the passed descriptor is a CFI type. Instead, the test is for a null pointer passed. The changes to trans-array.c(gfc_trans_create_temp_array) have an eye on the future, as well as PR90355. I am progressing towards the point where all descriptors have 'span' set correctly so that trans.c(get_array_span) can be eliminated and much of the code in the callers can be simplified. Bootstrapped and regtested on FC29/x86_64 - OK for trunk and 9-branch? Paul 2019-05-06 Paul Thomas PR fortran/90093 * trans-decl.c (convert_CFI_desc): Test that the dummy is present before doing any of the conversions. PR fortran/90352 * decl.c (gfc_verify_c_interop_param): Restore the error for charlen > 1 actual arguments passed to bind(C) procs. Clean up trailing white space. PR fortran/90355 * trans-array.c (gfc_trans_create_temp_array): Set the 'span' field to the element length for all types. (gfc_conv_expr_descriptor): The force_no_tmp flag is used to prevent temporary creation, especially for substrings. * trans-decl.c (gfc_trans_deferred_vars): Rather than assert that the backend decl for the string length is non-null, use it as a condition before calling gfc_trans_vla_type_sizes. * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): 'force_no_tmp' is set before calling gfc_conv_expr_descriptor. * trans.c (get_array_span): Move the code for extracting 'span' from gfc_build_array_ref to this function. This is specific to descriptors that are component and indirect references. * trans.h : Add the force_no_tmp flag bitfield to gfc_se. 2019-05-06 Paul Thomas PR fortran/90093 * gfortran.dg/ISO_Fortran_binding_12.f90: New test. * gfortran.dg/ISO_Fortran_binding_12.c: Supplementary code. PR fortran/90352 * gfortran.dg/iso_c_binding_char_1.f90: New test. PR fortran/90355 * gfortran.dg/ISO_Fortran_binding_4.f90: Add 'substr' to test the direct passing of substrings as descriptors to bind(C). * gfortran.dg/assign_10.f90: Increase the tree_dump count of 'atmp' to account for the setting of the 'span' field. * gfortran.dg/transpose_optimization_2.f90: Ditto.
Re: [patch, fortran] Inline packing for array temporaries
Hi Thomas, Could you provide the patch, please, or was it already posted? Cheers Paul On Sun, 28 Apr 2019 at 10:46, Thomas Koenig wrote: > > Hello world, > > going back a patch which was not included in gcc-9 because it was too > late in the development cycle, here is a patch which, when optimizing > and not optimizing for size, does inline packing for an argument. > As you can see from the code and the test cases, there is provision > for optional arguments. It was necessary to split some test cases > to take account for the new pack inline / pack in the library split. > > I did regression-testing on x86_64-pc-linux-gnu, in 64-bit mode. > (Dominique, could you tell us again what the magic incantation for > 32-bit mode is?) > > OK for trunk? (Not for backporting) > > Regards > > Thomas > > 2019-04-28 Thomas Koenig > > PR fortran/88821 > * expr.c (gfc_is_simply_contiguous): Return true for > an EXPR_ARRAY. > * trans-array.c (is_pointer): New function. > (gfc_conv_array_parameter): Call gfc_conv_subref_array_arg > when not optimizing and not optimizing for size if the formal > arg is passed by reference. > * trans-expr.c (gfc_conv_subref_array_arg): Add arguments > fsym, proc_name and sym. Add run-time warning for temporary > array creation. Wrap argument if passing on an optional > argument to an optional argument. > * trans.h (gfc_conv_subref_array_arg): Add optional arguments > fsym, proc_name and sym to prototype. > > 2019-04-28 Thomas Koenig > > PR fortran/88821 > * gfortran.dg/alloc_comp_auto_array_3.f90: Add -O0 to dg-options > to make sure the test for internal_pack is retained. > * gfortran.dg/assumed_type_2.f90: Split compile and run time > tests into this and > * gfortran.dg/assumed_type_2a.f90: New file. > * gfortran.dg/c_loc_test_22.f90: Likewise. > * gfortran.dg/contiguous_3.f90: Likewise. > * gfortran.dg/internal_pack_11.f90: Likewise. > * gfortran.dg/internal_pack_12.f90: Likewise. > * gfortran.dg/internal_pack_16.f90: Likewise. > * gfortran.dg/internal_pack_17.f90: Likewise. > * gfortran.dg/internal_pack_18.f90: Likewise. > * gfortran.dg/internal_pack_4.f90: Likewise. > * gfortran.dg/internal_pack_5.f90: Add -O0 to dg-options > to make sure the test for internal_pack is retained. > * gfortran.dg/internal_pack_6.f90: Split compile and run time > tests into this and > * gfortran.dg/internal_pack_6a.f90: New file. > * gfortran.dg/internal_pack_8.f90: Likewise. > * gfortran.dg/missing_optional_dummy_6: Split compile and run time > tests into this and > * gfortran.dg/missing_optional_dummy_6a.f90: New file. > * gfortran.dg/no_arg_check_2.f90: Split compile and run time tests > into this and > * gfortran.dg/no_arg_check_2a.f90: New file. > * gfortran.dg/typebound_assignment_5.f90: Split compile and run > time > tests into this and > * gfortran.dg/typebound_assignment_5a.f90: New file. > * gfortran.dg/typebound_assignment_6.f90: Split compile and run > time > tests into this and > * gfortran.dg/typebound_assignment_6a.f90: New file. > * gfortran.dg/internal_pack_19.f90: New file. > * gfortran.dg/internal_pack_20.f90: New file. > * gfortran.dg/internal_pack_21.f90: New file. -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.
Hi Manfred, I took a look at it and couldn't see what the problem is. I'll have another stab at it tomorrow morning. Cheers Paul On Fri, 26 Apr 2019 at 16:39, Manfred Schwarb wrote: > > Hi, > > I still see this issue on 32bit, is the plan to let > the test ISO_Fortran_binding_4.f90 be broken for the 9.0 release? > > Cheers, > Manfred > > > Am 15.04.19 um 10:30 schrieb Dominique d'Humières: > > Hi Paul, > > > > I have found another glitch with -m32 and -O1 or -Os, but not with other > > values: > > > > % gfc /opt/gcc/_clean/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_4.f90 > > -m32 -O > > % ./a.out > > FAIL > > Note: The following floating-point exceptions are signalling: IEEE_DENORMAL > > STOP 1 > > > > This looks tricky: if I add a line > > > > print *, x > > > > before > > > > if (any (abs (x - [1.,20.,3.,40.,5.,60.]) > 1.e-6)) stop 2 > > > > the test succeeds!-( > > > > Also you don’t want pr89844 to be solved, don’t you? > > > > TIA > > > > Dominique > > > > > >> Le 11 avr. 2019 à 16:44, Paul Richard Thomas > >> a écrit : > >> > >> Hi Dominique, > >> > >> Yes indeed - I used int(kind(loc(res))) to achieve the same effect. > >> > >> I am looking for but failing to find a similar problem for PR89846. > >> Tomorrow I turn my attention to an incorrect cast in the compiler. > >> > >> Regards > >> > >> Paul > > > > > -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran] PR57284 - [OOP] ICE with find_array_spec for polymorphic arrays
Thanks, Steve. Committed as revision 270489. Paul On Fri, 19 Apr 2019 at 18:28, Steve Kargl wrote: > > On Fri, Apr 19, 2019 at 06:19:00PM +0100, Paul Richard Thomas wrote: > > The part of this patch in resolve.c had essentially already been > > sorted out by Tobias Burnus in comment #2 of the PR. I suspect that he > > must have been put off the trail by the segfault that occurred when > > this was implemented. In the end, the reason for the segfault is quite > > straight forward and comes about because the temporary declarations > > representing class actual arguments cause gfc_conv_component_ref to > > barf, when porcessing the _data component. However, they are amenable > > to gfc_class_data_get and so this is used in the fix. > > > > Bootstrapped and regtested on FC29/x86_64 - OK for trunk? > > > > Looks good to me. Where are we in the release cycle? > Do you need release manager approval to apply the > patch? > > -- > Steve -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [PATCH] PR fortran/90166 -- check F2018:C1547
It looks good to me, modulo Dominique's query. OK for trunk (note from Richi - still aiming for zero P1 bugs so you're good to go). Thanks Paul On Fri, 19 Apr 2019 at 22:44, Steve Kargl wrote: > > The attached patch fixes PR fortran/91066. The original > code was feeding a nonsense string of tokens to the > assembler causing it to toss its cookies. It turns out > that gfortran was not enforcing the constraint C1547 > from Fortran 2018. The attached patch now performs > that check. Regression tested on x86_64-*-freebsd. > OK to commit? > > 2019-04-19 Steven G. Kargl > > PR fortran/90166 > * decl.c (in_module_or_interface): New function to check that the > current state is in a module, submodule, or interface. > (gfc_match_prefix): Use it. > > PR fortran/90166 > * gfortran.dg/submodule_22.f08: Add additional dg-error comments. > > -- > Steve -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
[Patch, fortran] PR57284 - [OOP] ICE with find_array_spec for polymorphic arrays
The part of this patch in resolve.c had essentially already been sorted out by Tobias Burnus in comment #2 of the PR. I suspect that he must have been put off the trail by the segfault that occurred when this was implemented. In the end, the reason for the segfault is quite straight forward and comes about because the temporary declarations representing class actual arguments cause gfc_conv_component_ref to barf, when porcessing the _data component. However, they are amenable to gfc_class_data_get and so this is used in the fix. Bootstrapped and regtested on FC29/x86_64 - OK for trunk? Paul 2019-04-19 Paul Thomas PR fortran/57284 * resolve.c (find_array_spec): If this is a class expression and the symbol and component array specs are the same, this is not an error. *trans-intrinsic.c (gfc_conv_intrinsic_size): If a class symbol argument, has no namespace, it has come from the interface mapping and the _data component must be accessed directly. 2019-04-19 Paul Thomas PR fortran/57284 * gfortran.dg/class_70.f03 Index: gcc/fortran/resolve.c === *** gcc/fortran/resolve.c (revision 270352) --- gcc/fortran/resolve.c (working copy) *** find_array_spec (gfc_expr *e) *** 4712,4720 gfc_array_spec *as; gfc_component *c; gfc_ref *ref; if (e->symtree->n.sym->ts.type == BT_CLASS) ! as = CLASS_DATA (e->symtree->n.sym)->as; else as = e->symtree->n.sym->as; --- 4712,4724 gfc_array_spec *as; gfc_component *c; gfc_ref *ref; + bool class_as = false; if (e->symtree->n.sym->ts.type == BT_CLASS) ! { ! as = CLASS_DATA (e->symtree->n.sym)->as; ! class_as = true; ! } else as = e->symtree->n.sym->as; *** find_array_spec (gfc_expr *e) *** 4733,4739 c = ref->u.c.component; if (c->attr.dimension) { ! if (as != NULL) gfc_internal_error ("find_array_spec(): unused as(1)"); as = c->as; } --- 4737,4743 c = ref->u.c.component; if (c->attr.dimension) { ! if (as != NULL && !(class_as && as == c->as)) gfc_internal_error ("find_array_spec(): unused as(1)"); as = c->as; } Index: gcc/fortran/trans-intrinsic.c === *** gcc/fortran/trans-intrinsic.c (revision 270352) --- gcc/fortran/trans-intrinsic.c (working copy) *** gfc_conv_intrinsic_size (gfc_se * se, gf *** 7446,7451 --- 7446,7453 tree fncall0; tree fncall1; gfc_se argse; + gfc_expr *e; + gfc_symbol *sym = NULL; gfc_init_se (&argse, NULL); actual = expr->value.function.actual; *** gfc_conv_intrinsic_size (gfc_se * se, gf *** 7453,7464 if (actual->expr->ts.type == BT_CLASS) gfc_add_class_array_ref (actual->expr); argse.data_not_needed = 1; ! if (gfc_is_class_array_function (actual->expr)) { /* For functions that return a class array conv_expr_descriptor is not able to get the descriptor right. Therefore this special case. */ ! gfc_conv_expr_reference (&argse, actual->expr); argse.expr = gfc_build_addr_expr (NULL_TREE, gfc_class_data_get (argse.expr)); } --- 7455,7485 if (actual->expr->ts.type == BT_CLASS) gfc_add_class_array_ref (actual->expr); + e = actual->expr; + + /* These are emerging from the interface mapping, when a class valued + function appears as the rhs in a realloc on assign statement, where + the size of the result is that of one of the actual arguments. */ + if (e->expr_type == EXPR_VARIABLE + && e->symtree->n.sym->ns == NULL /* This is distinctive! */ + && e->symtree->n.sym->ts.type == BT_CLASS + && e->ref && e->ref->type == REF_COMPONENT + && strcmp (e->ref->u.c.component->name, "_data") == 0) + sym = e->symtree->n.sym; + argse.data_not_needed = 1; ! if (gfc_is_class_array_function (e)) { /* For functions that return a class array conv_expr_descriptor is not able to get the descriptor right. Therefore this special case. */ ! gfc_conv_expr_reference (&argse, e); ! argse.expr = gfc_build_addr_expr (NULL_TREE, ! gfc_class_data_get (argse.expr)); ! } ! else if (sym && sym->backend_decl) ! { ! gcc_assert (GFC_CLASS_TYPE_P (TREE_TYPE (sym->backend_decl))); ! argse.expr = sym->backend_decl; argse.expr = gfc_build_addr_expr (NULL_TREE, gfc_class_data_get (argse.expr)); } Index: gcc/testsuite/gfortran.dg/class_70.f03 === *** gcc/testsuite/gfortran.dg/class_70.f03 (nonexistent) --- gcc/testsuite/gfortran.dg/class_70.f03 (working copy) *** *** 0 --- 1,38 + ! { dg-do run } + ! + ! Test the fix for PR57284 - [OOP] ICE with find_array_spec for polymor
Re: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.
Dear Dominique, Gilles and Reinhold, Thank you for your rapid feedback. We might even get a reasonably functional ISO Fortran binding in place for 9-branch release :-) On your remaining nits: (i) ISO_Fortran_binding_4.f90 -m32 -O1/Os looks awful. I will take a look, though. (ii) pr89844 being fixed by an earlier patch led me to give it lower priority. I will look to see whether another testcase is required to nail it down. (iii) I will take a look at 90093 - it should be straight forward. I do not regard it as being a regression, however, since the arguments were not being correctly handled until now - ie. were not converted from cfi to gfc descriptors. Cheers Paul On Mon, 15 Apr 2019 at 10:27, Bader, Reinhold wrote: > > Dear Paul, > > mostly looks good. Apart from a regression with optional arguments reported as > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90093 > all other test cases I have now execute correctly. > > Cheers > Reinhold > > > -Ursprüngliche Nachricht- > > Von: Paul Richard Thomas > > Gesendet: Sonntag, 14. April 2019 20:16 > > An: Thomas Koenig > > Cc: Gilles Gouaillardet ; Bader, Reinhold > > ; fort...@gcc.gnu.org; gcc-patches > patc...@gcc.gnu.org> > > Betreff: Re: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes. > > > > Hi Thomas, > > > > Thanks a lot. Committed as revision 270353. > > > > I was determined not to repeat the PDT experience, which is still nagging at > > me. That has to be the next major gfc project, I guess. > > > > Regards > > > > Paul > > > > On Sun, 14 Apr 2019 at 18:08, Thomas Koenig > > wrote: > > > > > > Hi Paul, > > > > > > > > > > Please find attached the updated patch, which fixes the problem with > > > > -m32 in PR90022, eliminates the temporary creation for INTENT(IN) > > > > dummies and fixes PR89846. > > > > > > > > While it looks like it should be intrusive because of its size, I > > > > believe that the patch is still safe for trunk since it is hidden > > > > behind tests for CFI descriptors. > > > > > > > > Bootstraps and regtests on FC29/x86_64 - OK for trunk? > > > > > > OK. > > > > > > I we're going into the gcc 9 release with an implementation of the C > > > interop features, it will be better with fewer bugs :-) > > > > > > Thanks a lot for working on it! > > > > > > Regards > > > > > > Thomas > > > > > > > > -- > > "If you can't explain it simply, you don't understand it well enough" > > - Albert Einstein -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.
Hi Thomas, Thanks a lot. Committed as revision 270353. I was determined not to repeat the PDT experience, which is still nagging at me. That has to be the next major gfc project, I guess. Regards Paul On Sun, 14 Apr 2019 at 18:08, Thomas Koenig wrote: > > Hi Paul, > > > > Please find attached the updated patch, which fixes the problem with > > -m32 in PR90022, eliminates the temporary creation for INTENT(IN) > > dummies and fixes PR89846. > > > > While it looks like it should be intrusive because of its size, I > > believe that the patch is still safe for trunk since it is hidden > > behind tests for CFI descriptors. > > > > Bootstraps and regtests on FC29/x86_64 - OK for trunk? > > OK. > > I we're going into the gcc 9 release with an implementation of the > C interop features, it will be better with fewer bugs :-) > > Thanks a lot for working on it! > > Regards > > Thomas -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: AW: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.
Dear All, Please find attached the updated patch, which fixes the problem with -m32 in PR90022, eliminates the temporary creation for INTENT(IN) dummies and fixes PR89846. While it looks like it should be intrusive because of its size, I believe that the patch is still safe for trunk since it is hidden behind tests for CFI descriptors. Bootstraps and regtests on FC29/x86_64 - OK for trunk? Cheers Paul 2019-04-14 Paul Thomas PR fortran/89843 * trans-decl.c (gfc_get_symbol_decl): Assumed shape and assumed rank dummies of bind C procs require deferred initialization. (convert_CFI_desc): New procedure to convert incoming CFI descriptors to gfc types and back again. (gfc_trans_deferred_vars): Call it. * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Null the CFI descriptor pointer. Free the descriptor in all cases. PR fortran/89846 * expr.c (is_CFI_desc): New function. (is_subref_array): Tidy up by referencing the symbol directly. * gfortran.h : Prototype for is_CFI_desc. * trans_array.c (get_CFI_desc): New function. (gfc_get_array_span, gfc_conv_scalarized_array_ref, gfc_conv_array_ref): Use it. * trans.c (get_array_span): Extract the span from descriptors that are indirect references. PR fortran/90022 * trans-decl.c (gfc_get_symbol_decl): Make sure that the se expression is a pointer type before converting it to the symbol backend_decl type. * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Eliminate temporary creation for intent(in). 2019-04-14 Paul Thomas PR fortran/89843 * gfortran.dg/ISO_Fortran_binding_4.f90: Modify the value of x in ctg. Test the conversion of the descriptor types in the main program. * gfortran.dg/ISO_Fortran_binding_10.f90: New test. * gfortran.dg/ISO_Fortran_binding_10.c: Called by it. PR fortran/89846 * gfortran.dg/ISO_Fortran_binding_11.f90: New test. * gfortran.dg/ISO_Fortran_binding_11.c: Called by it. PR fortran/90022 * gfortran.dg/ISO_Fortran_binding_1.c: Correct the indexing for the computation of 'ans'. Also, change the expected results for CFI_is_contiguous to comply with standard. * gfortran.dg/ISO_Fortran_binding_1.f90: Correct the expected results for CFI_is_contiguous to comply with standard. * gfortran.dg/ISO_Fortran_binding_9.f90: New test. * gfortran.dg/ISO_Fortran_binding_9.c: Called by it. 2019-04-14 Paul Thomas PR fortran/89843 * runtime/ISO_Fortran_binding.c (cfi_desc_to_gfc_desc): Only return immediately if the source pointer is null. Bring forward the extraction of the gfc type. Extract the kind so that the element size can be correctly computed for sections and components of derived type arrays. Remove the free of the CFI descriptor since this is now done in trans-expr.c. (gfc_desc_to_cfi_desc): Only allocate the CFI descriptor if it is not null. (CFI_section): Normalise the difference between the upper and lower bounds by the stride to correctly calculate the extents of the section. PR fortran/89846 * runtime/ISO_Fortran_binding.c (cfi_desc_to_gfc_desc): Use the stride measure for the gfc span if it is not a multiple of the element length. Otherwise use the element length. PR fortran/90022 * runtime/ISO_Fortran_binding.c (CFI_is_contiguous) : Return 1 for true and 0 otherwise to comply with the standard. Correct the contiguity check for rank 3 and greater by using the stride measure of the lower dimension rather than the element length. On Fri, 12 Apr 2019 at 10:08, Paul Richard Thomas wrote: > > Gilles & Reinhold, > > Following the discussion, I have decided to remove the copy-in for > intent(in). Dominique and I have located the problem with -m32 for > PR90022 and I am working on PR89846 right now. > > I will be resubmitting a bit later on. > > Cheers > > Paul > > > On Fri, 12 Apr 2019 at 01:25, Gilles Gouaillardet wrote: > > > > Reinhold, > > > > > > Thanks for the insights ! > > > > > > That means there is currently an other issue since copy-in is performed > > even if the argument is declared as ASYNCHRONOUS. > > > > > > I gave the copy-in mechanism some more thoughts, and as a library > > developers, I can clearly see a need *not* to do that > > > > on a case-by-case basis, mainly for performance reasons, but also to be > > friendly with legacy apps that are not strictly standard compliant. > > > > At this stage, I think the best way to move forward is to add an other > > directive in the interface definition. > > > > > > for example, we could declare > > > > > > module foo > > > > interface > > > > subroutine bar_f08(buf) BIND(C, name="bar_c") >
Re: [patch] Fix PR 84487, large rodata increase in tonto and other programs
Hi Thomas, Thanks for your determination in dealing with this. It has been on my TODO list for a long time but, like you at the outset, I had no idea how to deal with it. OK on the fortran side. Paul On Sat, 13 Apr 2019 at 19:48, Thomas Koenig wrote: > > Hello world, > > the attached patch fixes a 8/9 regression where _def_init, an internal > Fortran variable containing only zeros, was placed into the .rodata > section. This led to a large increase in executable size. > > There should be no impact on other languages because the change to > varasm.c is guarded by lang_GNU_Fortran (). > > Regarding the test case: I did find one other test which checks > for .bss, so I suppose this is safe. > > Regression-tested with a full test (--enable-languages=all and > make -j64 -k check) on POWER9. > > I would like to apply it to both affected branches. > > OK for the general and the Fortran part? > > Regards > > Thomas > > 2019-04-13 Thomas Koenig > > PR fortran/84487 > * trans-decl.c (gfc_get_symbol_decl): Mark _def_init as > artificial. > > 2019-04-13 Thomas Koenig > > PR fortran/84487 > * varasm.c (bss_initializer_p): If we are compiling Fortran, the > decl is artifical and it has a size larger than 255, it can be > put into BSS. > > 2019-04-13 Thomas Koenig > > PR fortran/84487 > * gfortran.dg/def_init_1.f90: New test. > > -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: AW: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.
Gilles & Reinhold, Following the discussion, I have decided to remove the copy-in for intent(in). Dominique and I have located the problem with -m32 for PR90022 and I am working on PR89846 right now. I will be resubmitting a bit later on. Cheers Paul On Fri, 12 Apr 2019 at 01:25, Gilles Gouaillardet wrote: > > Reinhold, > > > Thanks for the insights ! > > > That means there is currently an other issue since copy-in is performed > even if the argument is declared as ASYNCHRONOUS. > > > I gave the copy-in mechanism some more thoughts, and as a library > developers, I can clearly see a need *not* to do that > > on a case-by-case basis, mainly for performance reasons, but also to be > friendly with legacy apps that are not strictly standard compliant. > > At this stage, I think the best way to move forward is to add an other > directive in the interface definition. > > > for example, we could declare > > > module foo > > interface > > subroutine bar_f08(buf) BIND(C, name="bar_c") > > implicit none > > !GCC$ ATTRIBUTES NO_COPY_IN :: buf > > TYPE(*), DIMENSION(..), INTENT(IN) :: buf > > end subroutine > > end interface > > end module > > > Does this make sense ? > > > Gilles > > On 4/10/2019 4:22 PM, Bader, Reinhold wrote: > > Hi Gilles, > > > >> I also found an other potential issue with copy-in. > >> > >> If in Fortran, we > >> > >> call foo(buf(0,0)) > >> > >> then the C subroutine can only access buf(0,0), and other elements such > >> as buf(1025,1025) cannot be accessed. > >> > >> Such elements are valid in buf, but out of bounds in the copy (that > >> contains a single element). > >> > >> Strictly speaking, I cannot say whether this is a violation of the > >> standard or not, but I can see how this will > >> > >> break a lot of existing apps (once again, those apps might be incorrect > >> in the first place, but most of us got used to them working). > > > > The above call will only be conforming if the dummy argument is declared > > assumed or explicit size. > > Otherwise, the compiler should reject it due to rank mismatch. For assumed > > rank, the call would be > > legitimate, but the rank of the dummy argument is then zero. Even if no > > copy-in is performed, > > accessing data beyond the address range of that scalar is not strictly > > allowed. > > > > Of more interest is the situation where the dummy argument in Fortran is > > declared, e.g., > > > > TYPE(*), ASYNCHRONOUS, INTENT(IN) :: BUF(..) > > > > The standard's semantics *forbids* performing copy-in/out in this case, > > IIRC. > > Otherwise > > ASYNCHRONOUS semantics would not work, and non-blocking MPI calls would fail > > due > > to buffers vanishing into thin air. > > > > Regards > > Reinhold > > > >> To me, this is a second reason why copy-in is not desirable (at least as > >> a default option). > >> > >> > >> > >> Cheers, > >> > >> > >> Gilles > >> > >> On 4/9/2019 7:18 PM, Paul Richard Thomas wrote: > >>> The most part of this patch is concerned with implementing calls from > >>> C of of fortran bind c procedures with assumed shape or assumed rank > >>> dummies to completely fix PR89843. The conversion of the descriptors > >>> from CFI to gfc occur on entry to and reversed on exit from the > >>> procedure. > >>> > >>> This patch is safe for trunk, even at this late stage, because its > >>> effects are barricaded behind the tests for CFI descriptors. I believe > >>> that it appropriately rewards the bug reporters to have this feature > >>> work as well as possible at release. > >>> > >>> Between comments and the ChangeLogs, this patch is self explanatory. > >>> > >>> Bootstrapped and regtested on FC29/x86_64 - OK for trunk? > >>> > >>> Paul > >>> > >>> 2019-04-09 Paul Thomas > >>> > >>> PR fortran/89843 > >>> * trans-decl.c (gfc_get_symbol_decl): Assumed shape and assumed > >>> rank dummies of bind C procs require deferred initialization. > >>> (convert_CFI_desc): New procedure to convert incoming CFI > >>> descriptors to gfc types and back again. > >>> (gfc_trans_deferred_vars): Call it. > >>> * trans-ex
Re: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.
Hi Dominique, Yes indeed - I used int(kind(loc(res))) to achieve the same effect. I am looking for but failing to find a similar problem for PR89846. Tomorrow I turn my attention to an incorrect cast in the compiler. Regards Paul On Thu, 11 Apr 2019 at 14:54, Dominique d'Humières wrote: > > > With the following code > > module cdesc > interface > function cdesc_f08(buf, expected) result (res) BIND(C, name="cdesc_c") > USE, INTRINSIC :: ISO_C_BINDING > implicit none > INTEGER(C_INT) :: res > type(*), dimension(..), INTENT(INOUT) :: buf > type(c_ptr),INTENT(IN) :: expected > end function cdesc_f08 > end interface > end module > > program cdesc_test > USE, INTRINSIC :: ISO_C_BINDING > use cdesc > implicit none > integer(c_int), target :: a0, a1(10), a2(10,10), a3(10,10,10) > if (cdesc_f08(a0, C_LOC(a0)) .ne. 1) stop 1 > if (cdesc_f08(a1, C_LOC(a1(1))) .ne. 1) stop 2 > if (cdesc_f08(a2, C_LOC(a2(1,1))) .ne. 1) stop 3 > if (cdesc_f08(a3, C_LOC(a3(1,1,1))) .ne. 1) stop 4 > end program > > The test ISO_Fortran_binding_9.f90 executes without failure for both -m32 and > -m64. > > Dominique > > > Le 10 avr. 2019 à 00:42, Paul Richard Thomas > > a écrit : > > > > Many thanks, sir! I will look into it. > > > > Paul > -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.
Many thanks, sir! I will look into it. Paul On Tue, 9 Apr 2019 at 17:43, Dominique d'Humières wrote: > > Hi Paul, > > With your patch the test gfortran.dg/ISO_Fortran_binding_9.f90 fails in the > 32 bit mode, due to the errors > > /opt/gcc/work/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_9.f90:24:6: > Error: Type mismatch in argument 'expected' at (1); passed INTEGER(4) to > INTEGER(8) > /opt/gcc/work/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_9.f90:25:6: > Error: Type mismatch in argument 'expected' at (1); passed INTEGER(4) to > INTEGER(8) > /opt/gcc/work/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_9.f90:26:6: > Error: Type mismatch in argument 'expected' at (1); passed INTEGER(4) to > INTEGER(8) > /opt/gcc/work/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_9.f90:27:6: > Error: Type mismatch in argument 'expected' at (1); passed INTEGER(4) to > INTEGER(8) > > Note that LOC is a gnu extension. > > The patch also fixes pr89846 for -m64, but not for -m32 > > % gfc pr89846.c pr89846.f90 -m32 > % a.out > FAIL 2: > FAIL 4: > % gfc pr89846.c pr89846.f90 > % a.out > OK > OK > > TIA > > Dominique > -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
[Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.
The most part of this patch is concerned with implementing calls from C of of fortran bind c procedures with assumed shape or assumed rank dummies to completely fix PR89843. The conversion of the descriptors from CFI to gfc occur on entry to and reversed on exit from the procedure. This patch is safe for trunk, even at this late stage, because its effects are barricaded behind the tests for CFI descriptors. I believe that it appropriately rewards the bug reporters to have this feature work as well as possible at release. Between comments and the ChangeLogs, this patch is self explanatory. Bootstrapped and regtested on FC29/x86_64 - OK for trunk? Paul 2019-04-09 Paul Thomas PR fortran/89843 * trans-decl.c (gfc_get_symbol_decl): Assumed shape and assumed rank dummies of bind C procs require deferred initialization. (convert_CFI_desc): New procedure to convert incoming CFI descriptors to gfc types and back again. (gfc_trans_deferred_vars): Call it. * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Null the CFI descriptor pointer. Free the descriptor in all cases. PR fortran/90022 * trans-decl.c (gfc_get_symbol_decl): Make sure that the se expression is a pointer type before converting it to the symbol backend_decl type. 2019-04-09 Paul Thomas PR fortran/89843 * gfortran.dg/ISO_Fortran_binding_4.f90: Modify the value of x in ctg. Test the conversion of the descriptor types in the main program. * gfortran.dg/ISO_Fortran_binding_10.f90: New test. * gfortran.dg/ISO_Fortran_binding_10.c: Called by it. PR fortran/90022 * gfortran.dg/ISO_Fortran_binding_1.c: Correct the indexing for the computation of 'ans'. Also, change the expected results for CFI_is_contiguous to comply with standard. * gfortran.dg/ISO_Fortran_binding_1.f90: Correct the expected results for CFI_is_contiguous to comply with standard. * gfortran.dg/ISO_Fortran_binding_9.f90: New test. * gfortran.dg/ISO_Fortran_binding_9.c: Called by it. 2019-04-09 Paul Thomas PR fortran/89843 * runtime/ISO_Fortran_binding.c (cfi_desc_to_gfc_desc): Only return immediately if the source pointer is null. Bring forward the extraction of the gfc type. Extract the kind so that the element size can be correctly computed for sections and components of derived type arrays. Remove the free of the CFI descriptor since this is now done in trans-expr.c. (gfc_desc_to_cfi_desc): Only allocate the CFI descriptor if it is not null. (CFI_section): Normalise the difference between the upper and lower bounds by the stride to correctly calculate the extents of the section. PR fortran/90022 * runtime/ISO_Fortran_binding.c (CFI_is_contiguous) : Return 1 for true and 0 otherwise to comply with the standard. Correct the contiguity check for rank 3 and greater by using the stride measure of the lower dimension rather than the element length. Index: gcc/fortran/trans-decl.c === *** gcc/fortran/trans-decl.c (revision 270149) --- gcc/fortran/trans-decl.c (working copy) *** gfc_get_symbol_decl (gfc_symbol * sym) *** 1494,1499 --- 1494,1506 && sym->attr.dummy) gfc_defer_symbol_init (sym); + if (sym->attr.dummy + && sym->ns->proc_name->attr.is_bind_c + && sym->attr.dimension + && (sym->as->type == AS_ASSUMED_SHAPE + || sym->as->type == AS_ASSUMED_RANK)) + gfc_defer_symbol_init (sym); + /* All deferred character length procedures need to retain the backend decl, which is a pointer to the character length in the caller's namespace and to declare a local character length. */ *** gfc_null_and_pass_deferred_len (gfc_symb *** 4268,4273 --- 4275,4353 } + /* Convert CFI descriptor dummies into gfc types and back again. */ + static void + convert_CFI_desc (gfc_wrapped_block * block, gfc_symbol *sym) + { + tree gfc_desc; + tree gfc_desc_ptr; + tree CFI_desc; + tree CFI_desc_ptr; + tree dummy_ptr; + tree tmp; + tree incoming; + tree outgoing; + stmtblock_t tmpblock; + + /* dummy_ptr will be the pointer to the passed array descriptor, + while CFI_desc is the descriptor itself. */ + dummy_ptr = sym->backend_decl; + CFI_desc = sym->backend_decl; + if (POINTER_TYPE_P (TREE_TYPE (CFI_desc))) + CFI_desc = build_fold_indirect_ref_loc (input_location, CFI_desc); + if (!GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (CFI_desc))) + { + if (DECL_LANG_SPECIFIC (sym->backend_decl)) + CFI_desc = GFC_DECL_SAVED_DESCRIPTOR (sym->backend_decl); + else + CFI_desc = NULL; + dummy_ptr = CFI_desc; + } + + if (CFI_desc) + { + if (POINTER_TYPE_P (TREE_TYPE (CFI_desc))) + CFI_desc = build_fold_indirect_ref_loc (input_location, CFI_desc); + + /* The compiler will have given CFI_desc the correc
Re: [Patch, fortran] PR87127 - External function not recognised from within an associate block
Hi Dominique, I discovered the same thing myself this morning. The patch was developed on 8-branch because my working trunk had a hefty patch in an intermediate state of development. I cleared it off,ready to do the commit, and discovered the change on regtesting. I am trying to think of a way in which to distinguish the two cases. Thanks Paul On Sun, 31 Mar 2019 at 10:33, Dominique d'Humières wrote: > > Hi Paul, > > With your patch the error for the test gfortran.dg/pr88376.f90 is changed from > > Error: 'n' at (1) is not a function > > to > > Error: Specification function 'n' at (1) must be PURE > > TIA > > Dominique -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran] PR89841 - improper descriptor information passed to C
Hi Steve, Sorry about the delay. Daytime stuff caught up with me. While I was about it, I committed the fix for PR89841 with the fix for PR89842. The latter is even safer than the former. Committed as revision 270037. Thanks Paul 2019-03-30 Paul Thomas PR fortran/89841 * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Use the formal argument attributes rather than those of the actual argument. PR fortran/89842 * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Call 'set_dtype_for_unallocated' for any type of arrayspec. 2019-03-30 Paul Thomas PR fortran/89841 * gfortran.dg/ISO_Fortran_binding_1.f90: Change the interfaces for c_deallocate, c_allocate and c_assumed_size so that the attributes of the array arguments are correct and are typed. * gfortran.dg/ISO_Fortran_binding_7.f90: New test. * gfortran.dg/ISO_Fortran_binding_7.c: Additional source. PR fortran/89842 * gfortran.dg/ISO_Fortran_binding_8.f90: New test. * gfortran.dg/ISO_Fortran_binding_8.c: Additional source. On Wed, 27 Mar 2019 at 18:55, Steve Kargl wrote: > > On Wed, Mar 27, 2019 at 06:50:41PM +0000, Paul Richard Thomas wrote: > > This corrects a screw-up on my part. The attribute field of the CFI > > descriptor must be set by the formal argument in the interface and not > > the actual argument. > > > > Most of the work was in correcting > > > > Bootstrapped and regtested on FC29/x86_64 - OK for trunk? > > > > OK. > > -- > Steve -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
[Patch, fortran] PR87127 - External function not recognised from within an associate block
This patch is pretty self-explanatory. I have checked that a sensible errors are given if 'exfunc' in the testcase is referenced if it is a variable. Bootstrapped and regtested on FC29/x86_64 - OK for trunk? Paul 2019-03-30 Paul Thomas PR fortran/87127 * resolve.c (check_host_association): If an external function is typed but not declared explicitly to be external, change the old symbol from a variable to an external function. 2019-03-30 Paul Thomas PR fortran/87127 * gfortran.dg/external_procedures_4.f90: New test. Index: gcc/fortran/resolve.c === *** gcc/fortran/resolve.c (revision 269160) --- gcc/fortran/resolve.c (working copy) *** resolve_procedure: *** 5615,5625 /* Checks to see that the correct symbol has been host associated. !The only situation where this arises is that in which a twice !contained function is parsed after the host association is made. !Therefore, on detecting this, change the symbol in the expression !and convert the array reference into an actual arglist if the old !symbol is a variable. */ static bool check_host_association (gfc_expr *e) { --- 5615,5628 /* Checks to see that the correct symbol has been host associated. !The only situations where this arises are: ! (i) That in which a twice contained function is parsed after ! the host association is made. On detecting this, change ! the symbol in the expression and convert the array reference ! into an actual arglist if the old symbol is a variable; or ! (ii) That in which an external function is typed but not declared ! explicitly to be external. Here, the old symbol is changed ! from a variable to an external function. */ static bool check_host_association (gfc_expr *e) { *** check_host_association (gfc_expr *e) *** 5709,5714 --- 5712,5737 gfc_resolve_expr (e); sym->refs++; } + /* This case corresponds to a call, from a block or a contained + procedure, to an external function, which has not been declared + as being external in the main program but has been typed. */ + else if (sym && old_sym != sym + && !e->ref + && sym->ts.type == BT_UNKNOWN + && old_sym->ts.type != BT_UNKNOWN + && sym->attr.flavor == FL_PROCEDURE + && old_sym->attr.flavor == FL_VARIABLE + && sym->ns->parent == old_sym->ns + && sym->ns->proc_name + && (sym->ns->proc_name->attr.flavor == FL_LABEL + || sym->ns->proc_name->attr.flavor == FL_PROCEDURE)) + { + old_sym->attr.flavor = FL_PROCEDURE; + old_sym->attr.external = 1; + old_sym->attr.function = 1; + old_sym->result = old_sym; + gfc_resolve_expr (e); + } } /* This might have changed! */ return e->expr_type == EXPR_FUNCTION; Index: gcc/testsuite/gfortran.dg/external_procedures_4.f90 === *** gcc/testsuite/gfortran.dg/external_procedures_4.f90 (nonexistent) --- gcc/testsuite/gfortran.dg/external_procedures_4.f90 (working copy) *** *** 0 --- 1,28 + ! { dg-do run } + ! + ! Test the fix for PR87127 in which the references to exfunc cause + ! the error "‘exfunc’ at (1) is not a function". + ! + ! Contributed by Gerhard Steinmetz + ! + function exfunc(i) + implicit none + integer :: exfunc,i + exfunc = 2*i + end function + + ! contents of test.f90 + program test + implicit none + integer :: exfunc,i + integer,parameter :: array(2)=[6,7] + associate(i=>array(2))! Original bug + if (exfunc(i) .ne. 2*i) stop 1 + end associate + i = 99 + call foo + contains + subroutine foo() ! Comment #3 + if (exfunc(i) .ne. 2*i) stop 2 + end subroutine foo + end program
[Patch, fortran] PR89841 - improper descriptor information passed to C
This corrects a screw-up on my part. The attribute field of the CFI descriptor must be set by the formal argument in the interface and not the actual argument. Most of the work was in correcting Bootstrapped and regtested on FC29/x86_64 - OK for trunk? Cheers Paul 2019-03-27 Paul Thomas PR fortran/89841 * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Use the formal argument attributes rather than those of the actual argument. 2019-03-27 Paul Thomas PR fortran/89841 * gfortran.dg/ISO_Fortran_binding_1.f90: Change the interfaces for c_deallocate, c_allocate and c_assumed_size so that the attributes of the array arguments are correct and are typed. * gfortran.dg/ISO_Fortran_binding_7.f90: New test. * gfortran.dg/ISO_Fortran_binding_7.c: Additional source. Index: gcc/fortran/trans-expr.c === *** gcc/fortran/trans-expr.c (revision 269962) --- gcc/fortran/trans-expr.c (working copy) *** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p *** 4998,5006 attribute = 2; if (!e->rank || gfc_get_full_arrayspec_from_expr (e)) { ! if (attr.pointer) attribute = 0; ! else if (attr.allocatable) attribute = 1; } --- 4998,5006 attribute = 2; if (!e->rank || gfc_get_full_arrayspec_from_expr (e)) { ! if (fsym->attr.pointer) attribute = 0; ! else if (fsym->attr.allocatable) attribute = 1; } Index: gcc/testsuite/gfortran.dg/ISO_Fortran_binding_1.f90 === *** gcc/testsuite/gfortran.dg/ISO_Fortran_binding_1.f90 (revision 269961) --- gcc/testsuite/gfortran.dg/ISO_Fortran_binding_1.f90 (working copy) *** *** 25,37 FUNCTION c_deallocate(a) BIND(C, NAME="deallocate_c") RESULT(err) USE, INTRINSIC :: ISO_C_BINDING INTEGER(C_INT) :: err ! type(*), DIMENSION(..) :: a END FUNCTION c_deallocate FUNCTION c_allocate(a, lower, upper) BIND(C, NAME="allocate_c") RESULT(err) USE, INTRINSIC :: ISO_C_BINDING INTEGER(C_INT) :: err ! type(*), DIMENSION(..) :: a integer(C_INTPTR_T), DIMENSION(15) :: lower, upper END FUNCTION c_allocate --- 25,37 FUNCTION c_deallocate(a) BIND(C, NAME="deallocate_c") RESULT(err) USE, INTRINSIC :: ISO_C_BINDING INTEGER(C_INT) :: err ! INTEGER(C_INT), DIMENSION(..), allocatable :: a END FUNCTION c_deallocate FUNCTION c_allocate(a, lower, upper) BIND(C, NAME="allocate_c") RESULT(err) USE, INTRINSIC :: ISO_C_BINDING INTEGER(C_INT) :: err ! INTEGER(C_INT), DIMENSION(..), allocatable :: a integer(C_INTPTR_T), DIMENSION(15) :: lower, upper END FUNCTION c_allocate *** *** 67,73 USE, INTRINSIC :: ISO_C_BINDING INTEGER(C_INT) :: err INTEGER(C_INT), dimension(2) :: lbounds ! type(*), DIMENSION(..) :: a END FUNCTION c_setpointer FUNCTION c_assumed_size(a) BIND(C, NAME="assumed_size_c") RESULT(err) --- 67,73 USE, INTRINSIC :: ISO_C_BINDING INTEGER(C_INT) :: err INTEGER(C_INT), dimension(2) :: lbounds ! INTEGER(C_INT), DIMENSION(..), pointer :: a END FUNCTION c_setpointer FUNCTION c_assumed_size(a) BIND(C, NAME="assumed_size_c") RESULT(err) Index: gcc/testsuite/gfortran.dg/ISO_Fortran_binding_7.c === *** gcc/testsuite/gfortran.dg/ISO_Fortran_binding_7.c (nonexistent) --- gcc/testsuite/gfortran.dg/ISO_Fortran_binding_7.c (working copy) *** *** 0 --- 1,102 + /* Test the fix for PR89841. */ + + /* Contributed by Reinhold Bader */ + + #include "../../../libgfortran/ISO_Fortran_binding.h" + #include + #include + #include + + typedef struct + { + int i; + float r[2]; + } cstruct; + + + int Psuba(CFI_cdesc_t *this, CFI_cdesc_t *that, int Dcase) { + int status = 0; + cstruct *cu; + float *ct; + CFI_dim_t *dim; + if (this->elem_len != sizeof(float)) + { + printf("FAIL: Dcase %i - this->elem_len %i\n",Dcase, (int) this->elem_len); + status++; + } + if (this->type != CFI_type_float) + { + printf("FAIL: Dcase %i - this->type\n", Dcase); + status++; + } + if (this->rank != 2) + { + printf("FAIL: Dcase %i - this->rank %i\n",Dcase,this->rank); + status++; + } + if (this->attribute != CFI_attribute_other) + { + printf("FAIL: Dcase %i - this->attribute\n", Dcase); + status++; + } + + dim = this->dim; + if (dim[0].lower_bound != 0 || dim[0].extent != 3) + { + printf("FAIL: Dcase %i - dim[0] %i %i %i\n",Dcase, (int) dim[0].lower_bound, + (int)dim[0].extent,(int)dim[0].sm); + status++; + } + if (dim[1].lower_bound != 0 || dim[1].extent != 7) + { + printf("FAIL:
Re: Further bugs in extended C interop
Hi Reinhold, Many thanks - the bug number is out by one on the last. Jakub Jelnik got one in before it :-) I'll take a look this afternoon. Cheers Paul On Wed, 27 Mar 2019 at 10:25, Bader, Reinhold wrote: > > Dear Paul, > > Here are further bug reports, mostly on the various functions for manipulating > descriptors: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89841 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89842 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89843 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89844 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89845 > > Regards > > Reinhold -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
[Patch, fortran] PR88247 - [8/9 Regression] ICE in get_array_ctor_var_strlen, at fortran/trans-array.c:2068
This one started with a simple enough testscase and then evolved as I found out how little actually worked. All the changes in the patch involve gathering up the string length by hook or by crook. It will be noted that associate (y => x%d(:)(2:4)) still does not work correctly. The associate name is correctly formed with the right values of element length and span. However, it is unusable because the array indexing uses the element length as the span. I did not want to touch array indexing at this stage of 9-branch. Bootstraps and regtests on FC29/x86_64 - OK for 8- and 9-branches? Paul 2019-03-24 Paul Thomas PR fortran/88247 * expr.c (is_subref_array): Permit substrings to be detected as subref arrays. * trans-array.c (get_array_ctor_var_strlen): Obtain the length of deferred length strings. Handle substrings with a NULL end expression. (trans_array_constructor): Remove an unnecessary blank line. (gfc_conv_scalarized_array_ref): Skip to label 'done' if 'decl' is a pointer array. (get_array_charlen): If the expression is an array, convert the first element of the constructor and use its string length. Get a new charlen if necessary. (gfc_conv_expr_descriptor): Call 'get_array_charlen' for array constructor expressions. If the ss_info string length is available, use that to set the span of character arrays. * trans-expr.c (gfc_get_expr_charlen): Handle substrings * trans-stmt.c (trans_associate_var): Set the pointer array flag for variable targets and constant array constructors. Take care not to reset the string length or the span in the case of expressions that are not converted as direct by reference. 2019-03-24 Paul Thomas PR fortran/88247 * gfortran.dg/associate_47.f90: New test. Index: gcc/fortran/expr.c === *** gcc/fortran/expr.c (revision 269611) --- gcc/fortran/expr.c (working copy) *** is_subref_array (gfc_expr * e) *** 1077,1084 for (ref = e->ref; ref; ref = ref->next) { /* If we haven't seen the array reference and this is an intrinsic, ! what follows cannot be a subreference array. */ if (!seen_array && ref->type == REF_COMPONENT && ref->u.c.component->ts.type != BT_CLASS && !gfc_bt_struct (ref->u.c.component->ts.type)) return false; --- 1077,1086 for (ref = e->ref; ref; ref = ref->next) { /* If we haven't seen the array reference and this is an intrinsic, ! what follows cannot be a subreference array, unless there is a ! substring reference. */ if (!seen_array && ref->type == REF_COMPONENT + && ref->u.c.component->ts.type != BT_CHARACTER && ref->u.c.component->ts.type != BT_CLASS && !gfc_bt_struct (ref->u.c.component->ts.type)) return false; Index: gcc/fortran/trans-array.c === *** gcc/fortran/trans-array.c (revision 269611) --- gcc/fortran/trans-array.c (working copy) *** get_array_ctor_var_strlen (stmtblock_t * *** 2099,2104 --- 2099,2106 { case REF_ARRAY: /* Array references don't change the string length. */ + if (ts->deferred) + get_array_ctor_all_strlen (block, expr, len); break; case REF_COMPONENT: *** get_array_ctor_var_strlen (stmtblock_t * *** 2107,2113 break; case REF_SUBSTRING: ! if (ref->u.ss.start->expr_type != EXPR_CONSTANT || ref->u.ss.end->expr_type != EXPR_CONSTANT) { /* Note that this might evaluate expr. */ --- 2109,2116 break; case REF_SUBSTRING: ! if (ref->u.ss.end == NULL ! || ref->u.ss.start->expr_type != EXPR_CONSTANT || ref->u.ss.end->expr_type != EXPR_CONSTANT) { /* Note that this might evaluate expr. */ *** trans_array_constructor (gfc_ss * ss, lo *** 2507,2513 ss_info->string_length); ss_info->string_length = gfc_evaluate_now (ss_info->string_length, &length_se.pre); - gfc_add_block_to_block (&outer_loop->pre, &length_se.pre); gfc_add_block_to_block (&outer_loop->post, &length_se.post); } --- 2510,2515 *** gfc_conv_scalarized_array_ref (gfc_se * *** 3470,3475 --- 3472,3480 || expr->expr_type == EXPR_FUNCTION decl = expr->symtree->n.sym->backend_decl; + if (decl && GFC_DECL_PTR_ARRAY_P (decl)) + goto done; + /* A pointer array component can be detected from its field decl. Fix the descriptor, mark the resulting variable decl and pass it to gfc_build_array_ref. */ *** gfc_conv_scalarized_array_ref (gfc_se * *** 3486,3491 --- 3491,3497 decl = info->descriptor; } + done: se->expr = gfc_build_array_ref (base, index, decl); } *** get_array_charlen (gfc_expr *expr, gfc_s *** 6929,6934 --- 6935,6941 ---
Re: [patch, fortran] Fix PR 78865, ICE on invalid
Hi Thomas, This is OK for trunk and, after a wee delay for the other active branches. Thanks Paul On Fri, 22 Mar 2019 at 22:56, Thomas Koenig wrote: > > Hello world, > > the attached patch fixes a 7/8/9 regression. The problem was twofold: > If a subroutine was called more than once from a different subroutine, > the call was only checked the first time. Also, a type change in the > backend_decl initiated when there was already a declaration led to an > ICE. > > The solution also has two parts: Make sure that a hard error is > delivered in this case, and make sure the check is done every time. > > Regression-tested. OK for trunk and other affected branches? > > Regards > > Thomas > > 2019-03-22 Thomas Koenig > > PR fortran/78865 > * interface.c (compare_actual_formal): Change errors about > missing or extra to gfc_error_now to make sure they are issued. > Change "spec" to "specifier" in message. > * resolve.c (resolve_global_procedure): Also check for mismatching > interface with global symbols if the namespace has already been > resolved. > > 2019-03-22 Thomas Koenig > > PR fortran/78865 > * gfortran.dg/altreturn_10.f90: New test. > * gfortran.dg/whole_file_3.f90: Change dg-warning to dg-error. -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
[Patch, fortran] PRs 89363 and 89364 - wrinkles with assumed rank
The attached patch implements fixes for a couple of wrinkles with assumed rank: (i) PR89363 flagged the fact that the rank was not be set for assumed rank entities, associated with unallocated or unassociated arrays. The fix is straightforward, the work being done by trans-expr.c(set_dtype_for_unallocated). (ii) PR89364 points out that ubound and shape should be returning -1 for the final dimension and not 0, for assumed rank entities argument associated with assumed size arrays. The fix for this is bolted on to both intrinsics. BTW lbound seems lack a bit of fixing of intermediate expressions - the result is often overwhelmingly complicated. Is there some reason for this? Bootstrapped and regtested on FC29/x86_64 - OK for trunk? Paul PS I scoped out the select rank construct. It looks well doable but will have to await 10-branch opening. 2019-03-10 Paul Thomas PR fortran/89363 PR fortran/89364 * trans-expr.c (set_dtype_for_unallocated): New function. (gfc_conv_gfc_desc_to_cfi_desc): Call it for allocatable and pointer arguments. (gfc_conv_procedure_call): Likewise. Also, set the ubound of the final dimension to -1 for assumed rank formal args that are associated with assumed size arrays. * trans-intrinsic.c (gfc_conv_intrinsic_bound): Return -1 for the final dimension of assumed rank entities that are argument associated with assumed size arrays. (gfc_conv_intrinsic_shape): Likewise return -1 for the final dimension of the shape intrinsic. 2019-03-10 Paul Thomas PR fortran/89363 * gfortran.dg/assumed_rank_16.f90: New test. PR fortran/89364 * gfortran.dg/assumed_rank_17.f90: New test. Index: gcc/fortran/trans-expr.c === *** gcc/fortran/trans-expr.c (revision 269523) --- gcc/fortran/trans-expr.c (working copy) *** expr_may_alias_variables (gfc_expr *e, b *** 4919,4924 --- 4919,4970 } + /* A helper function to set the dtype for unallocated or unassociated +entities. */ + + static void + set_dtype_for_unallocated (gfc_se *parmse, gfc_expr *e) + { + tree tmp; + tree desc; + tree cond; + tree type; + stmtblock_t block; + + /* TODO Figure out how to handle optional dummies. */ + if (e && e->expr_type == EXPR_VARIABLE + && e->symtree->n.sym->attr.optional) + return; + + desc = parmse->expr; + if (desc == NULL_TREE) + return; + + if (POINTER_TYPE_P (TREE_TYPE (desc))) + desc = build_fold_indirect_ref_loc (input_location, desc); + + if (!GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (desc))) + return; + + gfc_init_block (&block); + tmp = gfc_conv_descriptor_data_get (desc); + cond = fold_build2_loc (input_location, EQ_EXPR, + logical_type_node, tmp, + build_int_cst (TREE_TYPE (tmp), 0)); + tmp = gfc_conv_descriptor_dtype (desc); + type = gfc_get_element_type (TREE_TYPE (desc)); + tmp = fold_build2_loc (input_location, MODIFY_EXPR, + TREE_TYPE (tmp), tmp, + gfc_get_dtype_rank_type (e->rank, type)); + gfc_add_expr_to_block (&block, tmp); + cond = build3_v (COND_EXPR, cond, + gfc_finish_block (&block), + build_empty_stmt (input_location)); + gfc_add_expr_to_block (&parmse->pre, cond); + } + + + /* Provide an interface between gfortran array descriptors and the F2018:18.4 ISO_Fortran_binding array descriptors. */ *** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p *** 4958,4963 --- 5004,5018 parmse->expr = build_fold_indirect_ref_loc (input_location, parmse->expr); + /* Unallocated allocatable arrays and unassociated pointer arrays + need their dtype setting if they are argument associated with + assumed rank dummies. */ + if (fsym && fsym->as + && fsym->as->type == AS_ASSUMED_RANK + && (gfc_expr_attr (e).pointer + || gfc_expr_attr (e).allocatable)) + set_dtype_for_unallocated (parmse, e); + /* All the temporary descriptors are marked as DECL_ARTIFICIAL. If the expression type is different from the descriptor type, then the offset must be found (eg. to a component ref or substring) *** gfc_conv_procedure_call (gfc_se * se, gf *** 5953,5958 --- 6008,6037 gfc_conv_array_parameter (&parmse, e, nodesc_arg, fsym, sym->name, NULL); + /* Unallocated allocatable arrays and unassociated pointer arrays + need their dtype setting if they are argument associated with + assumed rank dummies. */ + if (!sym->attr.is_bind_c && e && fsym && fsym->as + && fsym->as->type == AS_ASSUMED_RANK) + { + if (gfc_expr_attr (e).pointer + || gfc_expr_attr (e).allocatable) + set_dtype_for_unallocated (&parmse, e); + else if (e->expr_type == EXPR_VARIABLE + && e->symtree->n.sym->attr.dummy + && e->symtree->n.sym->as + && e->symtree->n.sym->as->type == AS_ASSUMED_SIZE) + { + tree mi
Re: [patch, fortran] Fix PR 72714, ICE on invalid
Hi Thomas, This is good for trunk. Thanks Paul On Sun, 3 Mar 2019 at 09:46, Thomas Koenig wrote: > > Hello world, > > the attached patch fixes a 7/8/9 regression by rejecting an invalid > expression in coarray allocation that led to an ICE. It also adds a few > more checks. > > One point that is checked for is that, unlike normal arrays, coarrays > cannot be empty. > > Regression-tested. OK for trunk and affected branches? > > Regards > > Thomas > > 2019-03-02 Thomas Koenig > > > > PR fortran/72714 > > * resolve.c (resolve_allocate_expr): Add some tests for > coarrays. > > > 2019-03-02 Thomas Koenig > > > > PR fortran/72714 > > * gfortran.dg/coarray_allocate_11.f90: New test. -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [PATCH] Use RANGE_EXPR in Fortran array initializers some more (PR fortran/43210)
Hi Jakub, Your timing is astonishing. This was next on my list of TODOs - not for this particular PR but to deal with the rodata bloat eg. 84487. I presume that this patch will make the latter go away? Yes, this is good for trunk and, if it fixes 84487, 8-branch as well. Thanks Paul On Mon, 25 Feb 2019 at 23:02, Jakub Jelinek wrote: > > Hi! > > When initializing whole array with a const, we can save quite some compile > time memory (and time in some cases) by using RANGE_EXPRs, instead of > duplicating the same initializer thousands of times in the CONSTRUCTOR. > In some cases the gimplifier even can optimize those better. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2019-02-25 Jakub Jelinek > > PR fortran/43210 > * trans-array.c (gfc_conv_array_initializer): Use RANGE_EXPR instead > of duplicating the initializer possibly many times. > > --- gcc/fortran/trans-array.c.jj2019-02-10 12:05:42.753718023 +0100 > +++ gcc/fortran/trans-array.c 2019-02-25 18:11:44.948166509 +0100 > @@ -5986,7 +5986,6 @@ gfc_conv_array_initializer (tree type, g > { >gfc_constructor *c; >tree tmp; > - offset_int wtmp; >gfc_se se; >tree index, range; >vec *v = NULL; > @@ -6009,13 +6008,10 @@ gfc_conv_array_initializer (tree type, g >else > gfc_conv_structure (&se, expr, 1); > > - wtmp = wi::to_offset (TYPE_MAX_VALUE (TYPE_DOMAIN (type))) + 1; > - /* This will probably eat buckets of memory for large arrays. */ > - while (wtmp != 0) > -{ > - CONSTRUCTOR_APPEND_ELT (v, NULL_TREE, se.expr); > - wtmp -= 1; > -} > + CONSTRUCTOR_APPEND_ELT (v, build2 (RANGE_EXPR, gfc_array_index_type, > +TYPE_MIN_VALUE (TYPE_DOMAIN (type)), > +TYPE_MAX_VALUE (TYPE_DOMAIN (type))), > + se.expr); >break; > > case EXPR_ARRAY: > > Jakub -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [patch, fortran] Fix PR 89174, segfault on allocate with MOLD
Hi Thomas, > Of course, there could also be a more profound way of fixing this > bug :-) I wish that it were the case. I am unable to decide whether the design choices made for an F95 compiler have cause the add-ons to be intrinsically complicated or whether or whether F2018, extensions and legacy support is the cause of the complexity - probably both. Either way, profundity is rarely to be found in bugfixes. > > Regression-tested on x86_64-pc-linux-gnu. OK for trunk and gcc-8? Yes, that's good for both branches. Thanks for the fix. Paul
[Patch, fortran] PR88117 - [9 Regression] ICE in gimplify_var_or_parm_decl, at gimplify.c:2697
This one was sufficiently 'obvious' that not only has it been committed to 9-branch but to 7- and 8-branches as well. Although the choking gimplifier was a regression, the testcase never worked on any branch because resolve.c:deferred_op_assign should always have returned false for pointer expressions, since the reallocate on assign cannot be used. The part of the patch intrans-expr.c fixed the regression. Committed in revisions 269157, 269160 & 269163. Paul 2019-02-23 Paul Thomas PR fortran/88117 * resolve.c (deferred_op_assign): Return if the lhs expression has the pointer attribute. * trans-expr.c (gfc_trans_assignment_1): Do not fix the string length if the lhs expression has the pointer attribute. 2019-02-23 Paul Thomas PR fortran/88117 * gfortran.dg/deferred_character_32.f90 : New test
Re: [patch, fortran] Fix PR 89384
Hi Thomas, That's just the job. OK for trunk. Thanks for the quick fix. Paul On Mon, 18 Feb 2019 at 22:03, Thomas Koenig wrote: > > Hello world, > > this patch fixes the 9 regression in C interop with contiguous > arguments recently reported by Reinhold Bader. > > ChangeLog and patch say it all. I hope I didn't overlook any > obvious things here (Paul, maybe you can take a look). > > Regression-tested. OK for trunk? > > Regards > > Thomas > > 2019-02-18 Thomas Koenig > > PR fortran/89384 > * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): If the dummy > argument is contiguous and the actual argument may not be, > use gfc_conv_subref_array_arg. > > 2019-02-18 Thomas Koenig > > PR fortran/89384 > * gfortran.dg/ISO_Fortran_binding_4.f90 -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [patch, fortran] Fix PR 71237
OK. Thanks for the patch. Paul On Wed, 6 Feb 2019 at 20:27, Thomas Koenig wrote: > > Hello world, > > this patch fixes a 7/8/9 regression where we tried to accept invalid > code, which led to an ICE later on. > > The patch is rather straightforward. The reason why I could not > use gfc_expr_attr is that it does not actually return the > flags the way they can be found in the original attributes; > for example, an expression containing a pointer attribute is > shown as having the target attribute, for reasons I cannot > fathom. > > Regression-tested. OK for trunk and other open branches? > > Regards > > Thomas > > 2019-02-06 Thomas Koenig > > PR fortran/71237 > * expr.c (gfc_check_assign): Add argument is_init_expr. If we are > looking at an init expression, issue error if the target is not a > TARGET and we are not looking at a procedure pointer. > * gfortran.h (gfc_check_assign): Add optional argument > is_init_expr. > > 2019-02-06 Thomas Koenig > > PR fortran/71237 > * gfortran.dg/pointer_init_2.f90: Adjust error messages. > * gfortran.dg/pointer_init_6.f90: Likewise. > * gfortran.dg/pointer_init_9.f90: New test. -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
[Patch, fortran] PR89200 - [9 Regression] Erroneous copying of a derived type with a deferred-length character array component
Committed as 'obvious' in revision 268721 after bootstrapping and regtesting. Even if not entirely obvious to the world at large, the patch is, in the words of Hitch Hikers Guide to the Galaxy, "mostly harmless". To explain the 'obviousness': Array and structure constructors make use of temporary descriptors, which are sometimes copied into the destination. Array referencing has been tightened up so that more use is being made of pointer arithmetic involving the 'span' field to obtain the stride measure. This is especially important in the case of references to components of derived type array elements, as in the testcase. It should be noted that the testcase leaks memory as in PR38319. Since it is an especially clear case, I might take a quick peek to see if I can fix this PR at long last. Paul 2019-02-09 Paul Thomas PR fortran/89200 * trans-array.c (gfc_trans_create_temp_array): Set the 'span' field for derived types. 2019-02-09 Paul Thomas PR fortran/89200 * gfortran.dg/array_reference_2.f90 : New test.
Re: [PR fortran/89077, patch] - ICE using * as len specifier for character parameter
Hi Harald, After nearly 12 years, I have been found out! OK for trunk. Since it has been such a long time, I suggest that you limit your backporting efforts to 8-branch and, at the most, 7-branch. Will you attempt to tackle the other issues in the PR? Thanks Paul On Sun, 3 Feb 2019 at 21:04, Harald Anlauf wrote: > > The attached patch fixes an ICE-on-valid that probably goes back to > rev.128130. Apparently the patch applied back then did not check > this code path which resulted in a NULL pointer dereference. This > is remedied by the new testcase base on comment #0 in this PR. > > The PR mentions another wrong-code issue to be addressed separately. > > OK for trunk? And shall this fix be backported? > > Thanks, > Harald > > 2019-02-03 Harald Anlauf > > PR fortran/89077 > * decl.c (add_init_expr_to_sym): Copy length of string initializer > to declared symbol. > > 2019-02-03 Harald Anlauf > > PR fortran/89077 > * gfortran.dg/pr89077.f90: New test. > -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [patch, fortran] Fix PR 88298
OK - thanks for the patch. Paul On Sat, 2 Feb 2019 at 14:41, Thomas Koenig wrote: > > Hi, > > the attached patch fixes a 7/8/9 regression where a conversion warning > was emitted for DIM. The problem was that the no-warn flag had not been > passed down to the arithmetic conversion routines, which is solved here > by adding and using a flag in gfc_expr. > > Regression-tested. OK for affected branches? > > Regards > > Thomas > > 2019-02-02 Thomas Koenig > > PR fortran/88298 > * arith.c (gfc_int2int): Do not warn if src->do_not_warn is set. > * gfortran.h (gfc_expr): Add flag do_not_warn. > * intrinsic.c (gfc_convert_type_warn): Set expr->do_not_warn if > no warning is desired. > > 2019-02-02 Thomas Koenig > > PR fortran/88298 > * gfortran.dg/warn_conversion_10.f90: New test. -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran] PR88393 - [7/8/9 Regression] [OOP] Segfault with type-bound assignment
Unfortunately, it doesn't. I have taken it though since it should pretty low hanging fruit. Cheers Paul On Fri, 1 Feb 2019 at 19:31, Steve Kargl wrote: > > On Fri, Feb 01, 2019 at 06:15:21PM +, Paul Richard Thomas wrote: > > I will commit this patch as 'obvious' tomorrow. > > > > Cheers > > > > Paul > > > > 2019-02-01 Paul Thomas > > > > PR fortran/88393 > > * trans-expr.c (gfc_conv_procedure_call): For derived entities, > > passed in parentheses to class formals, invert the order of > > copying allocatable components to taking taking the _data of > > the class expression. > > > > 2019-02-01 Paul Thomas > > > > PR fortran/88393 > > * gfortran.dg/alloc_comp_assign_16.f03 : New test. > > Paul, > > Does this patch also fix PR57710? > > -- > Steve -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran] PR88393 - [7/8/9 Regression] [OOP] Segfault with type-bound assignment
Hi Steve, > taking taking > OK OK > > Index: gcc/fortran/trans-expr.c > > === > > *** gcc/fortran/trans-expr.c (revision 268231) > > --- gcc/fortran/trans-expr.c (working copy) > > *** gfc_conv_procedure_call (gfc_se * se, gf > > *** 6042,6047 > > --- 6042,6057 > > break; > > } > > > > + if (e->ts.type == BT_DERIVED && fsym && fsym->ts.type == BT_CLASS) > > + { > > + /* The derived type is passed to gfc_deallocate_alloc_comp. > > + Therefore, class actuals can handled correctly but derived > > s/can handled/can be handled/ Thanks - in the original of course but I should have spotted it. Thanks for the review. Paul
[Patch, fortran] PR88393 - [7/8/9 Regression] [OOP] Segfault with type-bound assignment
I will commit this patch as 'obvious' tomorrow. Cheers Paul 2019-02-01 Paul Thomas PR fortran/88393 * trans-expr.c (gfc_conv_procedure_call): For derived entities, passed in parentheses to class formals, invert the order of copying allocatable components to taking taking the _data of the class expression. 2019-02-01 Paul Thomas PR fortran/88393 * gfortran.dg/alloc_comp_assign_16.f03 : New test. Index: gcc/fortran/trans-expr.c === *** gcc/fortran/trans-expr.c (revision 268231) --- gcc/fortran/trans-expr.c (working copy) *** gfc_conv_procedure_call (gfc_se * se, gf *** 6042,6047 --- 6042,6057 break; } + if (e->ts.type == BT_DERIVED && fsym && fsym->ts.type == BT_CLASS) + { + /* The derived type is passed to gfc_deallocate_alloc_comp. + Therefore, class actuals can handled correctly but derived + types passed to class formals need the _data component. */ + tmp = gfc_class_data_get (tmp); + if (!CLASS_DATA (fsym)->attr.dimension) + tmp = build_fold_indirect_ref_loc (input_location, tmp); + } + if (e->expr_type == EXPR_OP && e->value.op.op == INTRINSIC_PARENTHESES && e->value.op.op1->expr_type == EXPR_VARIABLE) *** gfc_conv_procedure_call (gfc_se * se, gf *** 6053,6068 gfc_add_expr_to_block (&se->post, local_tmp); } - if (e->ts.type == BT_DERIVED && fsym && fsym->ts.type == BT_CLASS) - { - /* The derived type is passed to gfc_deallocate_alloc_comp. - Therefore, class actuals can handled correctly but derived - types passed to class formals need the _data component. */ - tmp = gfc_class_data_get (tmp); - if (!CLASS_DATA (fsym)->attr.dimension) - tmp = build_fold_indirect_ref_loc (input_location, tmp); - } - if (!finalized && !e->must_finalize) { if ((e->ts.type == BT_CLASS --- 6063,6068 Index: gcc/testsuite/gfortran.dg/alloc_comp_assign_16.f03 === *** gcc/testsuite/gfortran.dg/alloc_comp_assign_16.f03 (nonexistent) --- gcc/testsuite/gfortran.dg/alloc_comp_assign_16.f03 (working copy) *** *** 0 --- 1,37 + ! { dg-do run } + ! + ! Test the fix for PR88393 in which a segfault occurred as indicated. + ! + ! Contributed by Janus Weil + ! + module m +implicit none +type :: t + character(len=:), allocatable :: cs +contains + procedure :: ass + generic :: assignment(=) => ass +end type + contains +subroutine ass(a, b) + class(t), intent(inout) :: a + class(t), intent(in):: b + a%cs = b%cs + print *, "ass" +end subroutine + end module + + program p +use m +implicit none +type :: t2 + type(t) :: c +end type +type(t2), dimension(1:2) :: arr +arr(1)%c%cs = "abcd" +arr(2)%c = arr(1)%c ! Segfault here. +print *, "done", arr(2)%c%cs, arr(2)%c%cs + ! Make sure with valgrind that there are no memory leaks. +deallocate (arr(1)%c%cs) +deallocate (arr(2)%c%cs) + end
[Patch, fortran] PR88980 - [9 regression] segfault on allocatable string member assignment
This patch is rather simpler than it looks. The segfault was occurring because r264724 changed the array reference for cases like these to use pointer arithmetic to obtain the element. Unfortunately, in the case, the span field of the descriptor was not being set during the allocation of the component items. The ChangeLog adequately explains the fix and results in the span field being set unconditionally. Bootstrapped and regtested on FC28/x86_64 - OK for trunk? Paul 2019-02-01 Paul Thomas PR fortran/88980 * trans-array.c (gfc_array_init_size): Add element_size to the arguments. (gfc_array_allocate): Remove the recalculation of the size of the element and use element_size from the call to the above. Unconditionally set the span field of the descriptor. 2019-02-01 Paul Thomas PR fortran/88980 * gfortran.dg/realloc_on_assign_32.f90 : New test. Index: gcc/fortran/trans-array.c === *** gcc/fortran/trans-array.c (revision 268231) --- gcc/fortran/trans-array.c (working copy) *** gfc_array_init_size (tree descriptor, in *** 5370,5383 gfc_expr ** lower, gfc_expr ** upper, stmtblock_t * pblock, stmtblock_t * descriptor_block, tree * overflow, tree expr3_elem_size, tree *nelems, gfc_expr *expr3, ! tree expr3_desc, bool e3_has_nodescriptor, gfc_expr *expr) { tree type; tree tmp; tree size; tree offset; tree stride; - tree element_size; tree or_expr; tree thencase; tree elsecase; --- 5370,5383 gfc_expr ** lower, gfc_expr ** upper, stmtblock_t * pblock, stmtblock_t * descriptor_block, tree * overflow, tree expr3_elem_size, tree *nelems, gfc_expr *expr3, ! tree expr3_desc, bool e3_has_nodescriptor, gfc_expr *expr, ! tree *element_size) { tree type; tree tmp; tree size; tree offset; tree stride; tree or_expr; tree thencase; tree elsecase; *** gfc_array_init_size (tree descriptor, in *** 5628,5637 tmp = TYPE_SIZE_UNIT (gfc_get_element_type (type)); /* Convert to size_t. */ ! element_size = fold_convert (size_type_node, tmp); if (rank == 0) ! return element_size; *nelems = gfc_evaluate_now (stride, pblock); stride = fold_convert (size_type_node, stride); --- 5628,5637 tmp = TYPE_SIZE_UNIT (gfc_get_element_type (type)); /* Convert to size_t. */ ! *element_size = fold_convert (size_type_node, tmp); if (rank == 0) ! return *element_size; *nelems = gfc_evaluate_now (stride, pblock); stride = fold_convert (size_type_node, stride); *** gfc_array_init_size (tree descriptor, in *** 5641,5654 dividing. */ tmp = fold_build2_loc (input_location, TRUNC_DIV_EXPR, size_type_node, ! TYPE_MAX_VALUE (size_type_node), element_size); cond = gfc_unlikely (fold_build2_loc (input_location, LT_EXPR, logical_type_node, tmp, stride), PRED_FORTRAN_OVERFLOW); tmp = fold_build3_loc (input_location, COND_EXPR, integer_type_node, cond, integer_one_node, integer_zero_node); cond = gfc_unlikely (fold_build2_loc (input_location, EQ_EXPR, ! logical_type_node, element_size, build_int_cst (size_type_node, 0)), PRED_FORTRAN_SIZE_ZERO); tmp = fold_build3_loc (input_location, COND_EXPR, integer_type_node, cond, --- 5641,5654 dividing. */ tmp = fold_build2_loc (input_location, TRUNC_DIV_EXPR, size_type_node, ! TYPE_MAX_VALUE (size_type_node), *element_size); cond = gfc_unlikely (fold_build2_loc (input_location, LT_EXPR, logical_type_node, tmp, stride), PRED_FORTRAN_OVERFLOW); tmp = fold_build3_loc (input_location, COND_EXPR, integer_type_node, cond, integer_one_node, integer_zero_node); cond = gfc_unlikely (fold_build2_loc (input_location, EQ_EXPR, ! logical_type_node, *element_size, build_int_cst (size_type_node, 0)), PRED_FORTRAN_SIZE_ZERO); tmp = fold_build3_loc (input_location, COND_EXPR, integer_type_node, cond, *** gfc_array_init_size (tree descriptor, in *** 5658,5664 *overflow = gfc_evaluate_now (tmp, pblock); size = fold_build2_loc (input_location, MULT_EXPR, size_type_node, ! stride, element_size); if (poffset != NULL) { --- 5658,5664 *overflow = gfc_evaluate_now (tmp, pblock); size = fold_build2_loc (input_location, MULT_EXPR, size_type_node, ! stride, *element_size); if (poffset != NULL) { *** gfc_array_allocate (gfc_se * se, gfc_exp *** 5736,5741 --- 5736,5742 tree var_overflow = NULL_TREE; tree cond; tree set_descriptor; + tree element_size = NULL_TREE; stmtblock_t set_descriptor_block; stmtblock_t elseblock; gfc_expr **lower; *** gfc_array_allocate (gfc_
Re: [Patch, fortran] PR88685 - [8/9 regression] pointer class array argument indexing
Sorry about the premature 'send'. This one is more or less obvious and is described in the ChangeLog. The key point is that full or section array references to intrinsic components were returning a false true from expr.c (is_subref_array). Returning false if a component is intrinsic and following anything other than an array element is an obvious remedy. Bootstrapped and regtested on FC28/x86_64 - OK for trunk and 8-branch? Paul 2019-01-30 Paul Thomas PR fortran/88685 * expr.c (is_subref_array): Move the check for class pointer dummy arrays to after the reference check. If we haven't seen an array reference other than an element and a component is not class or derived, return false. 2019-01-30 Paul Thomas PR fortran/88685 * gfortran.dg/pointer_array_component_3.f90 : New test. Index: gcc/fortran/expr.c === *** gcc/fortran/expr.c (revision 268230) --- gcc/fortran/expr.c (working copy) *** is_subref_array (gfc_expr * e) *** 1072,1086 if (e->symtree->n.sym->attr.subref_array_pointer) return true; - if (e->symtree->n.sym->ts.type == BT_CLASS - && e->symtree->n.sym->attr.dummy - && CLASS_DATA (e->symtree->n.sym)->attr.dimension - && CLASS_DATA (e->symtree->n.sym)->attr.class_pointer) - return true; - seen_array = false; for (ref = e->ref; ref; ref = ref->next) { if (ref->type == REF_ARRAY && ref->u.ar.type != AR_ELEMENT) seen_array = true; --- 1072,1086 if (e->symtree->n.sym->attr.subref_array_pointer) return true; seen_array = false; + for (ref = e->ref; ref; ref = ref->next) { + if (!seen_array && ref->type == REF_COMPONENT + && (ref->u.c.component->ts.type != BT_CLASS + && ref->u.c.component->ts.type != BT_DERIVED)) + return false; + if (ref->type == REF_ARRAY && ref->u.ar.type != AR_ELEMENT) seen_array = true; *** is_subref_array (gfc_expr * e) *** 1089,1094 --- 1089,1101 && ref->type != REF_ARRAY) return seen_array; } + + if (e->symtree->n.sym->ts.type == BT_CLASS + && e->symtree->n.sym->attr.dummy + && CLASS_DATA (e->symtree->n.sym)->attr.dimension + && CLASS_DATA (e->symtree->n.sym)->attr.class_pointer) + return true; + return false; } Index: gcc/testsuite/gfortran.dg/pointer_array_component_3.f90 === *** gcc/testsuite/gfortran.dg/pointer_array_component_3.f90 (nonexistent) --- gcc/testsuite/gfortran.dg/pointer_array_component_3.f90 (working copy) *** *** 0 --- 1,36 + ! { dg-do run } + ! + ! Test the fix for PR88685, in which the component array references in 'doit' + ! were being ascribed to the class pointer 'Cls' itself so that the stride + ! measure between elements was wrong. + ! + ! Contributed by Antony Lewis + ! + program tester + implicit none + Type TArr + integer, allocatable :: CL(:) + end Type TArr + + type(TArr), allocatable, target :: arr(:,:) + class(TArr), pointer:: Cls(:,:) + integer i + + allocate(arr(1,1)) + allocate(arr(1,1)%CL(3)) + arr(1,1)%CL=-1 + cls => arr + call doit(cls) + if (any (arr(1,1)%cl .ne. [3,2,1])) stop 3 + contains + subroutine doit(cls) + class(TArr), pointer :: Cls(:,:) + + cls(1,1)%CL(1) = 3 + cls(1,1)%CL(2:3) = [2,1] + + if (any (Cls(1,1)%CL .ne. [3,2,1])) stop 1 + if (Cls(1,1)%CL(2) .ne. 2) stop 2 + + end subroutine doit + end program tester
[Patch, fortran] PR88685 - [8/9 regression] pointer class array argument indexing
This one is more or less obvious and is described in the ChangeLog. The key point is that full or section array references to intrinsic components were returning a false true from expr.c (is_subref_array). Returning false if a component is intrinsic and following anything other than an array element is an obvious remedy. Bootstrapped and regtested on FC28/x86_64 - OK for trunk and 8-branch? Paul
Re: [PATCH] PR fortran/85780 -- alternate return and BIND(C) conflict
Hi Steve, That's OK. Thanks Paul On Fri, 25 Jan 2019 at 01:50, Steve Kargl wrote: > > All, > > My original patch for this PR simply fixed an ICE, which > then allowed the code to compile. In reality, an alternate > return is not ISO C interoperable, so an error message is > a more appropriate response. So, I re-opened the PR. > > The attached patch has been tested on x86_64-*-freebsd. > OK to commit? > > 2019-01-24 Steven G. Kargl > > PR fortran/85780 > * decl.c (gfc_match_subroutine): Check for conflict between BIND(C) > and alternative return. > > 2019-01-24 Steven G. Kargl > > PR fortran/85780 > * gfortran.dg/pr85780.f90: Update testcase for error message. > > -- > Steve -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran] PR88929 - ICE on building MPICH 3.2 with GCC 9 with ISO_Fortran_binding
Hi Steve, Fixed in revision 268231. This was a copy/paste/modify with the type built in. Have corrected both. > > tree > > + gfc_conv_descriptor_elem_len (tree desc) > > + { > > + tree tmp; > > + tree dtype; > > + > > + dtype = gfc_conv_descriptor_dtype (desc); > > + tmp = gfc_advance_chain (TYPE_FIELDS (TREE_TYPE (dtype)), > > +GFC_DTYPE_ELEM_LEN); > > + gcc_assert (tmp!= NULL_TREE > > space after tmp All the other corrections were made. Thanks for the review. Paul
[Patch, fortran] PR88929 - ICE on building MPICH 3.2 with GCC 9 with ISO_Fortran_binding
The attached patch allows MPICH 3.2 to build correctly and to test successfully. Two problems were addressed: (i) The original implementation of ISO_Fortran_binding did not take account of the possibility that assumed rank/assumed type arrays could be passed as dummy arguments. This necessitated the e->rank condition being changed to e->rank !=0 since assumed rank entities have rank == -1 in gfc_exprs. (ii) Intent in requires that a copy be made of the data to be passed to the C procedure. This is now implemented. The testcase provides two interfaces to the C-procedure; one with intent in and the other with intent inout. The C procedure changes the data and so this is detected in the inout case but not in the intent in case. For both, the C procedure checks that the sum over the array is correct. >From the point of view of the release, this is completely safe since the patch is isolated to the ISO_Fortran_binding interface, which is newly introduced and has no effect on the rest of the testsuite. I have bumped the testcase number by 1 to allow for a corrected version of the withdrawn patch for the test of the errors from the CFI API functions. I will return to this as soon as I can. Bootstrapped and regtested on FC28/x86_64 - OK for trunk? Paul 2019-01-23 Paul Thomas PR fortran/88929 * trans-array.c (gfc_conv_descriptor_elem_len): New function. * trans-array.h : Add prototype for above. * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Take account of assumed rank arrays being flagged by rank = -1 in expressions. Intent in arrays need a pointer to a copy of the data to be assigned to the descriptor passed for conversion. This should then be freed, together with the CFI descriptor on return from the C call. 2019-01-23 Paul Thomas PR fortran/88929 * gfortran.dg/ISO_Fortran_binding_3.f90 : New test * gfortran.dg/ISO_Fortran_binding_3.c : Subsidiary source. Index: gcc/fortran/trans-array.c === *** gcc/fortran/trans-array.c (revision 268193) --- gcc/fortran/trans-array.c (working copy) *** gfc_conv_descriptor_rank (tree desc) *** 293,298 --- 293,314 tree + gfc_conv_descriptor_elem_len (tree desc) + { + tree tmp; + tree dtype; + + dtype = gfc_conv_descriptor_dtype (desc); + tmp = gfc_advance_chain (TYPE_FIELDS (TREE_TYPE (dtype)), + GFC_DTYPE_ELEM_LEN); + gcc_assert (tmp!= NULL_TREE + && TREE_TYPE (tmp) == size_type_node); + return fold_build3_loc (input_location, COMPONENT_REF, TREE_TYPE (tmp), + dtype, tmp, NULL_TREE); + } + + + tree gfc_conv_descriptor_attribute (tree desc) { tree tmp; Index: gcc/fortran/trans-array.h === *** gcc/fortran/trans-array.h (revision 268193) --- gcc/fortran/trans-array.h (working copy) *** tree gfc_conv_descriptor_offset_get (tre *** 169,174 --- 169,175 tree gfc_conv_descriptor_span_get (tree); tree gfc_conv_descriptor_dtype (tree); tree gfc_conv_descriptor_rank (tree); + tree gfc_conv_descriptor_elem_len (tree); tree gfc_conv_descriptor_attribute (tree); tree gfc_get_descriptor_dimension (tree); tree gfc_conv_descriptor_stride_get (tree, tree); Index: gcc/fortran/trans-expr.c === *** gcc/fortran/trans-expr.c (revision 268193) --- gcc/fortran/trans-expr.c (working copy) *** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p *** 4924,4929 --- 4924,4931 tree tmp; tree cfi_desc_ptr; tree gfc_desc_ptr; + tree ptr = NULL_TREE; + tree size; tree type; int attribute; symbol_attribute attr = gfc_expr_attr (e); *** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p *** 4939,4945 attribute = 1; } ! if (e->rank) { gfc_conv_expr_descriptor (parmse, e); --- 4941,4947 attribute = 1; } ! if (e->rank != 0) { gfc_conv_expr_descriptor (parmse, e); *** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p *** 4950,4958 /* All the temporary descriptors are marked as DECL_ARTIFICIAL. If the expression type is different from the descriptor type, then the offset must be found (eg. to a component ref or substring) ! and the dtype updated. */ ! type = gfc_typenode_for_spec (&e->ts); ! if (DECL_ARTIFICIAL (parmse->expr) && type != gfc_get_element_type (TREE_TYPE (parmse->expr))) { /* Obtain the offset to the data. */ --- 4952,4965 /* All the temporary descriptors are marked as DECL_ARTIFICIAL. If the expression type is different from the descriptor type, then the offset must be found (eg. to a component ref or substring) ! and the dtype updated. Assumed type entities are only allowed ! to be dummies in fortran. They therefore lack the decl specific ! appendiges and s
Re: ISO_Fortran_binding patch
Hi everybody, I have done the minimum to make the testsuite failures to go away(thanks, Jakub) and to fix the first (offline) reported bug. Committed as r267946. As to the location of ISO_Fortran_binding_2.h, I am open to proposed fixes. Thomas kindly engineered that part of the original patch since I have tried to keep my nose out of the configure side of things. Regards Paul 2019-01-15 Paul Thomas * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Deal with exprs that are indirect references; ie. dummy arguments. 2019-01-15 Paul Thomas * gfortran.dg/ISO_Fortran_binding_2.c : Change reference to ISO_Fortran_binding_2.h. On Tue, 15 Jan 2019 at 08:02, Jakub Jelinek wrote: > > On Tue, Jan 15, 2019 at 08:05:59AM +0100, Richard Biener wrote: > > >It either should > > >#include "../../../libgfortran/ISO_Fortran_binding.h" > > >instead or the Fortran *.exp files should arrange for > > >-I.../libgfortran/ > > >to be added to all gfortran tests. Because right now it FAILs if you > > >don't > > >have ISO_Fortran_binding.h header installed, or succeeds, but includes > > >header from some other compiler version or even other compiler > > >altogether. > > This still needs to be fixed. > > > >Where is that header installed BTW? > > >Would be best if it got installed in directories like: > > >$prefix/lib/gcc/$target/$version/include > > > > > >See e.g. libssp or libsanitizer, both have something like > > >target_noncanonical = @target_noncanonical@ > > >libsubincludedir = > > >$(libdir)/gcc/$(target_noncanonical)/$(gcc_version)/include > > >nobase_libsubinclude_HEADERS = ssp/ssp.h ssp/string.h ssp/stdio.h > > >ssp/unistd.h > > > > > >You probably want it to go directly in the include dir, so without the > > >ssp/ > > >or whatever else prefixes. > > > > It's there, but also in the multilib locations (which is dubious? Not sure > > if we ever search tose include paths) > > Yeah, for -m32 it is in > .../lib/gcc/x86_64-pc-linux-gnu/9.0.0/32/include/ > which isn't that useful; while the finclude/ in there is needed, because > those are target specific, this header is the same and so it could be > just in .../9.0.0/include/ always (like e.g. the std*.h headers, intrinsics > etc.). > > Jakub -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: ISO_Fortran_binding patch
Done as revision 267884. Thanks again. Paul On Sat, 12 Jan 2019 at 18:29, Paul Richard Thomas wrote: > > Hi Steve, > > Many thanks for the heads up. I had seen similar problems with the the > second testcase and I thought that I had fixed them. I will delete > them from the tree and will do more work to fix the problem(s). > > Cheers > > Paul > > On Sat, 12 Jan 2019 at 17:17, Steve Kargl > wrote: > > > > On Sat, Jan 12, 2019 at 09:10:27AM -0800, Steve Kargl wrote: > > > On Sat, Jan 12, 2019 at 03:28:02PM +, Paul Richard Thomas wrote: > > > > Hi Thomas, > > > > > > > > Committed as revision 267881. I removed the duplicate include file and > > > > added some documentation, as suggested. > > > > > > > > Many thanks for all the help > > > > > > > > > > Paul, > > > > > > I'm seeing the following failures. Note, I have my uncommitted > > > ENTRY patch in my tree. I won't be able to investigate for about > > > 30 minutes. > > > > > > FAIL: gfortran.dg/ISO_Fortran_binding_2.f90 -O0 execution test > > > Running /safe/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/debug/debug.exp ... > > > Running /safe/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/dg.exp ... > > > FAIL: gfortran.dg/ISO_Fortran_binding_2.f90 -O2 execution test > > > FAIL: gfortran.dg/ISO_Fortran_binding_2.f90 -O3 -fomit-frame-pointer > > > -funroll-loops -fpeel-loops -ftracer -finline-functions execution test > > > FAIL: gfortran.dg/ISO_Fortran_binding_2.f90 -O3 -g execution test > > > > > > > Regression testing finished faster than I thought. Doing > > > > % gmake check-fortran RUNTESTFLAGS="dg.exp=ISO_Fortran_binding_2.f90" > > ... > > === gfortran Summary === > > > > # of expected passes8 > > # of unexpected failures4 > > > > The first failure in the gfortran.log file is > > > > CFI_address: subscripts[0], is out of bounds. dv->dim[0].extent = 3 > > subscripts[0] = 3. > > CFI_address: subscripts[0], is out of bounds. dv->dim[0].extent = 3 > > subscripts[0] = -1. > > CFI_address: base address of C Descriptor must not be NULL. > > CFI_deallocate: Base address is already NULL. > > CFI_deallocate: C Descriptor must describe a pointer or allocatable object. > > CFI_allocate: Base address of C descriptor must be NULL. > > CFI_allocate: The object of the C descriptor must be a pointer or > > allocatable variable. > > CFI_establish: Rank must be between 0 and 15, 0 < rank (0 !< 16). > > CFI_establish: If the C Descriptor represents an allocatable variable > > (dv->attribute = 1), its base address must be NULL (dv->base_addr = NULL). > > CFI_establish: If base address is not NULL (base_addr != NULL), the > > established C descriptor is for a nonallocatable entity (attribute != 1). > > CFI_is_contiguous: Base address of C Descriptor is already NULL. > > CFI_is_contiguous: C Descriptor must describe an array (0 < dv->rank = 0). > > CFI_section: Base address of source must not be NULL. > > CFI_section: Source must describe an array (0 < source->rank, 0 !< 0). > > CFI_section: Rank of result must be equal to the rank of source minus the > > number of zeros in strides (result->rank = source->rank - zero_count, 1 != > > 1 - 1). > > CFI_section: Lower bounds must be within the bounds of the fortran array > > (source->dim[0].lower_bound <= lower_bounds[0] <= > > source->dim[0].lower_bound + source->dim[0].extent - 1, 0 <= -1 <= 99). > > CFI_section: Lower bounds must be within the bounds of the fortran array > > (source->dim[0].lower_bound <= lower_bo > > unds[0] <= source->dim[0].lower_bound + source->dim[0].extent - 1, 0 <= 100 > > <= 99). > > > > Program received signal SIGSEGV: Segmentation fault - invalid memory > > reference. > > > > Backtrace for this error: > > #0 0x71a2 in ??? > > #1 0x0 in ??? > > > > The 2nd, 3rd, and 4th failures are > > > > CFI_address: subscripts[0], is out of bounds. dv->dim[0].extent = 3 > > subscripts[0] = 3. > > CFI_address: subscripts[0], is out of bounds. dv->dim[0].extent = 3 > > subscripts[0] = -1. > > CFI_address: base address of C Descriptor must not be NULL. > > CFI_deallocate: Base address is already NULL. > > > > Program received signal SIGFPE: Floating-point exception - erroneous > > arithmetic operation. > > > > Backtrace for this error: > > #0 0x71a2 in ??? > > #1 0x400eed in ??? > > #2 0x4021ea in _start > > at /usr/src/lib/csu/amd64/crt1.c:76 > > #3 0x200628fff in ??? > > > > > > -- > > Steve > > > > -- > "If you can't explain it simply, you don't understand it well enough" > - Albert Einstein -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: ISO_Fortran_binding patch
Hi Steve, Many thanks for the heads up. I had seen similar problems with the the second testcase and I thought that I had fixed them. I will delete them from the tree and will do more work to fix the problem(s). Cheers Paul On Sat, 12 Jan 2019 at 17:17, Steve Kargl wrote: > > On Sat, Jan 12, 2019 at 09:10:27AM -0800, Steve Kargl wrote: > > On Sat, Jan 12, 2019 at 03:28:02PM +, Paul Richard Thomas wrote: > > > Hi Thomas, > > > > > > Committed as revision 267881. I removed the duplicate include file and > > > added some documentation, as suggested. > > > > > > Many thanks for all the help > > > > > > > Paul, > > > > I'm seeing the following failures. Note, I have my uncommitted > > ENTRY patch in my tree. I won't be able to investigate for about > > 30 minutes. > > > > FAIL: gfortran.dg/ISO_Fortran_binding_2.f90 -O0 execution test > > Running /safe/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/debug/debug.exp ... > > Running /safe/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/dg.exp ... > > FAIL: gfortran.dg/ISO_Fortran_binding_2.f90 -O2 execution test > > FAIL: gfortran.dg/ISO_Fortran_binding_2.f90 -O3 -fomit-frame-pointer > > -funroll-loops -fpeel-loops -ftracer -finline-functions execution test > > FAIL: gfortran.dg/ISO_Fortran_binding_2.f90 -O3 -g execution test > > > > Regression testing finished faster than I thought. Doing > > % gmake check-fortran RUNTESTFLAGS="dg.exp=ISO_Fortran_binding_2.f90" > ... > === gfortran Summary === > > # of expected passes8 > # of unexpected failures4 > > The first failure in the gfortran.log file is > > CFI_address: subscripts[0], is out of bounds. dv->dim[0].extent = 3 > subscripts[0] = 3. > CFI_address: subscripts[0], is out of bounds. dv->dim[0].extent = 3 > subscripts[0] = -1. > CFI_address: base address of C Descriptor must not be NULL. > CFI_deallocate: Base address is already NULL. > CFI_deallocate: C Descriptor must describe a pointer or allocatable object. > CFI_allocate: Base address of C descriptor must be NULL. > CFI_allocate: The object of the C descriptor must be a pointer or allocatable > variable. > CFI_establish: Rank must be between 0 and 15, 0 < rank (0 !< 16). > CFI_establish: If the C Descriptor represents an allocatable variable > (dv->attribute = 1), its base address must be NULL (dv->base_addr = NULL). > CFI_establish: If base address is not NULL (base_addr != NULL), the > established C descriptor is for a nonallocatable entity (attribute != 1). > CFI_is_contiguous: Base address of C Descriptor is already NULL. > CFI_is_contiguous: C Descriptor must describe an array (0 < dv->rank = 0). > CFI_section: Base address of source must not be NULL. > CFI_section: Source must describe an array (0 < source->rank, 0 !< 0). > CFI_section: Rank of result must be equal to the rank of source minus the > number of zeros in strides (result->rank = source->rank - zero_count, 1 != 1 > - 1). > CFI_section: Lower bounds must be within the bounds of the fortran array > (source->dim[0].lower_bound <= lower_bounds[0] <= source->dim[0].lower_bound > + source->dim[0].extent - 1, 0 <= -1 <= 99). > CFI_section: Lower bounds must be within the bounds of the fortran array > (source->dim[0].lower_bound <= lower_bo > unds[0] <= source->dim[0].lower_bound + source->dim[0].extent - 1, 0 <= 100 > <= 99). > > Program received signal SIGSEGV: Segmentation fault - invalid memory > reference. > > Backtrace for this error: > #0 0x71a2 in ??? > #1 0x0 in ??? > > The 2nd, 3rd, and 4th failures are > > CFI_address: subscripts[0], is out of bounds. dv->dim[0].extent = 3 > subscripts[0] = 3. > CFI_address: subscripts[0], is out of bounds. dv->dim[0].extent = 3 > subscripts[0] = -1. > CFI_address: base address of C Descriptor must not be NULL. > CFI_deallocate: Base address is already NULL. > > Program received signal SIGFPE: Floating-point exception - erroneous > arithmetic operation. > > Backtrace for this error: > #0 0x71a2 in ??? > #1 0x400eed in ??? > #2 0x4021ea in _start > at /usr/src/lib/csu/amd64/crt1.c:76 > #3 0x200628fff in ??? > > > -- > Steve -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: ISO_Fortran_binding patch
Hi Thomas, Committed as revision 267881. I removed the duplicate include file and added some documentation, as suggested. Many thanks for all the help Paul On Tue, 8 Jan 2019 at 23:19, Thomas Koenig wrote: > > Hi Paul, > > > This is an updated version of the earlier patch. The main addition is > > a second testcase that checks the errors emitted by the CFI API > > functions. > > I notice that the header file ISO_Fortran_binding.h is found twice > in the patch. > > Is there any particular reason why you do not want to use > > ! { dg-additional-options "-I $srcdir/../../libgfortran" } > > in the test cases, and have it only once in the source trees? > > However, I have no real strong opinion on this matter, if you > want to keep it as submitted, it is also fine. > > Therefore: OK for trunk, and thanks a lot for the patch! > > Documentation we can add at a later date, I think. > > Regards > > Thomas -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [PATCH] PR fortran/61765 -- Avoid ENTRY names in check of repeditive symbols
Hi Steve, This is OK for trunk. Thanks Paul On Sat, 12 Jan 2019 at 04:34, Steve Kargl wrote: > > The attached patch has been tested on x86_64-*-freebsd. There > were no regression. The patch is less then obvious, but simple. > OK to commit? > > 2019-01-11 Steven G. Kargl > > PR fortran/61765 > * resolve.c (gfc_verify_binding_labels): Break if-elseif-elseif > structure into independent > if's with a return to simplify logic. Avoid a check for ENTRY name > with bind(c). > > 2019-01-11 Steven G. Kargl > > PR fortran/61765 > * gfortran.dg/pr61765.f90: New test. > > -- > Steve -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [PATCH] PR fortran/35031 -- Check F2018:C1246
Hi Steve, Yes, that's good for trunk. Thanks Paul On Fri, 11 Jan 2019 at 01:16, Steve Kargl wrote: > > An entry-name obtains the elemental attribute from its containing > procedure. F2018:C1546 prohibits an procedure from having a BIND(C) > attribute. BIND(C) can appear on the entry-stmt line, so gfortran > needs to check for a conflict. The attached patch does this check. > Tested on x86_64-*-freebsd. Ok to commit? > > 2019-01-10 Steven G. Kargl > > PR fortran/35031 > * decl.c (gfc_match_entry): Check for F2018:C1546. Fix nearby > mis-indentation. > > 2019-01-10 Steven G. Kargl > > PR fortran/35031 > * gfortran.dg/pr35031.f90: new test. > > -- > Steve -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: ISO_Fortran_binding patch
Hi Thomas, Aaaah! Light bulb moment - I was looking in the $PREFIX/include directory. For whatever reason, mine does not appear in lib64 but in lib. OK, that will have to do for now because the patch is blocking my tree for a number of other things. I'll fix Bernhard's nit and commit on Saturday. Thanks everybody. Paul On Wed, 9 Jan 2019 at 20:08, Thomas Koenig wrote: > > Hi Paul, > > > Incidentally, we need to make sure that it is distributed in the > > include directory. I have yet to figure out how to do that. > > It already does that, that was part of what I sent you :-) > > It's the > > +gfor_c_HEADERS = $(srcdir)/ISO_Fortran_binding.h > +gfor_cdir = > $(libdir)/gcc/$(target_alias)/$(gcc_version)$(MULTISUBDIR)/include > + > > part in Makefile.am which puts in the requisite magic into > Makefile.im. > > Header files are then installed into > > $PREFIX/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/include/ISO_Fortran_binding.h > $PREFIX/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/32/include/ISO_Fortran_binding.h > > So, OK for trunk. > > Thanks a lot for this! Looking at what we are currently > finishing, I think we will be quite close to F2008 conformance > when gcc 9 comes out, modulo a few bugs, of course. > > Regards > > Thomas > -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: ISO_Fortran_binding patch
Hi Thomas, > Is there any particular reason why you do not want to use > > ! { dg-additional-options "-I $srcdir/../../libgfortran" } > > in the test cases, and have it only once in the source trees? I will make it so. Thanks for the reminder. > > However, I have no real strong opinion on this matter, if you > want to keep it as submitted, it is also fine. Incidentally, we need to make sure that it is distributed in the include directory. I have yet to figure out how to do that. > > Therefore: OK for trunk, and thanks a lot for the patch! > > Documentation we can add at a later date, I think. I can work that up before committing the patch since I will not be in a position to work on gfortran until Saturday. In the longer term, I will take the descriptor conversion out of the library and will write a CFI equivalent of gfc_conv_expr_descriptor. However, I will await the inevitable bug reports before doing that :-) Thanks Paul > > Regards > > Thomas -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
[Patch, fortran] PR77703 - [7/8/9 Regression] ICE on assignment to pointer function
I will apply this as 'obvious' this evening, unless there are objections. The patch is entirely self-explanatory. Paul 2018-12-23 Paul Thomas PR fortran/77703 * resolve.c (get_temp_from_expr): Use the string length of constant character expressions. 2018-12-23 Paul Thomas PR fortran/77703 * gfortran.dg/ptr_func_assign_5.f08 : New test. Index: gcc/fortran/resolve.c === *** gcc/fortran/resolve.c (revision 267336) --- gcc/fortran/resolve.c (working copy) *** get_temp_from_expr (gfc_expr *e, gfc_nam *** 10637,10642 --- 10637,10647 gfc_get_sym_tree (name, ns, &tmp, false); gfc_add_type (tmp->n.sym, &e->ts, NULL); + if (e->expr_type == EXPR_CONSTANT && e->ts.type == BT_CHARACTER) + tmp->n.sym->ts.u.cl->length = gfc_get_int_expr (gfc_charlen_int_kind, + NULL, + e->value.character.length); + as = NULL; ref = NULL; aref = NULL; Index: gcc/testsuite/gfortran.dg/ptr_func_assign_5.f08 === *** gcc/testsuite/gfortran.dg/ptr_func_assign_5.f08 (nonexistent) --- gcc/testsuite/gfortran.dg/ptr_func_assign_5.f08 (working copy) *** *** 0 --- 1,45 + ! { dg-do run } + ! + ! Test the fix for PR77703, in which calls of the pointer function + ! caused an ICE in 'gfc_trans_auto_character_variable'. + ! + ! Contributed by Gerhard Steinmetz + ! + module m +implicit none +private +integer, parameter, public :: n = 2 +integer, parameter :: ell = 6 + +character(len=n*ell), target, public :: s + +public :: t + contains +function t( idx ) result( substr ) + integer, intent(in) :: idx + character(len=ell), pointer :: substr + + if ( (idx < 0).or.(idx > n) ) then + error stop + end if + substr => s((idx-1)*ell+1:idx*ell) +end function t + end module m + + program p +use m, only : s, t, n +integer :: i + +! Define 's' +s = "123456789012" + +! Then perform operations involving 't' +if (t(1) .ne. "123456") stop 1 +if (t(2) .ne. "789012") stop 2 + +! Do the pointer function assignments +t(1) = "Hello " +if (s .ne. "Hello 789012") Stop 3 +t(2) = "World!" +if (s .ne. "Hello World!") Stop 4 + end program p
Re: [patch, fortran] Fix PR 85544
Hi Thomas, That's OK for 7- through 9-branches. Thanks for the fix. Paul On Sun, 16 Dec 2018 at 22:01, Thomas Koenig wrote: > > Hello world, > > the PR pointed out an old regression because the front-end optimization > pass was substituting 2**n with ishift(1,n), where n was an array. > > Simply removing the optimization for that case would have been easy, > but also introduced a performance regression. > > So, for this, I moved the optimization to trans-*, where it makes more > sense. > > Regression-tested. This turned up that one of our tests, mvbits_1.f90, > depends on the behavior that 2**32 is zero. This is certainly not > guaranteed by the standard, but I chose to keep the behavior as not > to introduce any changes in behavior. > > This fixes a regression, so I would like to backport to all active > branches if this if possible. > > Oh yes, if anybody feels strongly that we should also optimize 32**n > and powers of other powers to two, now is the time to speak up :-) > > OK for affected branches? > > Regards > > Thomas > > 2018-12-16 Thomas Koenig > > PR fortran/85544 > * frontend-passes.c (optimize_power): Remove. > (optimize_op): Remove call to optimize_power. > * trans-expr.c (gfc_conv_power_op): Handle cases of 1**integer, > (2|4|8|16) ** integer and (-1) ** integer. > > 2018-12-16 Thomas Koenig > > PR fortran/85544 > * gfortran.dg/power_7.f90: New test. -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
[patch, fortran] PR87881 - gfortran.dg/inquiry_type_ref_(1.f08|3.f90) fail on darwin
Applied as 'obvious' after regtesting on FC28/x86_64. The second part of the patch (in simplify_ref_chain) is due to Jakub, for which many thanks. The first is consequent on the need to deal with more than one inquiry part ref (see the testcase) and yet be able to return true from simplify_ref_chain. The testcase checks this part. Paul 2018-12-21 Paul Thomas PR fortran/87881 * expr.c (find_inquiry_ref): Loop through the inquiry refs in case there are two of them. (simplify_ref_chain): Return true after a successful call to find_inquiry_ref. 2018-12-21 Paul Thomas PR fortran/87881 * gfortran.dg/inquiry_part_ref_4.f90: New test.
Implementation of F2018:18.4 C descriptors and ISO_Fortran_binding.h
The attached is an implementation of ISO_Fortran_binding. Please note that the ChangeLogs contain no mention of the changes that appear in the configure files on building in maintainer mode. The patch only couples to assumed rank and assumed shape formal arguments of bind_c procedures, via trans-expr.c(gfc_conv_procedure_call) calls to gfc_conv_gfc_desc_to_cfi_desc. Such calls in the past would have generated code that would have been 'invalid' unless typedefs for the gfortran descriptor were used. For this reason, I believe that this is safe to commit even at this stage in the release cycle. It would be nice if ISO_Fortran_binding.h were one of the standard C includes but I haven't tried to do this yet. The testing and, indeed, the provision of testcases is incomplete but I thought this to be a good time for review. One question for any reviewer is the need for the error messages in the CFI API functions? Daniel put in the work but they seem potentially excessive for a library function. In my mind, it is probably sufficient to return the error code. Thomas Koenig helped enormously with some build problems for which thanks. He experienced ICEs associated with malloc/free problems, which I did not see but believe to have eliminated with the help of valgrind. All were associated with the use of flexible arrays Bootstrapped and regtested on FC20/x86_64 - OK for trunk? Paul Index: gcc/configure === *** gcc/configure (revision 266426) --- gcc/configure (working copy) *** foo: .long 25 *** 24393,24404 ;; or1k*-*-*) conftest_s=' ! .section ".tdata","awT",@progbits ! foo:.long 25 ! .text ! l.movhi r3, tpoffha(foo) ! l.add r3, r3, r10 ! l.lwz r4, tpofflo(foo)(r3)' tls_first_major=2 tls_first_minor=30 tls_as_opt=--fatal-warnings --- 24393,24404 ;; or1k*-*-*) conftest_s=' ! .section ".tdata","awT",@progbits ! foo: .long 25 ! .text ! l.movhi r3, tpoffha(foo) ! l.add r3, r3, r10 ! l.lwz r4, tpofflo(foo)(r3)' tls_first_major=2 tls_first_minor=30 tls_as_opt=--fatal-warnings Index: gcc/fortran/trans-array.c === *** gcc/fortran/trans-array.c (revision 266426) --- gcc/fortran/trans-array.c (working copy) *** gfc_conv_descriptor_rank (tree desc) *** 293,298 --- 293,314 tree + gfc_conv_descriptor_attribute (tree desc) + { + tree tmp; + tree dtype; + + dtype = gfc_conv_descriptor_dtype (desc); + tmp = gfc_advance_chain (TYPE_FIELDS (TREE_TYPE (dtype)), + GFC_DTYPE_ATTRIBUTE); + gcc_assert (tmp!= NULL_TREE + && TREE_TYPE (tmp) == short_integer_type_node); + return fold_build3_loc (input_location, COMPONENT_REF, TREE_TYPE (tmp), + dtype, tmp, NULL_TREE); + } + + + tree gfc_get_descriptor_dimension (tree desc) { tree type, field; *** gfc_trans_dummy_array_bias (gfc_symbol * *** 6767,6773 /* Calculate the overall offset, including subreferences. */ ! static void gfc_get_dataptr_offset (stmtblock_t *block, tree parm, tree desc, tree offset, bool subref, gfc_expr *expr) { --- 6783,6789 /* Calculate the overall offset, including subreferences. */ ! void gfc_get_dataptr_offset (stmtblock_t *block, tree parm, tree desc, tree offset, bool subref, gfc_expr *expr) { Index: gcc/fortran/trans-array.h === *** gcc/fortran/trans-array.h (revision 266426) --- gcc/fortran/trans-array.h (working copy) *** void gfc_conv_tmp_array_ref (gfc_se * se *** 136,141 --- 136,143 /* Translate a reference to an array temporary. */ void gfc_conv_tmp_ref (gfc_se *); + /* Calculate the overall offset, including subreferences. */ + void gfc_get_dataptr_offset (stmtblock_t*, tree, tree, tree, bool, gfc_expr*); /* Obtain the span of an array. */ tree gfc_get_array_span (tree, gfc_expr *); /* Evaluate an array expression. */ *** tree gfc_conv_descriptor_offset_get (tre *** 167,172 --- 169,175 tree gfc_conv_descriptor_span_get (tree); tree gfc_conv_descriptor_dtype (tree); tree gfc_conv_descriptor_rank (tree); + tree gfc_conv_descriptor_attribute (tree); tree gfc_get_descriptor_dimension (tree); tree gfc_conv_descriptor_stride_get (tree, tree); tree gfc_conv_descriptor_lbound_get (tree, tree); Index: gcc/fortran/trans-decl.c === *** gcc/fortran/trans-decl.c (revision 266426) --- gcc/fortran/trans-decl.c (working copy) *** tree gfor_fndecl_fdate; *** 114,119 --- 114,121 tree gfor_fndecl_ttynam; tree gfor_fndecl_in_pack; tree gfor_fndecl_in_unpack; + tree gfor_fndecl_cfi_to_gfc; + tree gfor_fndecl_gfc_to_cfi; tree gfor_fndecl_associated; tree gfor_fndecl_system
Re: [PATCH] Support simd function declarations via a pre-include.
Hi All, Forget my remark about mp_prop_design.f90. ifort -parallel means just that and has nothing to do with vectorization. Sorry for the noise. Paul On Sat, 17 Nov 2018 at 13:29, Paul Richard Thomas wrote: > > Hi All, > > I took a few moments away from what I really must be doing to try out > an earlier version of the patch. There are quite a few CRs in the > patch and the third chunk in gcc.c was rejected, although I cannot see > why. I made a change to scanner.c to prevent the segfault that results > from not having the pre-include file in the right directory - see > attached. > > Everything works fine with the testcases and a slightly evolved > version shows reasonable speed-ups: > program test_overloaded_intrinsic > real(4) :: x4(3200), y4(3200, 1) > real(8) :: x8(3200), y8(3200) > >do i = 1,1 > y4(:,i) = cos(x4) > y4(:,i) = x4*y4(:,i) > y4(:,i) = sqrt(y4(:,i)) > end do > print *, y4(1,1) > end > > Disappointingly, though, one of the worst cases in > https://www.fortran.uk/fortran-compiler-comparisons/ of a big > difference between gfortran and ifort with vectorization, > mp_prop_design.f90, doesn't seem to pick up any of the vectorization > opportunities, even though it is peppered with trig functions. Should > I be expecting this? Yes, I did add the other trig functions to the > pre-include file :-) > > Best regards and thanks for taking care of this > > Paul > On Fri, 16 Nov 2018 at 15:13, Martin Liška wrote: > > > > On 11/16/18 2:49 PM, Jakub Jelinek wrote: > > > On Fri, Nov 16, 2018 at 02:24:42PM +0100, Martin Liška wrote: > > >> + if (gfc_match (" (%n) attributes simd", builtin) != MATCH_YES) > > >> +return MATCH_ERROR; > > >> + > > >> + int builtin_kind = 0; > > >> + if (gfc_match (" (notinbranch)") == MATCH_YES) > > > > > > I think you need " ( notinbranch )" here. > > > > > >> +builtin_kind = -1; > > >> + else if (gfc_match (" (inbranch)") == MATCH_YES) > > >> +builtin_kind = 1; > > > > > > And similarly here (+ testsuite coverage for whether you can in free form > > > insert spaces in all the spots that should be allowed). > > > !gcc$ builtin ( sinf ) attributes simd ( notinbranch ) ! comment > > > e.g. should be valid in free form (and fixed form too). > > > > > >> --- a/gcc/fortran/gfortran.h > > >> +++ b/gcc/fortran/gfortran.h > > >> @@ -2764,6 +2764,18 @@ bool gfc_in_match_data (void); > > >> match gfc_match_char_spec (gfc_typespec *); > > >> extern int directive_unroll; > > >> > > >> +/* Tuple for parsing of vectorized built-ins. */ > > >> +struct vect_builtin_tuple > > >> +{ > > >> + vect_builtin_tuple (const char *n, int t): name (n), simd_type (t) > > > > > > gfc_vect_builtin_tuple ? > > > + document what the simd_type is (or make it enum or whatever). > > > One option would be enum omp_clause_code and use OMP_CLAUSE_ERROR for > > > the case where the argument isn't specified, but I think generally > > > gfortran.h doesn't depend on tree* stuff and wants to have its own > > > enums etc. > > > > > >> +extern vec vectorized_builtins; > > > > > > gfc_vectorized_builtins ? > > > > > >> --- a/gcc/fortran/trans-intrinsic.c > > >> +++ b/gcc/fortran/trans-intrinsic.c > > >> @@ -597,7 +597,61 @@ define_quad_builtin (const char *name, tree type, > > >> bool is_const) > > >>return fndecl; > > >> } > > >> > > >> +/* Add SIMD attribute for FNDECL built-in if the built-in > > >> + name is in VECTORIZED_BUILTINS. */ > > >> +#include "print-tree.h" > > > > > > If you need to include a header, include it at the start of the file. > > > > > >> +static void > > >> +add_simd_flag_for_built_in (tree fndecl) > > >> +{ > > >> + if (fndecl == NULL_TREE) > > >> +return; > > >> + > > >> + const char *name = IDENTIFIER_POINTER (DECL_NAME (fndecl)); > > >> + for (unsigned i = 0; i < vectorized_builtins.length (); i++) > > >> +if (strcmp (vectorized_builtins[i].name, name) == 0) > > > > > > How many add_simd_flag_for_built_in calls are we expecting and how many > > > vectorized_builtins.length ()? If it is too much, perhaps e.g. sort > > > the vec
Re: *Ping* Fix PR 70260, ICE on invalid
Hi Thomas, OK for trunk. Thanks for working on it. Paul On Sat, 17 Nov 2018 at 15:10, Thomas Koenig wrote: > > Hi, > > > the attached patch fixes both ICEs in the PR by adding some tests. > > It was necessary to shuffle around a bit of code, plus to make sure that > > double error reporting did not become too bad. > > > > Regression-tested. OK for trunk? > > Ping? > > Regards > > Thomas -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [PATCH] Support simd function declarations via a pre-include.
Hi All, I took a few moments away from what I really must be doing to try out an earlier version of the patch. There are quite a few CRs in the patch and the third chunk in gcc.c was rejected, although I cannot see why. I made a change to scanner.c to prevent the segfault that results from not having the pre-include file in the right directory - see attached. Everything works fine with the testcases and a slightly evolved version shows reasonable speed-ups: program test_overloaded_intrinsic real(4) :: x4(3200), y4(3200, 1) real(8) :: x8(3200), y8(3200) do i = 1,1 y4(:,i) = cos(x4) y4(:,i) = x4*y4(:,i) y4(:,i) = sqrt(y4(:,i)) end do print *, y4(1,1) end Disappointingly, though, one of the worst cases in https://www.fortran.uk/fortran-compiler-comparisons/ of a big difference between gfortran and ifort with vectorization, mp_prop_design.f90, doesn't seem to pick up any of the vectorization opportunities, even though it is peppered with trig functions. Should I be expecting this? Yes, I did add the other trig functions to the pre-include file :-) Best regards and thanks for taking care of this Paul On Fri, 16 Nov 2018 at 15:13, Martin Liška wrote: > > On 11/16/18 2:49 PM, Jakub Jelinek wrote: > > On Fri, Nov 16, 2018 at 02:24:42PM +0100, Martin Liška wrote: > >> + if (gfc_match (" (%n) attributes simd", builtin) != MATCH_YES) > >> +return MATCH_ERROR; > >> + > >> + int builtin_kind = 0; > >> + if (gfc_match (" (notinbranch)") == MATCH_YES) > > > > I think you need " ( notinbranch )" here. > > > >> +builtin_kind = -1; > >> + else if (gfc_match (" (inbranch)") == MATCH_YES) > >> +builtin_kind = 1; > > > > And similarly here (+ testsuite coverage for whether you can in free form > > insert spaces in all the spots that should be allowed). > > !gcc$ builtin ( sinf ) attributes simd ( notinbranch ) ! comment > > e.g. should be valid in free form (and fixed form too). > > > >> --- a/gcc/fortran/gfortran.h > >> +++ b/gcc/fortran/gfortran.h > >> @@ -2764,6 +2764,18 @@ bool gfc_in_match_data (void); > >> match gfc_match_char_spec (gfc_typespec *); > >> extern int directive_unroll; > >> > >> +/* Tuple for parsing of vectorized built-ins. */ > >> +struct vect_builtin_tuple > >> +{ > >> + vect_builtin_tuple (const char *n, int t): name (n), simd_type (t) > > > > gfc_vect_builtin_tuple ? > > + document what the simd_type is (or make it enum or whatever). > > One option would be enum omp_clause_code and use OMP_CLAUSE_ERROR for > > the case where the argument isn't specified, but I think generally > > gfortran.h doesn't depend on tree* stuff and wants to have its own > > enums etc. > > > >> +extern vec vectorized_builtins; > > > > gfc_vectorized_builtins ? > > > >> --- a/gcc/fortran/trans-intrinsic.c > >> +++ b/gcc/fortran/trans-intrinsic.c > >> @@ -597,7 +597,61 @@ define_quad_builtin (const char *name, tree type, > >> bool is_const) > >>return fndecl; > >> } > >> > >> +/* Add SIMD attribute for FNDECL built-in if the built-in > >> + name is in VECTORIZED_BUILTINS. */ > >> +#include "print-tree.h" > > > > If you need to include a header, include it at the start of the file. > > > >> +static void > >> +add_simd_flag_for_built_in (tree fndecl) > >> +{ > >> + if (fndecl == NULL_TREE) > >> +return; > >> + > >> + const char *name = IDENTIFIER_POINTER (DECL_NAME (fndecl)); > >> + for (unsigned i = 0; i < vectorized_builtins.length (); i++) > >> +if (strcmp (vectorized_builtins[i].name, name) == 0) > > > > How many add_simd_flag_for_built_in calls are we expecting and how many > > vectorized_builtins.length ()? If it is too much, perhaps e.g. sort > > the vector by name and do a binary search. At least if it turns out to be > > non-trivial compile time. > >> + > >> + vectorized_builtins.truncate (0); > > > > That is a memory leak, right? The names are malloced. > > And why truncate rather than release? > >> + const char *path = find_a_file (&include_prefixes, argv[1], R_OK, true); > >> + if (path != NULL) > >> + return concat (argv[0], path, NULL); > > > > Formatting. > >> --- /dev/null > >> +++ b/gcc/testsuite/gfortran.dg/simd-builtins-1.h > >> @@ -0,0 +1,4 @@ > >> +!GCC$ builtin (sinf) attributes simd > >> +!GCC$ builtin (sinf) attributes simd (inbranch) > >> +!GCC$ builtin (sinf) attributes simd (notinbranch) > >> +!GCC$ builtin (cosf) attributes simd (notinbranch) > > > > Are you sure it is a good idea to have the 3 first lines for the same > > builtin, rather than different? > > > > It should be testsuite covered what we do in that case, but with the above > > you don't cover what happens e.g. with notinbranch alone, or no argument. > > > > Plus, as I said, I think you should have one *.f and one *.f90 test where > > you just use many of those !gcc$ builtin lines with spaces in various spots > > to verify it is parsed properly. > > > > Jakub > > > > Hi. > > I'm sending version, I changed the container to hash_map that should provi
Re: [Patch, fortran] PR40196 - [F03] [F08] Type parameter inquiry (str%len, a%kind) and Complex parts (z%re, z%im)
Hi Thomas, I tried failing cases of that kind; or assignment to len/kind part refs and returned correct errors. Must check where I was going wrong. Paul from a chilly Garching-bei-Muenchen On Sun, 28 Oct 2018, 13:38 Thomas Koenig Hi Paul, > > > >> inq would be easier to understand and unambiguous imho. > > > > Why? inquiry_type seems fine to me. > > I think Bernhard means the name of the member, i. > > I think it makes sense to leave as it is - gfc_ref is a > struct that occurs a lot in complicated expressions, and the other > members are one and two letters, too. > > > snip > >> Is the switch really worth it? I'd have used a plain chain of strcmp, > >> fwiw. > > > > I have done it. However, I might revert in order to combine the switch > > block where I set the typespec for the primary expression. > > Whatever suits you best. > > > I haven't added testcases for errors. Does anybody think that this is > necessary? > > Might not be a bad idea to run through at least each new error message > again. > > There is one illwfL test case which ICEs: > > $ cat b.f90 > program main >character(len=:), allocatable :: a >allocate(a,source="abc") >a%len = 2 >print *,a > end > $ gfortran b.f90 > gimplification failed: > (integer(kind=4)) .a type size > unit-size > align:32 warn_if_not_align:0 symtab:0 alias-set -1 > canonical-type 0x7f138acd15e8 precision:32 min 0x7f138acbcd68 -2147483648> max > pointer_to_this > > > arg:0 type size > unit-size > align:64 warn_if_not_align:0 symtab:0 alias-set -1 > canonical-type 0x7f138acd1738 precision:64 min 0x7f138acbcdf8 -9223372036854775808> max 9223372036854775807> > pointer_to_this > > used DI b.f90:1:0 size > unit-size > align:64 warn_if_not_align:0 context 0x7f138ae83200 MAIN__> > chain 0x7f138ae82540> > used unsigned DI b.f90:2:0 size 64> unit-size > align:64 warn_if_not_align:0 context 0x7f138ae83200 MAIN__ > b.f90:4:0: > > 4 | a%len = 2 >| > internal compiler error: gimplification failed > 0xb45602 gimplify_expr(tree_node**, gimple**, gimple**, bool > (*)(tree_node*), int) > ../../trunk/gcc/gimplify.c:12568 > > Regards > > Thomas >
Re: [Patch, fortran] PR40196 - [F03] [F08] Type parameter inquiry (str%len, a%kind) and Complex parts (z%re, z%im)
Hi Thomas, Thanks for finding the assignment a%len = 2 that escapes the check for lvalues. I am back home tomorrow night and will investigate why this one evades the trap. I think that an error test is needed in expr.c(gfc_check_assign). Cheers Paul On Sun, 28 Oct 2018 at 13:38, Thomas Koenig wrote: > > Hi Paul, > > > >> inq would be easier to understand and unambiguous imho. > > > > Why? inquiry_type seems fine to me. > > I think Bernhard means the name of the member, i. > > I think it makes sense to leave as it is - gfc_ref is a > struct that occurs a lot in complicated expressions, and the other > members are one and two letters, too. > > > snip > >> Is the switch really worth it? I'd have used a plain chain of strcmp, > >> fwiw. > > > > I have done it. However, I might revert in order to combine the switch > > block where I set the typespec for the primary expression. > > Whatever suits you best. > > > I haven't added testcases for errors. Does anybody think that this is > > necessary? > > Might not be a bad idea to run through at least each new error message > again. > > There is one illwfL test case which ICEs: > > $ cat b.f90 > program main >character(len=:), allocatable :: a >allocate(a,source="abc") >a%len = 2 >print *,a > end > $ gfortran b.f90 > gimplification failed: > (integer(kind=4)) .a type size > unit-size > align:32 warn_if_not_align:0 symtab:0 alias-set -1 > canonical-type 0x7f138acd15e8 precision:32 min 0x7f138acbcd68 -2147483648> max > pointer_to_this > > > arg:0 type size > unit-size > align:64 warn_if_not_align:0 symtab:0 alias-set -1 > canonical-type 0x7f138acd1738 precision:64 min 0x7f138acbcdf8 -9223372036854775808> max 9223372036854775807> > pointer_to_this > > used DI b.f90:1:0 size > unit-size > align:64 warn_if_not_align:0 context 0x7f138ae83200 MAIN__> > chain > used unsigned DI b.f90:2:0 size 64> unit-size > align:64 warn_if_not_align:0 context 0x7f138ae83200 MAIN__ > b.f90:4:0: > > 4 | a%len = 2 >| > internal compiler error: gimplification failed > 0xb45602 gimplify_expr(tree_node**, gimple**, gimple**, bool > (*)(tree_node*), int) > ../../trunk/gcc/gimplify.c:12568 > > Regards > > Thomas -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein