Re: [Patch] Fortran/openmp: Fix '!$omp end'
On 11.11.21 19:01, Jakub Jelinek wrote: On Thu, Nov 11, 2021 at 06:11:23PM +0100, Tobias Burnus wrote: --- a/gcc/fortran/parse.c +++ b/gcc/fortran/parse.c ... + matchs ("end distribute parallel do simd", gfc_match_omp_end_nowait, ... + matcho ("end distribute parallel do", gfc_match_omp_end_nowait, I think the above two changes are incorrect. At least looking at 5.1 which is clearer than 5.2, 5.1 [221:17-23] says for C/C++ that while nowait is allowed on worksharing-loop, it is not allowed on combined parallel worksharing-loop, and Fortran has that restriction through the syntax (no [nowait] on !$omp end parallel do). I did look at 5.2 and did miss it – but now after searching more careful: In 5.2 it is hidden in "17.3 Combined and Composite Directive Names" [342:14-16] "If directive-name-A is parallel then directive-name-B may be loop, sections, workshare, masked, for, do or the directive name of a combined or composite construct for which directive-name-A is masked, for or do." and "17.4 Combined Construct Semantics" [343:14-16] "If directive-name-A is parallel, the nowait and in_reduction clauses must not be specified." And for completeness, "nowait" (Sect. 15.6) is permitted for "Directives: dispatch, do, for, interop, scope, sections, single, target, target enter data, target exit data, target update, taskwait, workshare" * * * With the attached patch, the following combined/composite directives accept 'nowait' at 'end': "end critical" "end do simd" "end do" "end scope" "end sections" "end single" "end target parallel" (newly permits nowait) "end target simd" (newly permits nowait) "end target teams distribute simd" (newly permits nowait) "end target teams distribute" (newly permits nowait) "end target teams loop" (newly permits nowait) "end target teams" (newly permits nowait) "end target" "end workshare" and the following don't "end atomic" "end distribute parallel do simd" "end distribute parallel do" "end distribute simd" "end distribute" "end loop" (was completely missing before) "end simd" "end masked taskloop simd" "end masked taskloop" "end masked" "end master taskloop simd" "end master taskloop" "end master" "end ordered" "end parallel do simd" "end parallel do" "end parallel loop" (was completely missing before) "end parallel masked taskloop simd" "end parallel masked taskloop" "end parallel masked" "end parallel master taskloop simd" "end parallel master taskloop" "end parallel master" "end parallel sections" "end parallel workshare" "end parallel" "end target data" "end target parallel do simd" "end target parallel do" "end target parallel loop" (was completely missing before) "end target teams distribute parallel do simd" "end target teams distribute parallel do" "end taskgroup" "end taskloop simd" "end taskloop" "end task" "end teams distribute parallel do simd" "end teams distribute parallel do" "end teams distribute simd" "end teams distribute" "end teams loop" "end teams" * * * [target parallel do simd:] The above seems like a bug in 5.1 standard, haven't checked 5.2. !$omp end target parallel do simd nowait should be IMO valid, but [241:16] mistakenly doesn't list it. It is the same – A="target" → B="simd/parallel/teams" and there B can be also the A in a combined/composite construct, such that A=parallel (see first quote above). + matcho ("end target parallel do", gfc_match_omp_end_nowait, + matcho ("end target parallel loop", gfc_match_omp_end_nowait, + matcho ("end target parallel", gfc_match_omp_end_nowait, Similarly. While the first two are still invalid, in the latter parallel does not appear as "A" and is thus valid in 5.2 in my reading. - matchs ("end target simd", gfc_match_omp_eos_error, ST_OMP_END_TARGET_SIMD); + matchs ("end target simd", gfc_match_omp_end_nowait, ST_OMP_END_TARGET_SIMD); Similarly. Likewise now valid in my reading. Revised version attached – it does follow 5.2 by permitting all 'target' combined/composite constructions which does not contain 'parallel' as "A". — I hope I got it now right (following OMP 5.2). Tobias PS: I note that c-c++-common/gomp/clauses-1.c has the following. I think it follows into the category could be valid (and attached to 'target') but OMP 5.1 and OMP 5.2 do not permit it. [I have no checked other variations, I just saw them fail in the Fortranized version and looked at the original.] * #pragma omp target parallel for ... nowait (twice) * #pragma omp target parallel for simd ... nowait * #pragma omp target teams distribute parallel for ... nowait * #pragma omp target teams distribute parallel for simd .. nowait * #pragma omp target parallel loop ... nowait (twice) * #pragma omp target parallel for ... nowait (twice) * #pragma omp target parallel for simd ... nowait - Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; G
Re: [Patch] Fortran/openmp: Fix '!$omp end'
On Fri, Nov 12, 2021 at 12:01:27PM +0100, Tobias Burnus wrote: > With the attached patch, the following combined/composite > directives accept 'nowait' at 'end': I've filed https://github.com/OpenMP/spec/issues/3184 because I think OpenMP 5.2 got it wrong (and 5.1 got it wrong for the end directives in many other cases too). I believe the general rule should be: 1) non-combined/composite constructs allow nowait when they mention the clause (critical, for/do, scope, sections, single, workshare) 2) for simd, do simd allow it 3) anything combined with target allows it So: > > "end critical" > "end do simd" > "end do" > "end scope" > "end sections" > "end single" > "end target parallel" (newly permits nowait) > "end target simd" (newly permits nowait) > "end target teams distribute simd" (newly permits nowait) > "end target teams distribute" (newly permits nowait) > "end target teams loop" (newly permits nowait) > "end target teams" (newly permits nowait) > "end target" > "end workshare" is ok, but: > and the following don't > > "end target parallel do simd" > "end target parallel do" > "end target parallel loop" (was completely missing before) > "end target teams distribute parallel do simd" > "end target teams distribute parallel do" The above 5 should allow it too. As per e.g. 5.2 [341:23], "The effect of the nowait clause is as if it is applied to the outermost leaf construct that permits it." so even for the above 5 the clause splitting should put nowait on target and not the others and be done with it. Jakub
[committed] fortran: Ignore unused arguments for scalarisation [PR97896]
Committed as r12-5192. The KIND argument of the INDEX intrinsic is a compile time constant that is used at compile time only to resolve to a kind-specific library function. That argument is otherwise completely ignored at runtime, and there is no code generated for it as the library procedure has no kind argument. This confuses the scalarizer which expects to see every argument of elemental functions used when calling a procedure. This change removes the argument from the scalarization lists at the beginning of the scalarization process, so that the argument is completely ignored. This also reverts the existing workaround (commit d09847357b965a2c2cda063827ce362d4c9c86f2 except for its testcase). PR fortran/97896 gcc/fortran/ChangeLog: * intrinsic.c (add_sym_4ind): Remove. (add_functions): Use add_sym4 instead of add_sym4ind. Don’t special case the index intrinsic. * iresolve.c (gfc_resolve_index_func): Use the individual arguments directly instead of the full argument list. * intrinsic.h (gfc_resolve_index_func): Update the declaration accordingly. * trans-decl.c (gfc_get_extern_function_decl): Don’t modify the list of arguments in the case of the index intrinsic. * trans-array.h (gfc_get_intrinsic_for_expr, gfc_get_proc_ifc_for_expr): New. * trans-array.c (gfc_get_intrinsic_for_expr, arg_evaluated_for_scalarization): New. (gfc_walk_elemental_function_args): Add intrinsic procedure as argument. Count arguments. Check arg_evaluated_for_scalarization. * trans-intrinsic.c (gfc_walk_intrinsic_function): Update call. * trans-stmt.c (get_intrinsic_for_code): New. (gfc_trans_call): Update call. gcc/testsuite/ChangeLog: * gfortran.dg/index_5.f90: New. --- gcc/fortran/intrinsic.c | 48 +++-- gcc/fortran/intrinsic.h | 3 +- gcc/fortran/iresolve.c| 21 ++--- gcc/fortran/trans-array.c | 61 ++- gcc/fortran/trans-array.h | 3 ++ gcc/fortran/trans-decl.c | 24 +-- gcc/fortran/trans-intrinsic.c | 1 + gcc/fortran/trans-stmt.c | 20 + gcc/testsuite/gfortran.dg/index_5.f90 | 23 ++ 9 files changed, 121 insertions(+), 83 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/index_5.f90 diff --git a/gcc/fortran/intrinsic.c b/gcc/fortran/intrinsic.c index a5a087be083..2d7d2461fd0 100644 --- a/gcc/fortran/intrinsic.c +++ b/gcc/fortran/intrinsic.c @@ -889,39 +889,6 @@ add_sym_4 (const char *name, gfc_isym_id id, enum klass cl, int actual_ok, bt ty (void *) 0); } -/* Add a symbol to the function list where the function takes 4 - arguments and resolution may need to change the number or - arrangement of arguments. This is the case for INDEX, which needs - its KIND argument removed. */ - -static void -add_sym_4ind (const char *name, gfc_isym_id id, enum klass cl, int actual_ok, - bt type, int kind, int standard, - bool (*check) (gfc_expr *, gfc_expr *, gfc_expr *, gfc_expr *), - gfc_expr *(*simplify) (gfc_expr *, gfc_expr *, gfc_expr *, - gfc_expr *), - void (*resolve) (gfc_expr *, gfc_actual_arglist *), - const char *a1, bt type1, int kind1, int optional1, - const char *a2, bt type2, int kind2, int optional2, - const char *a3, bt type3, int kind3, int optional3, - const char *a4, bt type4, int kind4, int optional4 ) -{ - gfc_check_f cf; - gfc_simplify_f sf; - gfc_resolve_f rf; - - cf.f4 = check; - sf.f4 = simplify; - rf.f1m = resolve; - - add_sym (name, id, cl, actual_ok, type, kind, standard, cf, sf, rf, - a1, type1, kind1, optional1, INTENT_IN, - a2, type2, kind2, optional2, INTENT_IN, - a3, type3, kind3, optional3, INTENT_IN, - a4, type4, kind4, optional4, INTENT_IN, - (void *) 0); -} - /* Add a symbol to the subroutine list where the subroutine takes 4 arguments. */ @@ -2224,11 +2191,11 @@ add_functions (void) /* The resolution function for INDEX is called gfc_resolve_index_func because the name gfc_resolve_index is already used in resolve.c. */ - add_sym_4ind ("index", GFC_ISYM_INDEX, CLASS_ELEMENTAL, ACTUAL_YES, - BT_INTEGER, di, GFC_STD_F77, - gfc_check_index, gfc_simplify_index, gfc_resolve_index_func, - stg, BT_CHARACTER, dc, REQUIRED, ssg, BT_CHARACTER, dc, REQUIRED, - bck, BT_LOGICAL, dl, OPTIONAL, kind, BT_INTEGER, di, OPTIONAL); + add_sym_4 ("index", GFC_ISYM_INDEX, CLASS_ELEMENTAL, ACTUAL_YES, + BT_INTEGER, di, GFC_STD_F77, + gfc_check_index, gfc_simplify_index, gfc_resolve_index_func, + stg, BT_CHARACTER, dc, REQUIRED, ssg, BT_CHARACTER, dc, REQUIRED, + bck, BT_LOGICAL, dl, OPTIONAL, kind, BT_INTEGER, di, OPTIONAL); make_generic ("index", GFC_ISYM_INDEX, GFC_STD_F77); @@ -4531,10 +4498,9 @@ resolve_intrinsic (gfc_intrinsic_sym *specific, gfc_expr *e) arg = e->value.function.actual; - /* Special case hacks for MIN, MAX and INDEX. */ + /* Special case hacks fo
Re: [Patch] Fortran/openmp: Fix '!$omp end'
On 12.11.21 13:02, Jakub Jelinek wrote: 3) anything combined with target allows it ... and puts it on 'target' as it shouldn't be on 'for' or 'do' in 'target ... parallel do/for ...', I'd guess. Updated patch attach. Tobias - Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955 Fortran/openmp: Fix '!$omp end' gcc/fortran/ChangeLog: * parse.c (decode_omp_directive): Fix permitting 'nowait' for some combined directives, add missing 'omp end ... loop'. (gfc_ascii_statement): Fix ST_OMP_END_TEAMS_LOOP result. * openmp.c (resolve_omp_clauses): Add missing combined loop constructs case values to the 'if(directive-name: ...)' check. * trans-openmp.c (gfc_split_omp_clauses): Put nowait on target if first leaf construct accepting it. gcc/testsuite/ChangeLog: * gfortran.dg/gomp/unexpected-end.f90: Update dg-error. * gfortran.dg/gomp/clauses-1.f90: New test. * gfortran.dg/gomp/nowait-2.f90: New test. * gfortran.dg/gomp/nowait-3.f90: New test. gcc/fortran/openmp.c | 3 + gcc/fortran/parse.c | 31 +- gcc/fortran/trans-openmp.c| 9 +- gcc/testsuite/gfortran.dg/gomp/clauses-1.f90 | 667 ++ gcc/testsuite/gfortran.dg/gomp/nowait-2.f90 | 315 ++ gcc/testsuite/gfortran.dg/gomp/unexpected-end.f90 | 12 +- 6 files changed, 1016 insertions(+), 21 deletions(-) diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c index 7b2df0d0be3..2893ab2befb 100644 --- a/gcc/fortran/openmp.c +++ b/gcc/fortran/openmp.c @@ -6232,6 +6232,7 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses, case EXEC_OMP_PARALLEL: case EXEC_OMP_PARALLEL_DO: + case EXEC_OMP_PARALLEL_LOOP: case EXEC_OMP_PARALLEL_MASKED: case EXEC_OMP_PARALLEL_MASTER: case EXEC_OMP_PARALLEL_SECTIONS: @@ -6285,6 +6286,7 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses, case EXEC_OMP_TARGET: case EXEC_OMP_TARGET_TEAMS: case EXEC_OMP_TARGET_TEAMS_DISTRIBUTE: + case EXEC_OMP_TARGET_TEAMS_LOOP: ok = ifc == OMP_IF_TARGET; break; @@ -6312,6 +6314,7 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses, case EXEC_OMP_TARGET_TEAMS_DISTRIBUTE_PARALLEL_DO: case EXEC_OMP_TARGET_PARALLEL: case EXEC_OMP_TARGET_PARALLEL_DO: + case EXEC_OMP_TARGET_PARALLEL_LOOP: ok = ifc == OMP_IF_TARGET || ifc == OMP_IF_PARALLEL; break; diff --git a/gcc/fortran/parse.c b/gcc/fortran/parse.c index 12aa80ec45c..94b677f2a70 100644 --- a/gcc/fortran/parse.c +++ b/gcc/fortran/parse.c @@ -924,6 +924,7 @@ decode_omp_directive (void) matcho ("end distribute", gfc_match_omp_eos_error, ST_OMP_END_DISTRIBUTE); matchs ("end do simd", gfc_match_omp_end_nowait, ST_OMP_END_DO_SIMD); matcho ("end do", gfc_match_omp_end_nowait, ST_OMP_END_DO); + matcho ("end loop", gfc_match_omp_eos_error, ST_OMP_END_LOOP); matchs ("end simd", gfc_match_omp_eos_error, ST_OMP_END_SIMD); matcho ("end masked taskloop simd", gfc_match_omp_eos_error, ST_OMP_END_MASKED_TASKLOOP_SIMD); @@ -939,6 +940,8 @@ decode_omp_directive (void) matchs ("end parallel do simd", gfc_match_omp_eos_error, ST_OMP_END_PARALLEL_DO_SIMD); matcho ("end parallel do", gfc_match_omp_eos_error, ST_OMP_END_PARALLEL_DO); + matcho ("end parallel loop", gfc_match_omp_eos_error, + ST_OMP_END_PARALLEL_LOOP); matcho ("end parallel masked taskloop simd", gfc_match_omp_eos_error, ST_OMP_END_PARALLEL_MASKED_TASKLOOP_SIMD); matcho ("end parallel masked taskloop", gfc_match_omp_eos_error, @@ -960,24 +963,29 @@ decode_omp_directive (void) matcho ("end sections", gfc_match_omp_end_nowait, ST_OMP_END_SECTIONS); matcho ("end single", gfc_match_omp_end_single, ST_OMP_END_SINGLE); matcho ("end target data", gfc_match_omp_eos_error, ST_OMP_END_TARGET_DATA); - matchs ("end target parallel do simd", gfc_match_omp_eos_error, + matchs ("end target parallel do simd", gfc_match_omp_end_nowait, ST_OMP_END_TARGET_PARALLEL_DO_SIMD); - matcho ("end target parallel do", gfc_match_omp_eos_error, + matcho ("end target parallel do", gfc_match_omp_end_nowait, ST_OMP_END_TARGET_PARALLEL_DO); - matcho ("end target parallel", gfc_match_omp_eos_error, + matcho ("end target parallel loop", gfc_match_omp_end_nowait, + ST_OMP_END_TARGET_PARALLEL_LOOP); + matcho ("end target parallel", gfc_match_omp_end_nowait, ST_OMP_END_TARGET_PARALLEL); - matchs ("end target simd", gfc_match_omp_eos_error, ST_OMP_END_TARGET_SIMD); + matchs ("end target simd", gfc_match_omp_end_nowait, ST_OMP_END_TARGET_SIMD); m
Re: [Patch] Fortran/openmp: Fix '!$omp end'
On Fri, Nov 12, 2021 at 04:56:37PM +0100, Tobias Burnus wrote: > Fortran/openmp: Fix '!$omp end' > > gcc/fortran/ChangeLog: > > * parse.c (decode_omp_directive): Fix permitting 'nowait' for some > combined directives, add missing 'omp end ... loop'. > (gfc_ascii_statement): Fix ST_OMP_END_TEAMS_LOOP result. > * openmp.c (resolve_omp_clauses): Add missing combined loop constructs > case values to the 'if(directive-name: ...)' check. > * trans-openmp.c (gfc_split_omp_clauses): Put nowait on target if > first leaf construct accepting it. > > gcc/testsuite/ChangeLog: > > * gfortran.dg/gomp/unexpected-end.f90: Update dg-error. > * gfortran.dg/gomp/clauses-1.f90: New test. > * gfortran.dg/gomp/nowait-2.f90: New test. > * gfortran.dg/gomp/nowait-3.f90: New test. Mostly good, except: > @@ -6132,10 +6134,9 @@ gfc_split_omp_clauses (gfc_code *code, > if (mask & GFC_OMP_MASK_TEAMS && innermost != GFC_OMP_MASK_TEAMS) > gfc_add_clause_implicitly (&clausesa[GFC_OMP_SPLIT_TEAMS], > code->ext.omp_clauses, false, false); > - if (((mask & (GFC_OMP_MASK_PARALLEL | GFC_OMP_MASK_DO)) > - == (GFC_OMP_MASK_PARALLEL | GFC_OMP_MASK_DO)) > - && !is_loop) > -clausesa[GFC_OMP_SPLIT_DO].nowait = true; > + if ((mask & (GFC_OMP_MASK_PARALLEL | GFC_OMP_MASK_DO)) > + == (GFC_OMP_MASK_PARALLEL | GFC_OMP_MASK_DO)) > +clausesa[GFC_OMP_SPLIT_DO].nowait = false; > } this. In the standard, yes, for parallel {do,sections,workshare} indeed the do/sections/workshare doesn't get nowait (either it is not allowed to specify it at all, or if combined with target, nowait should go to target and nothing else). But, for the middle-end, we actually want nowait true whenever a worksharing construct is combined with parallel, because when the worksharing construct ends, doing a barrier there will mean we wait, then immediately get to the implicit barrier at the end of parallel. c_omp_split_clauses does: /* Add implicit nowait clause on #pragma omp parallel {for,for simd,sections}. */ if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_THREADS)) != 0) switch (code) { case OMP_FOR: case OMP_SIMD: if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_SCHEDULE)) != 0) cclauses[C_OMP_CLAUSE_SPLIT_FOR] = build_omp_clause (loc, OMP_CLAUSE_NOWAIT); break; case OMP_SECTIONS: cclauses[C_OMP_CLAUSE_SPLIT_SECTIONS] = build_omp_clause (loc, OMP_CLAUSE_NOWAIT); break; default: break; } and I think the previous code did exactly that. So, the patch is ok for trunk without the above hunk. Jakub
[PATCH] PR fortran/102368 - Failure to compile program using the C_SIZEOF function in ISO_C_BINDING
Dear Fortranners, F2008:15.3.5 relaxed the condition on interoperable character variables and now allows values different from one. Similar text in F2018:18.3.4. This required an adjustment in the interoperability check. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 1fc44a5bf0b294021490f3c0a1539982a09000f5 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Fri, 12 Nov 2021 18:32:18 +0100 Subject: [PATCH] Fortran: fix interoperability check for character variables for F2008 gcc/fortran/ChangeLog: PR fortran/102368 * check.c (is_c_interoperable): F2008:15.3.5 relaxed the condition on interoperable character variables and allows values different from one. gcc/testsuite/ChangeLog: PR fortran/102368 * gfortran.dg/c_sizeof_7.f90: New test. --- gcc/fortran/check.c | 20 ++-- gcc/testsuite/gfortran.dg/c_sizeof_7.f90 | 13 + 2 files changed, 27 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/c_sizeof_7.f90 diff --git a/gcc/fortran/check.c b/gcc/fortran/check.c index ffa07b510cd..69a2e35e81b 100644 --- a/gcc/fortran/check.c +++ b/gcc/fortran/check.c @@ -5272,13 +5272,21 @@ is_c_interoperable (gfc_expr *expr, const char **msg, bool c_loc, bool c_f_ptr) && !gfc_simplify_expr (expr->ts.u.cl->length, 0)) gfc_internal_error ("is_c_interoperable(): gfc_simplify_expr failed"); -if (!c_loc && expr->ts.u.cl - && (!expr->ts.u.cl->length - || expr->ts.u.cl->length->expr_type != EXPR_CONSTANT - || mpz_cmp_si (expr->ts.u.cl->length->value.integer, 1) != 0)) +if (!c_loc && expr->ts.u.cl) { - *msg = "Type shall have a character length of 1"; - return false; + bool len_ok = (expr->ts.u.cl->length + && expr->ts.u.cl->length->expr_type == EXPR_CONSTANT); + + /* F2003:15.2.1 required the length of a character variable to be one. + F2008:15.3.5 relaxed this to constant length. */ + if (len_ok && !(gfc_option.allow_std & GFC_STD_F2008)) + len_ok = mpz_cmp_si (expr->ts.u.cl->length->value.integer, 1) == 0; + + if (!len_ok) + { + *msg = "Type shall have a character length of 1"; + return false; + } } } diff --git a/gcc/testsuite/gfortran.dg/c_sizeof_7.f90 b/gcc/testsuite/gfortran.dg/c_sizeof_7.f90 new file mode 100644 index 000..3cfa3371f72 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/c_sizeof_7.f90 @@ -0,0 +1,13 @@ +! { dg-do compile } +! { dg-options "-std=f2008 -fdump-tree-original" } +! { dg-final { scan-tree-dump-times "_gfortran_stop_numeric" 0 "original" } } +! PR fortran/102368 + +program main + use, intrinsic :: iso_c_binding + implicit none + character(kind=c_char, len=*), parameter :: a = 'abc' + character(kind=c_char, len=8):: b + if (c_sizeof (a) /= 3) stop 1 + if (c_sizeof (b) /= 8) stop 2 +end program main -- 2.26.2
Re: [PATCH] PR fortran/102368 - Failure to compile program using the C_SIZEOF function in ISO_C_BINDING
On Fri, 12 Nov 2021 18:39:48 +0100 Harald Anlauf via Fortran wrote: Sounds plausible. Nits: > diff --git a/gcc/testsuite/gfortran.dg/c_sizeof_7.f90 > b/gcc/testsuite/gfortran.dg/c_sizeof_7.f90 > new file mode 100644 > index 000..3cfa3371f72 > --- /dev/null > +++ b/gcc/testsuite/gfortran.dg/c_sizeof_7.f90 [I'd name this .f08, no?] > @@ -0,0 +1,13 @@ > +! { dg-do compile } > +! { dg-options "-std=f2008 -fdump-tree-original" } [and drop the -std] > +! { dg-final { scan-tree-dump-times "_gfortran_stop_numeric" 0 "original" } } [ ...-times 0 == scan-tree-dump-not ] > +! PR fortran/102368 > + > +program main > + use, intrinsic :: iso_c_binding > + implicit none > + character(kind=c_char, len=*), parameter :: a = 'abc' > + character(kind=c_char, len=8):: b character(kind=c_char, len=-42) :: c ! { dg-error "positive integer greater than 0" } character(kind=c_char, len=-0) :: d ! { dg-error "positive integer greater than 0" } character(kind=c_char, len=0) :: e ! { dg-error "positive integer greater than 0" } character(kind=c_char, len=+0) :: f ! { dg-error "positive integer greater than 0" } character(kind=c_char, len=0.0d) :: g ! { dg-error "positive integer greater than 0" } character(kind=c_char, len=3.) :: h ! { dg-error "positive integer greater than 0" } character(kind=c_char, len=.031415e2) :: i ! { dg-error "positive integer greater than 0" } ... are caught elsewhere if one assumes that len should be a positive int > 0 (didn't look) Also did not look if character(kind=c_char, len=SELECTED_REAL_KIND(10)) :: j ! is that constant? Should it be? > + if (c_sizeof (a) /= 3) stop 1 > + if (c_sizeof (b) /= 8) stop 2 indeed. cheers, > +end program main > -- > 2.26.2 >
Re: [PATCH] PR fortran/102368 - Failure to compile program using the C_SIZEOF function in ISO_C_BINDING
Hi Bernhard, Am 12.11.21 um 21:18 schrieb Bernhard Reutner-Fischer via Fortran: On Fri, 12 Nov 2021 18:39:48 +0100 Harald Anlauf via Fortran wrote: Sounds plausible. this is what I thought, too. And nvfortran and flang accept the testcase, as well as crayftn (cce/12.0.2). Intel accepts the first case (a), but rejects the second (b). I asked in the Intel forum. Steve Lionel doubts that the code is valid. There might be some confusion on my side, but having Cray on my side feels good. (Although the PR was entered into bugzilla by a Cray employee). Nits: diff --git a/gcc/testsuite/gfortran.dg/c_sizeof_7.f90 b/gcc/testsuite/gfortran.dg/c_sizeof_7.f90 new file mode 100644 index 000..3cfa3371f72 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/c_sizeof_7.f90 [I'd name this .f08, no?] @@ -0,0 +1,13 @@ +! { dg-do compile } +! { dg-options "-std=f2008 -fdump-tree-original" } [and drop the -std] +! { dg-final { scan-tree-dump-times "_gfortran_stop_numeric" 0 "original" } } [ ...-times 0 == scan-tree-dump-not ] Good point. +! PR fortran/102368 + +program main + use, intrinsic :: iso_c_binding + implicit none + character(kind=c_char, len=*), parameter :: a = 'abc' + character(kind=c_char, len=8):: b character(kind=c_char, len=-42) :: c ! { dg-error "positive integer greater than 0" } character(kind=c_char, len=-0) :: d ! { dg-error "positive integer greater than 0" } character(kind=c_char, len=0) :: e ! { dg-error "positive integer greater than 0" } character(kind=c_char, len=+0) :: f ! { dg-error "positive integer greater than 0" } character(kind=c_char, len=0.0d) :: g ! { dg-error "positive integer greater than 0" } character(kind=c_char, len=3.) :: h ! { dg-error "positive integer greater than 0" } character(kind=c_char, len=.031415e2) :: i ! { dg-error "positive integer greater than 0" } ... are caught elsewhere if one assumes that len should be a positive int > 0 (didn't look) Also did not look if character(kind=c_char, len=SELECTED_REAL_KIND(10)) :: j ! is that constant? Should it be? These things should already be handled in general and elsewhere, as they are not about interoperability. + if (c_sizeof (a) /= 3) stop 1 + if (c_sizeof (b) /= 8) stop 2 indeed. cheers, +end program main -- 2.26.2 Thanks, Harald
Re: [PATCH] PR fortran/102368 - Failure to compile program using the C_SIZEOF function in ISO_C_BINDING
On Fri, 12 Nov 2021 21:35:42 +0100 Harald Anlauf wrote: > Hi Bernhard, > > Am 12.11.21 um 21:18 schrieb Bernhard Reutner-Fischer via Fortran: > > On Fri, 12 Nov 2021 18:39:48 +0100 > > Harald Anlauf via Fortran wrote: > > > > Sounds plausible. > > this is what I thought, too. And nvfortran and flang accept the > testcase, as well as crayftn (cce/12.0.2). Don't know about the former 2, but cray is really the MIPS compiler (open64 / pathscale etc), no? I always liked the SGI compiler (it was marvellous in mem addressing really, at least in my cases in former times) but i'm not familiar with the other two so cannot deduct anything from their opinion TBH. > > Intel accepts the first case (a), but rejects the second (b). > I asked in the Intel forum. Steve Lionel doubts that the code is > valid. On what grounds does Steve L. think it's invalid? Missing initializer to rectify the len=8? If so, what's the reasoning to doubt that? > > There might be some confusion on my side, but having Cray on my > side feels good. (Although the PR was entered into bugzilla by > a Cray employee). Vendors. Well. It would certainly be the first time a vendor was not entirely correct. IME vendors tended to favour compatibility over correctness more often than not. This certainly may have, erm, has changed. > > are caught elsewhere if one assumes that len should be a positive int > 0 > > (didn't look) > > Also did not look if > > character(kind=c_char, len=SELECTED_REAL_KIND(10)) :: j ! is that constant? > > Should it be? > > These things should already be handled in general and > elsewhere, as they are not about interoperability. Excellent. I'd ACK your patch then but i cannot approve it. Thanks for the patch and cheers,
Re: [PATCH] PR fortran/102368 - Failure to compile program using the C_SIZEOF function in ISO_C_BINDING
Hi Bernhard, Am 12.11.21 um 22:58 schrieb Bernhard Reutner-Fischer via Fortran: On Fri, 12 Nov 2021 21:35:42 +0100 Harald Anlauf wrote: Hi Bernhard, Am 12.11.21 um 21:18 schrieb Bernhard Reutner-Fischer via Fortran: On Fri, 12 Nov 2021 18:39:48 +0100 Harald Anlauf via Fortran wrote: Sounds plausible. this is what I thought, too. And nvfortran and flang accept the testcase, as well as crayftn (cce/12.0.2). Don't know about the former 2, but cray is really the MIPS compiler (open64 / pathscale etc), no? I thought that most of Cray's compilers was their own development targeting their vector hardware. My own experience is limited to their compilers on x86-type systems (+ GPUs). They seemed to be rather progressive in developing the Fortran standard as well as implementing it in their own compilers, besides supporting open standards. (That seemed to slow down w.r.t. OpenACC). I always liked the SGI compiler (it was marvellous in mem addressing really, at least in my cases in former times) but i'm not familiar with the other two so cannot deduct anything from their opinion TBH. Intel accepts the first case (a), but rejects the second (b). I asked in the Intel forum. Steve Lionel doubts that the code is valid. On what grounds does Steve L. think it's invalid? Missing initializer to rectify the len=8? If so, what's the reasoning to doubt that? See: https://community.intel.com/t5/Intel-Fortran-Compiler/Interoperability-of-character-variables/td-p/1329554 There might be some confusion on my side, but having Cray on my side feels good. (Although the PR was entered into bugzilla by a Cray employee). Vendors. Well. It would certainly be the first time a vendor was not entirely correct. IME vendors tended to favour compatibility over correctness more often than not. This certainly may have, erm, has changed. Well, Cray not only sells hardware, but systems with support. You get multiple programming environments on their systems, one of which is likely cce (Cray Compilation Environment), and very often the GNU compilers. Depending on the contract you get in addition Intel, Nvidia, ... Users may report issues with these components through their support contract. With GNU, I guess they just add a PR to bugzilla; with commercial software vendors their might be different ways. are caught elsewhere if assumes that len should be a positive int > 0 (didn't look) Also did not look if character(kind=c_char, len=SELECTED_REAL_KIND(10)) :: j ! is that constant? Should it be? These things should already be handled in general and elsewhere, as they are not about interoperability. Excellent. I'd ACK your patch then but i cannot approve it. Thanks for the patch and cheers, There'll be a way to resolve this PR. Maybe Tobias or Thomas have an opinion. There are strange ways in the standard anyway to pass Fortran character strings to BIND(C) procedures. Look e.g. at "5.5.2.11 Sequence association" which sort of hacks this situation for some applications relevant to me. Cheers, Harald