RE: [wwwdocs] benchmarks/ -- cp2k.berlios.de/gfortran/ seems gone?
Hi Gerald, cp2k moved to https://www.cp2k.org/ , but isn't running a nightly gcc trunk tester anymore. So, yes, patch is OK, unfortunately. Joost
RE: [PATCH][COMMITTED] Revert r238497 because of PR 71961.
Thanks.. I wonder if you could add the testcase in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71961#c11 to the testsuite, as it catches the underlying issue. Regards, Joost VandeVondele
RE: [PATCH, fortran, v4] Use Levenshtein spelling suggestions in Fortran FE
From my point of view, would be really nice to have. Joost
RE: [PATCH] RFC: Use Levenshtein spelling suggestions in Fortran FE
So, I have tested the patch, it seems to work well. I would really like to see this feature in the compiler, I'm sure it will help people developing Fortran code. I have already an enhancement request, catching the name of 'Keyword argument' : > cat test.f90 MODULE test CONTAINS SUBROUTINE foo(bar) INTEGER :: bar END SUBROUTINE END MODULE USE test CALL foo(baz=1) END
Re: [PATCH] RFC: Use Levenshtein spelling suggestions in Fortran FE
Today, I ran 'gfortran -static-libfortran test.f90' and was very pleased with the answer: gfortran: error: unrecognized command line option ‘-static-libfortran’; did you mean ‘-static-libgfortran’? So thanks David, and hopefully we get this user experience for the FE as well. Joost
Re: [PATCH] RFC: Enable graphite at -O3 -fprofile_use
I'm all in favour of requiring isl and enabling graphite by default, but would suggest to enable it with -Ofast instead. One reason is that certainly extracting testcases from a PGO build is more difficult, and initially there will certainly be miscompiles with graphite (CP2K is right now). Furthermore, unless graphite is particularly effective with PGO (does it use average loop trip counts already?), I don't see a particular connection. Joost
RE: [PATCH] PR67518 and PR53852 -- add testcase.
r229967 2015-11-08 Joost VandeVondele * gfortran.dg/PR67518.f90: move from here... * gfortran.dg/graphite/PR67518.f90: to here. * gfortran.dg/PR53852.f90: move from here... * gfortran.dg/graphite/PR53852.f90: to here.
RE: [PATCH] PR67518 and PR53852 -- add testcase.
I see, graphite is optional. Trying to find the dejagnu-ery, I think the obvious thing is to move the tests from gfortran.dg/ to gfortran.dg/graphite/ I'll do that under the obvious rule, unless this get's preapproved before that ...
RE: [PATCH] PR67518 and PR53852 -- add testcase.
Thanks Paul. I believe PR53852 won't be fixed on 4.9/5 as it seems to depend on the recent graphite cleanup work and recent isl. As such I'll commit to trunk only.
RE: [PATCH] PR67518 and PR53852 -- add testcase.
> Attached testcases for two previously fixed PRs (and thanks to Dominique who > was quicker for PR67982). ping ?
[PATCH] PR67518 and PR53852 -- add testcase.
Attached testcases for two previously fixed PRs (and thanks to Dominique who was quicker for PR67982). 2015-11-03 Joost VandeVondele PR middle-end/53852 PR middle-end/67518 * gfortran.dg/PR67518.f90: New test. * gfortran.dg/PR53852.f90: New test. OK for trunk after finished bootstrap and testing ? Joost patch.prs Description: patch.prs
RE: [PATCH] select isl-0.15 in download_prerequisites
> can isl-0.15 be built with 4.8 and 4.9? ..with an old libc? isl-0.15 builds for me with gcc 4.6.4, 4.7.4, 4.8.3, 4.9.2, not sure about older libc. I can't find any reference to max_align_t in the isl-0.15 source. (for me actually also isl-0.14 builds fine down to gcc 4.6). Maybe somebody with an older toolchain could test ?
RE: [PATCH] select isl-0.15 in download_prerequisites
> I think it is fine to bump the default version of isl to be downloaded by > default. > Are there other reviewers who would oppose committing this patch? So, OK for trunk ?
[PATCH] PR middle-end/68002: introduce -fkeep-static-functions
In some cases (e.g. coverage testing) it is useful to emit code for static
functions even if they are never used, which currently is not possible at -O1
and above. The following patch introduces a flag for this, which basically
triggers the same code that keeps those functions alive at -O0. Thanks to Marc
Glisse for replying at gcc-help and for suggesting where to look.
Bootstrapped and regtested on x86_64-unknown-linux-gnu
OK for trunk ?
Joostgcc/ChangeLog:
2015-10-17 Joost VandeVondele
PR middle-end/68002
* common.opt (fkeep-static-functions): New option.
* doc/invoke.texi: Document it.
* cgraphunit.c (cgraph_node::finalize_function): Use it.
gcc/testsuite/ChangeLog:
2015-10-17 Joost VandeVondele
PR middle-end/68002
* gcc.dg/PR68002.c: New test.
Index: gcc/doc/invoke.texi
===
--- gcc/doc/invoke.texi (revision 228932)
+++ gcc/doc/invoke.texi (working copy)
@@ -410,8 +410,8 @@ Objective-C and Objective-C++ Dialects}.
-fira-loop-pressure -fno-ira-share-save-slots @gol
-fno-ira-share-spill-slots -fira-verbose=@var{n} @gol
-fisolate-erroneous-paths-dereference -fisolate-erroneous-paths-attribute @gol
--fivopts -fkeep-inline-functions -fkeep-static-consts @gol
--flive-range-shrinkage @gol
+-fivopts -fkeep-inline-functions -fkeep-static-functions @gol
+-fkeep-static-consts -flive-range-shrinkage @gol
-floop-block -floop-interchange -floop-strip-mine @gol
-floop-unroll-and-jam -floop-nest-optimize @gol
-floop-parallelize-all -flra-remat -flto -flto-compression-level @gol
@@ -8013,6 +8013,11 @@ of its callers. This switch does not af
@code{extern inline} extension in GNU C90@. In C++, emit any and all
inline functions into the object file.
+@item -fkeep-static-functions
+@optindex fkeep-static-functions
+Emit @code{static} functions into the object file, even if the function
+is never used.
+
@item -fkeep-static-consts
@opindex fkeep-static-consts
Emit variables declared @code{static const} when optimization isn't turned
Index: gcc/cgraphunit.c
===
--- gcc/cgraphunit.c (revision 228932)
+++ gcc/cgraphunit.c (working copy)
@@ -451,7 +451,7 @@ cgraph_node::finalize_function (tree dec
declared inline and nested functions. These were optimized out
in the original implementation and it is unclear whether we want
to change the behavior here. */
- if ((!opt_for_fn (decl, optimize)
+ if (((!opt_for_fn (decl, optimize) || flag_keep_static_functions)
&& !node->cpp_implicit_alias
&& !DECL_DISREGARD_INLINE_LIMITS (decl)
&& !DECL_DECLARED_INLINE_P (decl)
Index: gcc/testsuite/gcc.dg/PR68002.c
===
--- gcc/testsuite/gcc.dg/PR68002.c (revision 0)
+++ gcc/testsuite/gcc.dg/PR68002.c (revision 0)
@@ -0,0 +1,7 @@
+/* Ensure static functions can be kept. */
+/* { dg-do compile } */
+/* { dg-options "-O1 -fkeep-static-functions" } */
+
+static void bar () { }
+
+/* { dg-final { scan-assembler "bar" } } */
Index: gcc/common.opt
===
--- gcc/common.opt (revision 228932)
+++ gcc/common.opt (working copy)
@@ -1589,6 +1589,10 @@ fkeep-inline-functions
Common Report Var(flag_keep_inline_functions)
Generate code for functions even if they are fully inlined
+fkeep-static-functions
+Common Report Var(flag_keep_static_functions)
+Generate code for static functions even if they are never called
+
fkeep-static-consts
Common Report Var(flag_keep_static_consts) Init(1)
Emit static const variables even if they are not used
RE: [PATCH] select isl-0.15 in download_prerequisites
> can 4.8 and 4.9 be built with isl-0.15? The patch only influences an in-tree build of isl for trunk, so does it matter ?
[PATCH] select isl-0.15 in download_prerequisites
For the recent fix of PR53852, isl-0.15 is needed, which is already available at ftp://gcc.gnu.org/pub/gcc/infrastructure/ . Thus, it seems to make sense to update the download_prerequisites script, as done with the attached patch. OK for trunk ? JoostIndex: contrib/download_prerequisites === --- contrib/download_prerequisites (revision 227480) +++ contrib/download_prerequisites (working copy) @@ -43,7 +43,7 @@ ln -sf $MPC mpc || exit 1 # Necessary to build GCC with the Graphite loop optimizations. if [ "$GRAPHITE_LOOP_OPT" = "yes" ] ; then - ISL=isl-0.14 + ISL=isl-0.15 wget ftp://gcc.gnu.org/pub/gcc/infrastructure/$ISL.tar.bz2 || exit 1 tar xjf $ISL.tar.bz2 || exit 1 contrib/ChangeLog: 2015-09-04 Joost VandeVondele * download_prerequisites: update to isl-0.15.
RE: Fix PR libgomp/66761 and PR libgomp/67303 (tsan warnings)
Thanks, committed to trunk r227119 with the format fix. Do you want this on the 5 branch as well ? Joost
Fix PR libgomp/66761 and PR libgomp/67303 (tsan warnings)
The following fixes two warnings reported by tsan when running OMP'ed code. As
suggested by Andrew Pinski in PR67303 for gomp_iter_guided_next, by employing a
relaxed atomic load. The same pattern occurred a couple of more times, so fixed
as well. I used the same approach for the warning in do_spin (PR66761), even
though this is a different context.
Bootstrapped and regtested on x86_64-unknown-linux-gnu for trunk, and manually
verified that the PR is fixed using a tsan compiled libgomp based on gcc-5.1.
OK for trunk ?
This applies without changes to the 5 branch, should I commit there as well ?
libgomp/ChangeLog:
2015-08-22 Joost VandeVondele
PR libgomp/66761
PR libgomp/67303
* iter.c (gomp_iter_dynamic_next): Employ an atomic load.
(gomp_iter_guided_next): Idem.
* iter_ull.c (gomp_iter_ull_dynamic_next): Idem.
(gomp_iter_ull_guided_next): Idem.
* config/linux/wait.h (do_spin): Idem.
Index: libgomp/iter.c
===
--- libgomp/iter.c (revision 227094)
+++ libgomp/iter.c (working copy)
@@ -218,7 +218,7 @@ gomp_iter_dynamic_next (long *pstart, lo
}
}
- start = ws->next;
+ start = __atomic_load_n (&ws->next, MEMMODEL_RELAXED);
while (1)
{
long left = end - start;
@@ -301,7 +301,7 @@ gomp_iter_guided_next (long *pstart, lon
long start, end, nend, incr;
unsigned long chunk_size;
- start = ws->next;
+ start = __atomic_load_n (&ws->next, MEMMODEL_RELAXED);
end = ws->end;
incr = ws->incr;
chunk_size = ws->chunk_size;
Index: libgomp/iter_ull.c
===
--- libgomp/iter_ull.c (revision 227094)
+++ libgomp/iter_ull.c (working copy)
@@ -219,7 +219,7 @@ gomp_iter_ull_dynamic_next (gomp_ull *ps
}
}
- start = ws->next_ull;
+ start = __atomic_load_n (&ws->next_ull, MEMMODEL_RELAXED);
while (1)
{
gomp_ull left = end - start;
@@ -305,7 +305,7 @@ gomp_iter_ull_guided_next (gomp_ull *pst
gomp_ull start, end, nend, incr;
gomp_ull chunk_size;
- start = ws->next_ull;
+ start = __atomic_load_n (&ws->next_ull, MEMMODEL_RELAXED);
end = ws->end_ull;
incr = ws->incr_ull;
chunk_size = ws->chunk_size_ull;
Index: libgomp/config/linux/wait.h
===
--- libgomp/config/linux/wait.h (revision 227094)
+++ libgomp/config/linux/wait.h (working copy)
@@ -49,7 +49,8 @@ static inline int do_spin (int *addr, in
{
unsigned long long i, count = gomp_spin_count_var;
- if (__builtin_expect (gomp_managed_threads > gomp_available_cpus, 0))
+ if (__builtin_expect (__atomic_load_n (&gomp_managed_threads,
+ MEMMODEL_RELAXED) > gomp_available_cpus, 0))
count = gomp_throttled_spin_count_var;
for (i = 0; i < count; i++)
if (__builtin_expect (__atomic_load_n (addr, MEMMODEL_RELAXED) != val, 0))
the nvptx port
Hi Bernd, reading the patches, it seems like there is no mention of sm_35, only sm_30. So, I'm wondering what 'sub'targets will initially be supported, and if/how/when various processors will be selected. Thanks, Joost
RE: [PATCH] Introduce warning -Womp-default-scope
> a) I don't like the option name, -Womp-no-default-clause would be IMHO better a bit related to b), I first had -Womp-no-default-none-clause but it becomes long and contains twice 'no' (also consider the no-omp-no-default... form). If you like that more, I'll make that change (see below) otherwise I retain the current form. > b) I think you shouldn't warn for explicit default(shared), or > default(firstprivate) or default(private) clauses, there user explicitly > tells what should happen for the non-listed vars (and it is pretty rare) I would still like to warn for those, default(shared) is equivalent to just leaving it out, and unfortunately, I have observed bugs with explicit default(shared/private). Usually caused by people updating code in large parallel sections, not realizing that they are in an OMP region. This is essentially what I would like to catch and to (effectively) enforce default(none). It is somewhat similar to the -fimplicit-none option in the Fortran FE. > c) in the documentation I think you should make it clear that it is just a >coding style warning (dunno if we have some verbiage for such warnings) Like so (and associated code/test changes) ? Index: gcc/common.opt === --- gcc/common.opt (revision 215323) +++ gcc/common.opt (working copy) @@ -591,6 +591,10 @@ Wodr Common Var(warn_odr_violations) Init(1) Warning Warn about some C++ One Definition Rule violations during link time optimization +Womp-no-default-none-clause +Common Var(warn_omp_no_default_none_clause) Warning +Warn for error-prone code style in which an OMP parallel/task/teams construct has no explicit default(none) clause. + Woverflow Common Var(warn_overflow) Init(1) Warning Warn about overflow in arithmetic expressions
[PATCH] Introduce warning -Womp-default-scope
Attached patch introduces an optional warning for an OMP parallel/task/teams
construct without explicit default(none). This would effectively catch the
number one reason of easy-to-avoid OMP bugs in our project.
Tested with check-fortran, full bootstrap & check in progress. OK for trunk if
this passes ?
JoostIndex: gcc/testsuite/gfortran.dg/gomp/omp-default-scope.f90
===
--- gcc/testsuite/gfortran.dg/gomp/omp-default-scope.f90 (revision 0)
+++ gcc/testsuite/gfortran.dg/gomp/omp-default-scope.f90 (revision 0)
@@ -0,0 +1,11 @@
+! { dg-do compile }
+! { dg-options "-fopenmp -Womp-default-scope" }
+ SUBROUTINE S1(d)
+IMPLICIT NONE
+INTEGER :: i,d(*),c
+!$OMP PARALLEL DO ! { dg-warning "construct without explicit" }
+DO i=1,100
+ c= d(i)
+ d(i)=c*c
+ENDDO
+ END SUBROUTINE S1
Index: gcc/gimplify.c
===
--- gcc/gimplify.c (revision 215323)
+++ gcc/gimplify.c (working copy)
@@ -6255,6 +6255,13 @@ gimplify_scan_omp_clauses (tree *list_p,
list_p = &OMP_CLAUSE_CHAIN (c);
}
+ if (ctx->default_kind != OMP_CLAUSE_DEFAULT_NONE
+ && (ctx->region_type & ORT_PARALLEL
+ || ctx->region_type & ORT_TASK
+ || ctx->region_type == ORT_TEAMS))
+warning_at (ctx->location, OPT_Womp_default_scope,
+ "OMP parallel/task/teams construct without explicit default(none)");
+
gimplify_omp_ctxp = ctx;
}
Index: gcc/common.opt
===
--- gcc/common.opt (revision 215323)
+++ gcc/common.opt (working copy)
@@ -591,6 +591,10 @@ Wodr
Common Var(warn_odr_violations) Init(1) Warning
Warn about some C++ One Definition Rule violations during link time optimization
+Womp-default-scope
+Common Var(warn_omp_default_scope) Warning
+Warn if an OMP parallel/task/teams construct has no explicit default(none) clause.
+
Woverflow
Common Var(warn_overflow) Init(1) Warning
Warn about overflow in arithmetic expressions
gcc/ChangeLog:
2014-09-23 Joost VandeVondele
* gimplify.c (gimplify_scan_omp_clauses): Introduce new warning.
* common.opt (Womp-default-scope): New flag.
gcc/testsuite/ChangeLog:
2014-09-23 Joost VandeVondele
* gfortran.dg/gomp/omp-default-scope.f90: New test.
RE: [PATCH] Fix whitespace in comments.
Hi Tobias, > However, I wanted to point out that seemingly trivial and obviously > correct patches can have a downside. (One can still do such changes, but > at least one should have weighted them against the downside.) I agree, that's why I asked explicitly. Joost
[PATCH] Fix whitespace in comments.
A somewhat trivial patch to cleanup whitespace issues in comments: sed "s/\.
\*\//\. \*\//g"
Tested with a recompile only.
Ok for trunk ?
gcc/fortran/ChangeLog:
2014-09-20 Joost VandeVondele
* trans-expr.c (gfc_reset_vptr): Fix comment whitespace.
(gfc_conv_class_to_class): Likewise.
(gfc_conv_procedure_call): Likewise.
(arrayfunc_assign_needs_temporary): Likewise.
(realloc_lhs_loop_for_fcn_call): Likewise.
(gfc_trans_assignment_1): Likewise.
* trans-array.c (gfc_conv_array_ref): Likewise.
(gfc_array_allocate): Likewise.
(gfc_alloc_allocatable_for_assignment): Likewise.
* symbol.c (generate_isocbinding_symbol): Likewise.
* class.c (finalization_scalarizer): Likewise.
(finalizer_insert_packed_call): Likewise.
(generate_finalization_wrapper): Likewise.
(find_intrinsic_vtab): Likewise.
* decl.c (gfc_match_import): Likewise.
(match_procedure_decl): Likewise.
(gfc_match_subroutine): Likewise.
(gfc_match_bind_c): Likewise.
(gfc_match_volatile): Likewise.
* trans-common.c (create_common): Likewise.
* error.c (gfc_diagnostic_starter): Likewise.
* trans-stmt.c (gfc_trans_sync): Likewise.
(gfc_trans_critical): Likewise.
(gfc_trans_simple_do): Likewise.
(gfc_trans_do): Likewise.
(gfc_trans_where_assign): Likewise.
* expr.c (gfc_is_simply_contiguous): Likewise.
* module.c (unquote_string): Likewise.
* trans.c (gfc_add_finalizer_call): Likewise.
* trans-types.c (gfc_init_kinds): Likewise.
* scanner.c (preprocessor_line): Likewise.
* gfortranspec.c (lang_specific_driver): Likewise.
* frontend-passes.c (create_var): Likewise.
(cfe_expr_0): Likewise.
* resolve.c (check_host_association): Likewise.
(gfc_resolve_code): Likewise.
(resolve_fl_derived0): Likewise.
(resolve_symbol): Likewise.
* f95-lang.c (poplevel): Likewise.
* trans-decl.c (create_main_function): Likewise.
* trans-io.c (transfer_expr): Likewise.
* arith.c (gfc_arith_divide): Likewise.
* parse.c (resolve_all_program_units): Likewise.
* check.c (gfc_check_rank): Likewise.
(gfc_check_sizeof): Likewise.
(is_c_interoperable): Likewise.
* dependency.c (gfc_dep_difference): Likewise.
* primary.c (gfc_match_rvalue): Likewise.
* trans-intrinsic.c (conv_intrinsic_system_clock): Likewise.
(conv_isocbinding_subroutine): Likewise.
* options.c (gfc_post_options): Likewise.
(gfc_handle_fpe_option): Likewise.
(gfc_get_option_string): Likewise.
* simplify.c (simplify_transformation_to_scalar): Likewise.
(gfc_simplify_spread): Likewise.
Index: gcc/fortran/trans-expr.c
===
--- gcc/fortran/trans-expr.c (revision 215323)
+++ gcc/fortran/trans-expr.c (working copy)
@@ -231,7 +231,7 @@ gfc_reset_vptr (stmtblock_t *block, gfc_
gfc_ref *ref;
/* If we have a class array, we need go back to the class
- container. */
+ container. */
if (lhs->ref && lhs->ref->next && !lhs->ref->next->next
&& lhs->ref->next->type == REF_ARRAY
&& lhs->ref->next->u.ar.type == AR_FULL
@@ -729,7 +729,7 @@ gfc_conv_class_to_class (gfc_se *parmse,
ctree = gfc_class_vptr_get (var);
/* The vptr is the second field of the actual argument.
- First we have to find the corresponding class reference. */
+ First we have to find the corresponding class reference. */
tmp = NULL_TREE;
if (class_ref == NULL
@@ -4953,7 +4953,7 @@ gfc_conv_procedure_call (gfc_se * se, gf
&& CLASS_DATA (fsym)->attr.codimension
&& !CLASS_DATA (fsym)->attr.allocatable)))
{
- /* Token and offset. */
+ /* Token and offset. */
vec_safe_push (stringargs, null_pointer_node);
vec_safe_push (stringargs, build_int_cst (gfc_array_index_type, 0));
gcc_assert (fsym->attr.optional);
@@ -7391,7 +7391,7 @@ arrayfunc_assign_needs_temporary (gfc_ex
{
/* A temporary is not needed if the function is not contained and
the variable is local or host associated and not a pointer or
- a target. */
+ a target. */
if (!expr2->value.function.esym->attr.contained)
return false;
@@ -7420,7 +7420,7 @@ realloc_lhs_loop_for_fcn_call (gfc_se *s
gfc_loopinfo *loop)
{
/* Signal that the function call should not be made by
- gfc_conv_loop_setup. */
+ gfc_conv_loop_setup. */
se->ss->is_alloc_lhs = 1;
gfc_init_loopinfo (loop);
gfc_add_ss_to_loop (loop, *ss);
@@ -8252,7 +8252,7 @@ gfc_trans_assignment_1 (gfc_expr * expr1
the function call must happen before the (re)allocation of the lhs -
otherwise the character length of the result is not known.
NOTE: This relies on having the exact dependence of the lengt
RE: [PATCH] Fix PR63152
>> The following fixes PR63152 zeroing the data field only for allocatables,
>> not pointers. The benefit of the patch is a >small speedup, and it avoids
>> that code starts to rely on behavior that is undefined in the standard. With
>> this patch, >something like
>>
>> INTEGER, DIMENSION(:), POINTER :: foo
>> IF (ASSOCIATED(foo)) ...
>>
>> will be detected by valgrind as undefined behavior.
>
>The code you touch is exercised in four different cases, as far as I can see
>from the assert earlier in the function:
>
> gcc_assert (sym->attr.pointer || sym->attr.allocatable || sym_has_alloc_comp
> || has_finalizer);
>
>So do we want to test (sym->attr.allocatable), or (!sym->attr.pointer)? Or,
>asked another way: should we NULLIFY in the case of sym_has_alloc_comp ||
>has_finalizer?
Hi FX,
thanks for your good question. I think it is equivalent, as it seems that
GFC_DESCRIPTOR_TYPE_P (type) implies either sym->attr.allocatable or
sym->attr.pointer. To check, I rank a check-fortran with the explicit patch
below, and this made no difference. Code gen for a number of additional
testcases involving alloc_comp and finalizers looked good as well. So, I think
the original patch is still fine.
Joost
Index: trans-array.c
===
--- trans-array.c (revision 215373)
+++ trans-array.c (working copy)
@@ -8647,9 +8647,18 @@ gfc_trans_deferred_array (gfc_symbol * s
type = TREE_TYPE (descriptor);
}
- /* NULLIFY the data pointer. */
+ /* NULLIFY the data pointer, for non-saved allocatables. */
if (GFC_DESCRIPTOR_TYPE_P (type) && !sym->attr.save)
-gfc_conv_descriptor_data_set (&init, descriptor, null_pointer_node);
+{
+ if (sym->attr.allocatable)
+{
+ gfc_conv_descriptor_data_set (&init, descriptor, null_pointer_node);
+}
+ else
+{
+ if (!sym->attr.pointer) gcc_unreachable ();
+}
+}
gfc_restore_backend_locus (&loc);
gfc_init_block (&cleanup);
[PATCH] Fix PR63152
The following fixes PR63152 zeroing the data field only for allocatables, not
pointers. The benefit of the patch is a small speedup, and it avoids that code
starts to rely on behavior that is undefined in the standard. With this patch,
something like
INTEGER, DIMENSION(:), POINTER :: foo
IF (ASSOCIATED(foo)) ...
will be detected by valgrind as undefined behavior.
tested on x86_64-unknown-linux-gnu.
OK for trunk ?
JoostIndex: gcc/testsuite/gfortran.dg/auto_char_dummy_array_1.f90
===
--- gcc/testsuite/gfortran.dg/auto_char_dummy_array_1.f90 (revision 215321)
+++ gcc/testsuite/gfortran.dg/auto_char_dummy_array_1.f90 (working copy)
@@ -11,6 +11,8 @@ end module global
program oh_no_not_pr15908_again
character(12), dimension(:), pointer :: ptr
+ nullify(ptr)
+
call a (ptr, 12)
if (.not.associated (ptr) ) call abort ()
if (any (ptr.ne."abc")) call abort ()
Index: gcc/testsuite/gfortran.dg/pr63152.f90
===
--- gcc/testsuite/gfortran.dg/pr63152.f90 (revision 0)
+++ gcc/testsuite/gfortran.dg/pr63152.f90 (revision 0)
@@ -0,0 +1,17 @@
+! { dg-do compile }
+! { dg-options "-fdump-tree-original" }
+!
+! PR 63152 : needless init of local pointer arrays
+!
+! Contributed by Joost VandeVondele
+ SUBROUTINE S1()
+ INTEGER, POINTER, DIMENSION(:) :: v
+ INTERFACE
+SUBROUTINE foo(v)
+ INTEGER, POINTER, DIMENSION(:) :: v
+END SUBROUTINE
+ END INTERFACE
+ CALL foo(v)
+ END SUBROUTINE S1
+! { dg-final { scan-tree-dump-times "= 0B" 0 "original" } }
+! { dg-final { cleanup-tree-dump "original" } }
Index: gcc/fortran/trans-array.c
===
--- gcc/fortran/trans-array.c (revision 215321)
+++ gcc/fortran/trans-array.c (working copy)
@@ -8647,8 +8647,8 @@ gfc_trans_deferred_array (gfc_symbol * s
type = TREE_TYPE (descriptor);
}
- /* NULLIFY the data pointer. */
- if (GFC_DESCRIPTOR_TYPE_P (type) && !sym->attr.save)
+ /* NULLIFY the data pointer, for non-saved allocatables. */
+ if (GFC_DESCRIPTOR_TYPE_P (type) && !sym->attr.save && sym->attr.allocatable)
gfc_conv_descriptor_data_set (&init, descriptor, null_pointer_node);
gfc_restore_backend_locus (&loc);
gcc/fortran/ChangeLog:
2014-09-17 Joost VandeVondele
PR fortran/63152
* trans-array.c (gfc_trans_deferred_array): Only nullify allocatables.
gcc/testsuite/ChangeLog:
2014-09-17 Joost VandeVondele
PR fortran/63152
* gfortran.dg/auto_char_dummy_array_1.f90: Fix undefined behavior.
* gfortran.dg/pr63152.f90: New test.
RE: [PATCH] gcc parallel make check
>> > These numbers are useful to try and ensure the overhead (scaling factor) >> > is reasonable, thanks. >> >> A nice improvement indeed. The patched result is 15 times faster >> than the serial unpatched run. So there is room for improvement > > Note, the box used was oldish AMD 16-core, no ht, box, haven't tried it on > anything on a 32 core box, no ht, I see these timings: time make -j32 -k check >& log.check32 ; time make -j8 -k check >& log.check8 real18m14.562s user260m21.578s sys 264m26.042s real41m33.210s user233m4.563s sys 72m11.429s so it is not quite reaching the ideal 4x speedup. Counting the number of 'expect' processes they are nicely at around 32 and 8 for the full test, with only a very short tail near the end. So, there might be some overhead somewhere. Total user time is similar, but time in sys goes up.
RE: [PATCH] Avoid inter-test dependencies in gfortran.dg (PR fortran/56408)
> Does the following patch fix this? Works for me with dejagnu 1.5.1. and works for me with 1.4.4 Joost
RE: [PATCH] Avoid inter-test dependencies in gfortran.dg (PR fortran/56408)
> > Framework version is 1.4.4 > You need at least dejagnu 1.5, which includes this fix: I see, but that's contrary to : https://gcc.gnu.org/install/prerequisites.html
RE: [PATCH] Avoid inter-test dependencies in gfortran.dg (PR fortran/56408)
>> What dejagnu version are you using? > runtest --version WARNING: Couldn't find the global config file. Expect version is 5.44.1.15 Tcl version is 8.5 Framework version is1.4.4
RE: [PATCH] Avoid inter-test dependencies in gfortran.dg (PR fortran/56408)
Hi Jakub,
thanks!
> +dg-test $gfortran_test_path/[lindex $args 1] ""
> $gfortran_aux_module_flags
> +# cleanup-modules isn't intentionally invoked here.
should this be 'is intentionally not invoked here' ?
I'm currently seeing a lot of errors in the log of make -j32 -k check. Similar
to the ones below.
Joost
ERROR: tcl error sourcing
/data/vjoost/gnu/gcc_trunk/gcc/gcc/testsuite/gfortran.dg/dg.exp.
ERROR: can't rename "dg-save-unknown": command doesn't exist
while executing
"rename dg-save-unknown unknown"
(procedure "saved-dg-test" line 96)
invoked from within
"saved-dg-test
/data/vjoost/gnu/gcc_trunk/gcc/gcc/testsuite/gfortran.dg/test_common_binding_labels_2_main.f03
{ -O } { -pedantic-errors}"
("eval" body line 1)
invoked from within
"eval saved-dg-test $args "
(procedure "dg-test" line 11)
invoked from within
"dg-test $test "$flags $flags_t" ${default-extra-flags}"
(procedure "gfortran-dg-runtest" line 28)
invoked from within
"gfortran-dg-runtest [lsort \
[glob -nocomplain $srcdir/$subdir/*.\[fF\]{,90,95,03,08} ] ] ""
$DEFAULT_FFLAGS"
(file "/data/vjoost/gnu/gcc_trunk/gcc/gcc/testsuite/gfortran.dg/dg.exp"
line 47)
invoked from within
"source /data/vjoost/gnu/gcc_trunk/gcc/gcc/testsuite/gfortran.dg/dg.exp"
("uplevel" body line 1)
invoked from within
"uplevel #0 source
/data/vjoost/gnu/gcc_trunk/gcc/gcc/testsuite/gfortran.dg/dg.exp"
invoked from within
"catch "uplevel #0 source $test_file_name""
Running /data/vjoost/gnu/gcc_trunk/gcc/gcc/testsuite/gfortran.dg/gomp/gomp.exp
...
ERROR: tcl error sourcing
/data/vjoost/gnu/gcc_trunk/gcc/gcc/testsuite/gfortran.dg/gomp/gomp.exp.
ERROR: torture-init: torture_without_loops is not empty as expected
while executing
"error "torture-init: torture_without_loops is not empty as expected""
invoked from within
"if [info exists torture_without_loops] {
error "torture-init: torture_without_loops is not empty as expected"
}"
(procedure "torture-init" line 4)
invoked from within
"torture-init"
(procedure "gfortran-dg-runtest" line 5)
invoked from within
"gfortran-dg-runtest [lsort \
[find $srcdir/$subdir *.\[fF\]{,90,95,03,08} ] ] "" "-fopenmp""
(file
"/data/vjoost/gnu/gcc_trunk/gcc/gcc/testsuite/gfortran.dg/gomp/gomp.exp" line
32)
invoked from within
"source /data/vjoost/gnu/gcc_trunk/gcc/gcc/testsuite/gfortran.dg/gomp/gomp.exp"
("uplevel" body line 1)
invoked from within
"uplevel #0 source
/data/vjoost/gnu/gcc_trunk/gcc/gcc/testsuite/gfortran.dg/gomp/gomp.exp"
invoked from within
"catch "uplevel #0 source $test_file_name""
Running
/data/vjoost/gnu/gcc_trunk/gcc/gcc/testsuite/gfortran.dg/graphite/graphite.exp
...
ERROR: tcl error sourcing
/data/vjoost/gnu/gcc_trunk/gcc/gcc/testsuite/gfortran.dg/graphite/graphite.exp.
RE: [PATCH] gcc parallel make check
> So, I’d love to see the numbers for 5 and 20 to double check that 10 is the > right number to pick. This sort of refinement is trivial post checkin. So, some timings with the patch, I think this is great. Doing the testing you suggest, changing the variable doesn't influence things much (at least for Fortran, and on this system). make -j32 -k check-fortran real3m27.875s -> gcc_runtest_parallelize_counter_minor == 02 (several testsuite errors: binding_label_tests_10_main.f03, binding_label_tests_11_main.f03, class_45b.f03, class_4b.f03, class_4c.f03, coarray_29_2.f90, test_common_binding_labels_3_main.f03) real3m26.234s -> gcc_runtest_parallelize_counter_minor == 05 (one additional testsuite error: whole_file_31.f90) real3m36.405s -> gcc_runtest_parallelize_counter_minor == 10 real3m38.736s -> gcc_runtest_parallelize_counter_minor == 20 check-c real8m26.935s check-c++ real7m4.165s check real 17m45.185s
RE: [PATCH] gcc parallel make check
>> Regtested on x86_64-linux, ok for trunk? > >Oh, forgot to say, PR56408 isn't fixed by this patch, but given the >higher granularity (10 tests instead of 1) we don't happen to trigger it >right now. which means that any commit to that dir could trigger it, right ?
RE: [PATCH] gcc parallel make check
> a newer patch (v8) I'll send soon attached with updated changelog. Compared to the previously posted v6, only the libstdc++-v3/testsuite/Makefile.am has been refined to split a little more the e*/* pattern, and two quickly running goal have been merged, in addition to fixing the pre-exisiting error in some of the patterns in that file. Checked comparing testsuite results before after. Obviously, if Jakub's patch can be made to work around the testsuite special cases, I believe it should be superior. If not, the attached patch is working as far as I can tell, and provides a significant improvement over current trunk. Joostcontrib/ChangeLog 2014-09-12 Joost VandeVondele * generate_tcl_patterns.sh: New file. gcc/fortran/ChangeLog 2014-09-12 Joost VandeVondele * Make-lang.in (check_gfortran_parallelize): Improved parallelism. gcc/Changelog 2014-09-12 Joost VandeVondele * Makefile.in (check_gcc_parallelize): Improved parallelism. (check_p_numbers): Increase maximum value. (dg_target_exps): Mention targets as separate words only. (null,space,comma,dg_target_exps_p1,dg_target_exps_p2, dg_target_exps_p3,dg_target_exps_p4): New variables. gcc/cp/ChangeLog 2014-09-12 Joost VandeVondele * Make-lang.in (check_g++_parallelize): Improved parallelism. libstdc++-v3/ChangeLog 2014-09-12 Joost VandeVondele * testsuite/Makefile.am (check_DEJAGNU_normal_targets): Add check-DEJAGNUnormal[11-15]. (check-DEJAGNU): Split into 15 jobs for parallel testing, correct pattern. * testsuite/Makefile.in: Regenerated. Index: libstdc++-v3/testsuite/Makefile.in === --- libstdc++-v3/testsuite/Makefile.in (revision 215147) +++ libstdc++-v3/testsuite/Makefile.in (working copy) @@ -301,7 +301,7 @@ lists_of_files = \ extract_symvers = $(glibcxx_builddir)/scripts/extract_symvers baseline_subdir := $(shell $(CXX) $(baseline_subdir_switch)) -check_DEJAGNU_normal_targets = $(patsubst %,check-DEJAGNUnormal%,0 1 2 3 4 5 6 7 8 9 10) +check_DEJAGNU_normal_targets = $(patsubst %,check-DEJAGNUnormal%,0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15) # Runs the testsuite, but in compile only mode. # Can be used to test sources with non-GNU FE's at various warning @@ -562,7 +562,7 @@ check-DEJAGNU $(check_DEJAGNU_normal_tar if [ -z "$*$(filter-out --target_board=%, $(RUNTESTFLAGS))" ] \ && [ "$(filter -j, $(MFLAGS))" = "-j" ]; then \ $(MAKE) $(AM_MAKEFLAGS) $(check_DEJAGNU_normal_targets); \ - for idx in 0 1 2 3 4 5 6 7 8 9 10; do \ + for idx in 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15; do \ mv -f normal$$idx/libstdc++.sum normal$$idx/libstdc++.sum.sep; \ mv -f normal$$idx/libstdc++.log normal$$idx/libstdc++.log.sep; \ done; \ @@ -589,25 +589,35 @@ check-DEJAGNU $(check_DEJAGNU_normal_tar fi; \ dirs="`cd $$srcdir; echo [013-9][0-9]_*/*`";; \ normal1) \ - dirs="`cd $$srcdir; echo [ab]* de* [ep]*/*`";; \ + dirs="`cd $$srcdir; echo experimental/* ext/[a-m]*`";; \ normal2) \ - dirs="`cd $$srcdir; echo 2[01]_*/*`";; \ + dirs="`cd $$srcdir; echo 28_*/a*`";; \ normal3) \ - dirs="`cd $$srcdir; echo 22_*/*`";; \ + dirs="`cd $$srcdir; echo 23_*/[lu]*`";; \ normal4) \ - dirs="`cd $$srcdir; echo 23_*/[a-km-tw-z]*`";; \ + dirs="`cd $$srcdir; echo 2[459]_*/*`";; \ normal5) \ - dirs="`cd $$srcdir; echo 23_*/[luv]*`";; \ + dirs="`cd $$srcdir; echo 2[01]_*/*`";; \ normal6) \ - dirs="`cd $$srcdir; echo 2[459]_*/*`";; \ + dirs="`cd $$srcdir; echo 23_*/[m-tw-z]*`";; \ normal7) \ - dirs="`cd $$srcdir; echo 26_*/* 28_*/[c-z]*`";; \ + dirs="`cd $$srcdir; echo 26_*/*`";; \ normal8) \ dirs="`cd $$srcdir; echo 27_*/*`";; \ normal9) \ - dirs="`cd $$srcdir; echo 28_*/[ab]*`";; \ + dirs="`cd $$srcdir; echo 22_*/*`";; \ normal10) \ dirs="`cd $$srcdir; echo t*/*`";; \ + normal11) \ + dirs="`cd $$srcdir; echo 28_*/b*`";; \ + normal12) \ + dirs="`cd $$srcdir; echo 28_*/[c-z]*`";; \ + normal13) \ + dirs="`cd $$srcdir; echo ext/[n-z]*`";; \ + normal14) \ + dirs="`cd $$srcdir; echo de*/* p*/* [ab]*/* 23_*/v*`";; \ + normal15) \ + dirs="`cd $$srcdir; echo 23_*/[a-k]*`";; \ esac; \ if [ -n "$*" ]; then cd "$*"; fi; \ if $(SHELL) -c "$$runtest --version" > /dev/null 2>&1; then \ Index: libstdc++-v3/testsuite/Makefile.am === --- libstdc++-v3/testsuite/Makefile.am (revision 215147) +++ libstdc++-v3/testsuite/Makefile.am (working copy) @@ -101,7 +101,7 @@ new-abi-baseline: @test ! -f $*/site.exp || mv $*/site.exp $*/site.bak @mv $*/site.exp.tmp $*/site.exp -check_DEJAGNU_normal_targets = $(patsubst %,check-DEJAGNUnormal%,0 1 2 3 4 5 6 7 8 9 10) +check_DEJAGNU_normal_targets = $(patsubst %,check-DEJAGNUnormal%,0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1
RE: [PATCH] gcc parallel make check
>>> >For PR56408 we need some fix. >> BTW, is there anything special about Fortran ? There are at least 180 test >> files that contain 'dg-additional-sources' >some in a very non-local way: >The current scheme comes at its limits in that case. . See the files listed in >the PR for issues. So, what about a pragmatic solution, and move the tests that rely on being serialized to a subdirectory serialized/ where, like now, we rely on the implicit ordering we have now ? At least it makes this assumption somewhat explicit. Joost
RE: [PATCH] gcc parallel make check
> And these Fortran inter-test dependencies, which Tobias told me is
> PR56408.
> For PR56408 we need some fix.
BTW, is there anything special about Fortran ? There are at least 180 test
files that contain 'dg-additional-sources' some in a very non-local way:
./objc.dg/foreach-2.m: /* { dg-additional-sources
"../objc-obj-c++-shared/nsconstantstring-class-impl.m" } */
Joost
RE: [PATCH] gcc parallel make check
> Here is a patch I'm testing now:
Hi Jakub,
I also tested your patch to compare timings vs a newer patch (v8) I'll send soon
== patch v8 == make -j32 -k ==
check-fortran 4m58.178s
check-c++ ~10m
check-c ~10m
check 15m29.873s
== patch Jakub
check-c++ ~20m
check-fortran 3m31.237s
check-c 8m8
on the positive side, your patch provides a further speedup e.g. fortran and c
testing (where it splits things nicely). The libstdc++ bottleneck is not
solved, but I guess that is expected.
As you have presumably found as well, your patch introduces a number failures,
because some tests seem to have additional dependencies, either explicit or
implicit:
e.g. in gfortran.dg/binding_label_tests_10_main.f03
! { dg-do compile }
! This file must be compiled AFTER binding_label_tests_10.f03, which it
! should be because dejagnu will sort the files.
module binding_label_tests_10_main
in gfortran.dg/class_45b.f03
! { dg-do link }
! { dg-additional-sources class_45a.f03 }
This could clearly trigger as well in the current scheme of splitting, only we
have been lucky that dependencies seem to be 'well behaved' in having the same
initial letter in the filename.
Joost
RE: [PATCH] RE: gcc parallel make check
>> could it be that the pattern in normal1 should have been '[ab]*/ de*/ >> [ep]*/*' ? > >Yes, we are running these tests multiple times: > >PASS: 23_containers/map/modifiers/erase/abi_tag.cc (test for excess errors) >PASS: 23_containers/multimap/modifiers/erase/abi_tag.cc (test for excess >errors) >PASS: 23_containers/multiset/modifiers/erase/abi_tag.cc (test for excess >errors) >PASS: 23_containers/set/modifiers/erase/abi_tag.cc (test for excess errors) >PASS: 26_numerics/complex/abi_tag.cc (test for excess errors) > >I'll fix that. Actually, the proper pattern should presumably be '[ab]*/* de*/* [ep]*/*' even though it seems to make no difference in testing. I'll have this included in yet another version of the parallel make check patch (plus some further reschuffling as requested by Jakub), so I think there is no need for you to fix this now.
RE: [PATCH] RE: gcc parallel make check
> could it be that the pattern in normal1 should have been '[ab]*/ de*/ > [ep]*/*' ? I've checked that this fixes the bug in the current trunk split. I.e. files are stil tested, but now only once. Consider this change added to the previously submitted patch.
RE: [PATCH] RE: gcc parallel make check
Jakub, > First of all, the -j2 testing shows more tests tested in gcc and libstdc++: > >-# of expected passes 10133 >+# of expected passes 10152 > >+PASS: 23_containers/set/modifiers/erase/abi_tag.cc (test for excess errors) >[...] > >Not sure where the bug is, could be e.g. in i386.exp for gcc, but for >libstdc++ less likely to be there rather than in the split. I looked into this, and believe this problem is already in current trunk, and not due to my patch. I.e. unmodified trunk also has these tests executed several times: libstdc++-v3/testsuite/normal4/libstdc++.log.sep:PASS: 23_containers/map/modifiers/erase/abi_tag.cc libstdc++-v3/testsuite/normal1/libstdc++.log.sep:PASS: 23_containers/map/modifiers/erase/abi_tag.cc I believe the current trunk pattern could indeed match those twice (Makefile.in in trunk): normal1) \ dirs="`cd $$srcdir; echo [ab]* de* [ep]*/*`";; \ normal4) \ dirs="`cd $$srcdir; echo 23_*/[a-km-tw-z]*`";; \ could it be that the pattern in normal1 should have been '[ab]*/ de*/ [ep]*/*' ? Joost
RE: [PATCH] RE: gcc parallel make check
> You mean enhancing the script to split across arbitrarily long prefixes? > That would be great. I've now a script that does something like that: ~/test$ find /data/vjoost/gnu/gcc_trunk/gcc/gcc/testsuite/gfortran.dg/ -maxdepth 1 -type f -printf "%f\n" | ./generate_patterns.py 500 foo All 3947 files matched the pattern ^[0-9A-Za-z_#+-]*([.][0-9A-Za-z_#+-]+)+ without exception Final 12 patterns and match count: (^[j-z_#+-][p-z_#+-][0-9A-Za-i][0-9A-Za-z_#+-]*([.][0-9A-Za-z_#+-]+)+|^[j-z_#+-][0-9A-Za-o][0-9A-Za-m]([.][0-9A-Za-z_#+-]+)+) matching 469 files (^[0-9A-Za-i][0-9A-Za-n][0-9A-Za-n][0-9A-Za-o][0-9A-Za-z_#+-]*([.][0-9A-Za-z_#+-]+)+|^([.][0-9A-Za-z_#+-]+)+) matching 433 files (^[j-z_#+-][0-9A-Za-o][n-z_#+-][0-9A-Za-z_#+-]*([.][0-9A-Za-z_#+-]+)+|^[0-9A-Za-i][0-9A-Za-n][o-z_#+-]([.][0-9A-Za-z_#+-]+)+) matching 400 files (^[j-z_#+-][p-z_#+-][j-z_#+-][0-9A-Za-z_#+-]*([.][0-9A-Za-z_#+-]+)+|^[0-9A-Za-i]([.][0-9A-Za-z_#+-]+)+) matching 371 files (^[0-9A-Za-i][o-z_#+-][s-z_#+-][0-9A-Za-z_#+-]*([.][0-9A-Za-z_#+-]+)+|^[0-9A-Za-i][0-9A-Za-n][0-9A-Za-n]([.][0-9A-Za-z_#+-]+)+) matching 323 files (^[0-9A-Za-i][o-z_#+-][0-9A-Za-r][o-z_#+-][0-9A-Za-z_#+-]*([.][0-9A-Za-z_#+-]+)+|^[j-z_#+-][p-z_#+-]([.][0-9A-Za-z_#+-]+)+) matching 314 files (^[0-9A-Za-i][o-z_#+-][0-9A-Za-r][0-9A-Za-n][0-9A-Za-z_#+-]*([.][0-9A-Za-z_#+-]+)+|^[j-z_#+-][0-9A-Za-o]([.][0-9A-Za-z_#+-]+)+) matching 314 files (^[j-z_#+-][0-9A-Za-o][0-9A-Za-m][0-9A-Za-i][0-9A-Za-z_#+-]*([.][0-9A-Za-z_#+-]+)+|^[j-z_#+-]([.][0-9A-Za-z_#+-]+)+) matching 272 files (^[0-9A-Za-i][0-9A-Za-n][0-9A-Za-n][p-z_#+-][0-9A-Za-z_#+-]*([.][0-9A-Za-z_#+-]+)+|^[0-9A-Za-i][o-z_#+-]([.][0-9A-Za-z_#+-]+)+) matching 270 files (^[0-9A-Za-i][0-9A-Za-n][o-z_#+-][0-9A-Za-l][0-9A-Za-z_#+-]*([.][0-9A-Za-z_#+-]+)+|^[0-9A-Za-i][0-9A-Za-n]([.][0-9A-Za-z_#+-]+)+) matching 265 files (^[0-9A-Za-i][0-9A-Za-n][o-z_#+-][m-z_#+-][0-9A-Za-z_#+-]*([.][0-9A-Za-z_#+-]+)+|^[0-9A-Za-i][o-z_#+-][0-9A-Za-r]([.][0-9A-Za-z_#+-]+)+) matching 260 files ^[j-z_#+-][0-9A-Za-o][0-9A-Za-m][j-z_#+-][0-9A-Za-z_#+-]*([.][0-9A-Za-z_#+-]+)+ matching 256 files It is a set of patterns that will match any file of the form '^[0-9A-Za-z_#+-]*([.][0-9A-Za-z_#+-]+)+', but such that it splits a list of input files roughly in equal chunks (e.g. between 500 and 500/2 in this example), even if files have long overlapping prefixes. However, I'm unsure if/how this can be integrated, i.e. what precisely is allowed for testsuite filenames, and if this regexp format can be employed in gcc makefiles / tcl / expect harness, suggestions/help appreciated.
RE: [PATCH] RE: gcc parallel make check
Thanks for testing. The vect-args.c I explained earlier, and is indeed due to i386.exp hardcoding those. The libstdc++ double counts didn't appear in my testing, but I'll have a look. Note that these patterns are handwritten, so error prone. The long tests in libstdc++ come from (in timing order, from my machine): normal1) \ dirs="`cd $$srcdir; echo e*/*`";; \ normal2) \ dirs="`cd $$srcdir; echo 28_*/a*`";; \ normal3) \ dirs="`cd $$srcdir; echo 23_*/[lu]*`";; \ normal4) \ dirs="`cd $$srcdir; echo 2[459]_*/*`";; \
RE: [PATCH] RE: gcc parallel make check
Attached is a further revision of the patch, now dealing with check-c++. Roughly 50% speedup here at '-j32' (18m vs 12m). For my setup (--enable-languages=c,c++,fortran) I have now improved all targets called in 'make -j32 -k check'. The latter is now 30% faster (15m vs 20m). Note that there are +- 1m fluctuations in these numbers, easily. I currently have no plans to work on other check targets before this patch is committed. OK for trunk ? Joost contrib/ChangeLog 2014-09-09 Joost VandeVondele * generate_tcl_patterns.sh: New file. gcc/fortran/ChangeLog 2014-09-09 Joost VandeVondele * Make-lang.in (check_gfortran_parallelize): Improved parallelism. gcc/Changelog 2014-09-09 Joost VandeVondele * Makefile.in (check_gcc_parallelize): Improved parallelism. (check_p_numbers): Increase maximum value. (dg_target_exps): Mention targets as separate words only. (null,space,comma,dg_target_exps_p1,dg_target_exps_p2, dg_target_exps_p3,dg_target_exps_p4): New variables. gcc/cp/ChangeLog 2014-09-09 Joost VandeVondele * Make-lang.in (check_g++_parallelize): Improved parallelism. libstdc++-v3/ChangeLog 2014-09-09 Joost VandeVondele * testsuite/Makefile.am (check_DEJAGNU_normal_targets): Add check-DEJAGNUnormal[11-15]. (check-DEJAGNU): Split into 15 jobs for parallel testing. * testsuite/Makefile.in: Regenerated. Index: libstdc++-v3/testsuite/Makefile.am === --- libstdc++-v3/testsuite/Makefile.am (revision 215017) +++ libstdc++-v3/testsuite/Makefile.am (working copy) @@ -101,7 +101,7 @@ new-abi-baseline: @test ! -f $*/site.exp || mv $*/site.exp $*/site.bak @mv $*/site.exp.tmp $*/site.exp -check_DEJAGNU_normal_targets = $(patsubst %,check-DEJAGNUnormal%,0 1 2 3 4 5 6 7 8 9 10) +check_DEJAGNU_normal_targets = $(patsubst %,check-DEJAGNUnormal%,0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15) $(check_DEJAGNU_normal_targets): check-DEJAGNUnormal%: normal%/site.exp # Run the testsuite in normal mode. @@ -111,7 +111,7 @@ check-DEJAGNU $(check_DEJAGNU_normal_tar if [ -z "$*$(filter-out --target_board=%, $(RUNTESTFLAGS))" ] \ && [ "$(filter -j, $(MFLAGS))" = "-j" ]; then \ $(MAKE) $(AM_MAKEFLAGS) $(check_DEJAGNU_normal_targets); \ - for idx in 0 1 2 3 4 5 6 7 8 9 10; do \ + for idx in 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15; do \ mv -f normal$$idx/libstdc++.sum normal$$idx/libstdc++.sum.sep; \ mv -f normal$$idx/libstdc++.log normal$$idx/libstdc++.log.sep; \ done; \ @@ -138,25 +138,35 @@ check-DEJAGNU $(check_DEJAGNU_normal_tar fi; \ dirs="`cd $$srcdir; echo [013-9][0-9]_*/*`";; \ normal1) \ - dirs="`cd $$srcdir; echo [ab]* de* [ep]*/*`";; \ + dirs="`cd $$srcdir; echo e*/*`";; \ normal2) \ - dirs="`cd $$srcdir; echo 2[01]_*/*`";; \ + dirs="`cd $$srcdir; echo 28_*/a*`";; \ normal3) \ - dirs="`cd $$srcdir; echo 22_*/*`";; \ + dirs="`cd $$srcdir; echo 23_*/[lu]*`";; \ normal4) \ - dirs="`cd $$srcdir; echo 23_*/[a-km-tw-z]*`";; \ + dirs="`cd $$srcdir; echo 2[459]_*/*`";; \ normal5) \ - dirs="`cd $$srcdir; echo 23_*/[luv]*`";; \ + dirs="`cd $$srcdir; echo 2[01]_*/*`";; \ normal6) \ - dirs="`cd $$srcdir; echo 2[459]_*/*`";; \ + dirs="`cd $$srcdir; echo 23_*/[m-tw-z]*`";; \ normal7) \ - dirs="`cd $$srcdir; echo 26_*/* 28_*/[c-z]*`";; \ + dirs="`cd $$srcdir; echo 26_*/*`";; \ normal8) \ dirs="`cd $$srcdir; echo 27_*/*`";; \ normal9) \ - dirs="`cd $$srcdir; echo 28_*/[ab]*`";; \ + dirs="`cd $$srcdir; echo 22_*/*`";; \ normal10) \ dirs="`cd $$srcdir; echo t*/*`";; \ + normal11) \ + dirs="`cd $$srcdir; echo 28_*/b*`";; \ + normal12) \ + dirs="`cd $$srcdir; echo 28_*/[c-z]*`";; \ + normal13) \ + dirs="`cd $$srcdir; echo de* p*/*`";; \ + normal14) \ + dirs="`cd $$srcdir; echo [ab]* 23_*/v*`";; \ + normal15) \ + dirs="`cd $$srcdir; echo 23_*/[a-k]*`";; \ esac; \ if [ -n "$*" ]; then cd "$*"; fi; \ if $(SHELL) -c "$$runtest --version" > /dev/null 2>&1; then \ Index: libstdc++-v3/testsuite/Makefile.in === --- libstdc++-v3/testsuite/Makefile.in (revision 215017) +++ libstdc++-v3/testsuite/Makefile.in (working copy) @@ -301,7 +301,7 @@ lists_of_files = \ extract_symvers = $(glibcxx_builddir)/scripts/extract_symvers baseline_subdir := $(shell $(CXX) $(baseline_subdir_switch)) -check_DEJAGNU_normal_targets = $(patsubst %,check-DEJAGNUnormal%,0 1 2 3 4 5 6 7 8 9 10) +check_DEJAGNU_normal_targets = $(patsubst %,check-DEJAGNUnormal%,0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15) # Runs the testsuite, but in compile only mode. # Can be used to test sources with non-GNU FE's at various warning @@ -562,7 +562,7 @@ check-DEJAGNU $(check_DEJAGNU_normal_tar if [ -z "$*$(filter-out --target_board=%, $(RUNTES
RE: [PATCH] RE: gcc parallel make check
Now with gzipped figure.. why do these bounce ? > But if there are jobs that just take 1s to complete, then clearly it doesn't > make sense to split them off as separate job. I think we don't need 100% > even split, but at least roughly is highly desirable. Let me add some data, attached is a graph (logscale y) showing the runtime of tests before and after my changes (including a new patch for c++). There is virtually no change for tests running shorter than 50s, only slowly running tests have been split. Now, there are only very few slow tests remaining: gcc_trunk/obj.new> find . -name "*.log" | xargs grep " completed in " | sort -n -k 5 | tail -n 10 ./gcc/testsuite/gcc/gcc.log:testcase /data/vjoost/gnu/gcc_trunk/gcc/gcc/testsuite/gcc.dg/torture/dg-torture.exp completed in 521 seconds ./x86_64-unknown-linux-gnu/libstdc++-v3/testsuite/libstdc++.log:testcase /data/vjoost/gnu/gcc_trunk/gcc/libstdc++-v3/testsuite/libstdc++-dg/conformance.exp completed in 530 seconds ./x86_64-unknown-linux-gnu/libstdc++-v3/testsuite/libstdc++.log:testcase /data/vjoost/gnu/gcc_trunk/gcc/libstdc++-v3/testsuite/libstdc++-dg/conformance.exp completed in 553 seconds ./x86_64-unknown-linux-gnu/libgomp/testsuite/libgomp.log:testcase /data/vjoost/gnu/gcc_trunk/gcc/libgomp/testsuite/libgomp.fortran/fortran.exp completed in 561 seconds ./gcc/testsuite/gcc/gcc.log:testcase /data/vjoost/gnu/gcc_trunk/gcc/gcc/testsuite/gcc.c-torture/compile/compile.exp completed in 625 seconds ./x86_64-unknown-linux-gnu/libstdc++-v3/testsuite/libstdc++.log:testcase /data/vjoost/gnu/gcc_trunk/gcc/libstdc++-v3/testsuite/libstdc++-dg/conformance.exp completed in 683 seconds ./gcc/testsuite/g++/g++.log:testcase /data/vjoost/gnu/gcc_trunk/gcc/gcc/testsuite/g++.dg/dg.exp completed in 702 seconds ./x86_64-unknown-linux-gnu/libstdc++-v3/testsuite/libstdc++.log:testcase /data/vjoost/gnu/gcc_trunk/gcc/libstdc++-v3/testsuite/libstdc++-dg/conformance.exp completed in 726 seconds ./gcc/testsuite/gcc/gcc.log:testcase /data/vjoost/gnu/gcc_trunk/gcc/gcc/testsuite/gcc.c-torture/execute/execute.exp completed in 752 seconds ./x86_64-unknown-linux-gnu/libstdc++-v3/testsuite/libstdc++.log:testcase /data/vjoost/gnu/gcc_trunk/gcc/libstdc++-v3/testsuite/libstdc++-dg/conformance.exp completed in 904 seconds They, of course, limit the ultimate speedup. timings.png.gz Description: timings.png.gz
RE: [PATCH] RE: gcc parallel make check
> If you get whitespace right, one can provide multiple different wildcards to > a single *.exp file, e.g. > make check-gcc RUNTESTFLAGS="dg.exp='p[0-9A-Za-qs-z]* pr[9A-Za-z]*'" should > cover all tests starting with p other than pr[0-8]*.c (where you could split > say pr[0-2]* into another job, pr[3-5]* into another and pr[6-8]* into > another. I think this confirms that it becomes very delicate to try and write these more complex patterns. The above would miss p_test.c, p-1.c, etc ? For other classes of files the difference is even further down the filename (e.g. using dates as in 20020508-3.c going from 2000 to 2014, or avx*), making the automatic generation of the patterns more complicated. I certainly don't want to claim that the patch I have now is perfect, it is rather an incremental improvement on the current setup.
RE: [PATCH] RE: gcc parallel make check
> No. As I wrote earlier, splitting on filenames and test counts only is only > very rough split, all the splits really need to be backed out by real timing > data from popular targets. I'm actually doing quite some testing trying to get a reasonable balance, checking 'completed in' in all *.log.sep files. However, it is important that the procedure is semi-automatic, otherwise few people will be interested in doing so. Furthermore, for parallel performance, it is not so important that times are distributed evenly (it is anyway unlikely the number of goals is exactly divided by N of -jN), but rather that the goals are ordered (executed) from slow to fast (similar to omp schedule guided). Most of the real bottlenecks are single letter patterns (e.g. p* since pr is such a common filename), and this is ultimately limiting. In the project (CP2K) I'm working on, we also parallelize testing over directories, but we keep a list of approximate runtimes per directory, and keep that (global) list sorted. Testing follows that list. As a result, we have near perfect parallel speedup, despite (or because) timings per directory ranging from a few 100s to 1s. > Also, I'm afraid of some tests being left out > unintentionally (e.g. the wildcards created at some point, then a new test > is added with a weird starting character that hasn't been used before and > suddenly it will not be tested with make -j?). I agree this is an issue, partially addressed by not having to write patterns by hand anymore (i.e. a script does this), and by having the script check its input. There are something like 10 testnames that do not fall in [0-9A-Za-z], as mentioned in a previous email.
RE: [PATCH] RE: gcc parallel make check
> +# ls -1 | ../../../contrib/generate_tcl_patterns.sh 300
> "dg.exp=gfortran.dg/"
>
> How does this work with subdirectories? Can we replace ls with find?
The input to the script is general, you can use this to your advantage. For
example, I've been using:
ls -1 g++.*/* | cut -c5- | ../../../contrib/generate_tcl_patterns.sh 700
old-deja.exp=g++.old-deja/g++.
to split at a deeper level or
find . -name "[0-9A-Za-z]*" -type f -printf "%f\n" |
../../../../contrib/generate_tcl_patterns.sh 300 dg-torture.exp=torture/
to collect statistics also from subdirs.
> + if (_assert_exit) exit 1
>
> Haven't you already exited above?
yes, but the END{} block in awk is nevertheless executed, unless protected as
above.
RE: [PATCH] RE: gcc parallel make check
Attached is an extended version of the patch, it brings a 100% improvement in
make -j32 -k check-gcc (down from 20min to <10min) by modification of
check_gcc_parallelize.
It includes one non-trivial part, namely a split of the target exps. They are
now all split using a common choice (based on i386), which I believe is
reasonable as it is the target with most tests, and the patterns will be
somewhat similar for other targets (e.g. split of p(rxxx)). The implementation
of this in the makefile uses an odd looking technique to substitute spaces with
commas in a variable, if this can be done more elegantly, I'm happy to make the
change.
Bootstrap and testing revealed one issue, i386.exp hard-codes a loop for the
testcase 'vect-args.c' in order to test 10 different combinations of options.
With the current split (i.e. target x4) this test will thus be executed 4
times. There are two easy options
1) keep the current setup, overhead is small
2) keep the .exp file simple and just replicate this test 10x
I've selected 1), but I can update a patch with 2). Ideally dg-options in the
testcase file itself could be repeated, but I haven't found an example of this.
The script now includes sorting and compression of the ranges, and an
additional sanity check on the input, i.e. that file names start with
[0-9A-Za-z]. Some (few) files seem to start with _ or # (in ./gcc.dg/cpp/).
I'll follow up with a separate patch to improve check_g++_parallelize.
Full 'make -j k32 check' is now dominated by libstdc++ testing, which contains
single goals that run ~1100s (e.g. regex related tests). These uses a slightly
different syntax (see gcc/libstdc++-v3/testsuite/Makefile.am) and I'm not yet
sure how to deal with the .am files.
current patch OK for trunk ?
Joost
patch-speedup-checkfortran-v05.CL
Description: patch-speedup-checkfortran-v05.CL
Index: contrib/generate_tcl_patterns.sh
===
--- contrib/generate_tcl_patterns.sh (revision 0)
+++ contrib/generate_tcl_patterns.sh (revision 0)
@@ -0,0 +1,114 @@
+#! /bin/sh
+
+#
+# based on a list of filenames as input, starting with [0-9A-Za-z],
+# generate regexps that match subsets trying to not exceed a
+# 'maxcount' parameter. Most useful to generate the
+# check_LANG_parallelize assignments needed to split
+# testsuite directories, defining prefix appropriately.
+#
+# Example usage:
+# cd gcc/gcc/testsuite/gfortran.dg
+# ls -1 | ../../../contrib/generate_tcl_patterns.sh 300 "dg.exp=gfortran.dg/"
+#
+# the first parameter is the maximum number of files.
+# the second parameter the prefix used for printing.
+#
+
+# Copyright (C) 2014 Free Software Foundation
+# Contributed by Joost VandeVondele
+#
+# This file is part of GCC.
+#
+# GCC is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3, or (at your option)
+# any later version.
+#
+# GCC is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with GCC; see the file COPYING. If not, write to
+# the Free Software Foundation, 51 Franklin Street, Fifth Floor,
+# Boston, MA 02110-1301, USA.
+
+gawk -v maxcount=$1 -v prefix=$2 '
+BEGIN{
+ # list of allowed starting chars for a file name in a dir to split
+ achars="0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
+ ranget="112233"
+}
+{
+ if (index(achars,substr($1,1,1))==0){
+ print "file : " $1 " does not start with an allowed character."
+ _assert_exit = 1
+ exit 1
+ }
+ nfiles++ ; files[nfiles]=$1
+}
+END{
+ if (_assert_exit) exit 1
+ for(i=1; i<=length(achars); i++) count[substr(achars,i,1)]=0
+ for(i=1; i<=nfiles; i++) {
+ if (length(files[i]>0)) { count[substr(files[i],1,1)]++ }
+ };
+ asort(count,ordered)
+ countsingle=0
+ groups=0
+ label=""
+ for(i=length(achars);i>=1;i--) {
+countsingle=countsingle+ordered[i]
+for(j=1;j<=length(achars);j++) {
+ if(count[substr(achars,j,1)]==ordered[i]) found=substr(achars,j,1)
+}
+count[found]=-1
+label=label found
+if(i==1) { val=maxcount+1 } else { val=ordered[i-1] }
+if(countsingle+val>maxcount) {
+ subset[label]=countsingle
+ print "Adding label: ", label, "matching files:" countsingle
+ groups++
+ countsingle=0
+ label=""
+}
+ }
+ print "patterns:"
+ asort(subset,ordered)
+ for(i=groups;i>=1;i--) {
+for(j in subset){
+ if(subset[j]==ordered[i]) found=j
+}
+subset[found]=-1
+if (length(found)==1) {
+ printf("%s%s* \\\n",prefix,found)
+} else {
+ sortandcompress()
+ pri
RE: [PATCH] RE: gcc parallel make check
>> > Please sort the letters (LC_ALL=C sort) and where consecutive, use ranges.
>> > Thus \[0-9A-Zhjqvx-z\]*
OK, works fine with the attached patch, and looks cleaner in Make-lang.in.
Now, with the proper email address for gcc-patches... I wonder how many time
I'll be punished for typos.
unmodified CL.
Joost
Index: contrib/generate_tcl_patterns.sh
===
--- contrib/generate_tcl_patterns.sh (revision 0)
+++ contrib/generate_tcl_patterns.sh (revision 0)
@@ -0,0 +1,108 @@
+#! /bin/sh
+
+#
+# based on a list of filenames as input,
+# generate regexps that match subsets trying to not exceed a
+# 'maxcount' parameter. Most useful to generate the
+# check_LANG_parallelize assignments needed to split
+# testsuite directories, defining prefix appropriately.
+#
+# Example usage:
+# cd gcc/gcc/testsuite/gfortran.dg
+# ls -1 | ../../../contrib/generate_tcl_patterns.sh 300 "dg.exp=gfortran.dg/"
+#
+# the first parameter is the maximum number of files.
+# the second parameter the prefix used for printing.
+#
+
+# Copyright (C) 2014 Free Software Foundation
+# Contributed by Joost VandeVondele
+#
+# This file is part of GCC.
+#
+# GCC is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3, or (at your option)
+# any later version.
+#
+# GCC is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with GCC; see the file COPYING. If not, write to
+# the Free Software Foundation, 51 Franklin Street, Fifth Floor,
+# Boston, MA 02110-1301, USA.
+
+gawk -v maxcount=$1 -v prefix=$2 '
+BEGIN{
+ # list of allowed starting chars for a file name in a dir to split
+ achars="0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
+ ranget="112233"
+}
+{
+ nfiles++ ; files[nfiles]=$1
+}
+END{
+ for(i=1; i<=length(achars); i++) count[substr(achars,i,1)]=0
+ for(i=1; i<=nfiles; i++) {
+ if (length(files[i]>0)) { count[substr(files[i],1,1)]++ }
+ };
+ asort(count,ordered)
+ countsingle=0
+ groups=0
+ label=""
+ for(i=length(achars);i>=1;i--) {
+countsingle=countsingle+ordered[i]
+for(j=1;j<=length(achars);j++) {
+ if(count[substr(achars,j,1)]==ordered[i]) found=substr(achars,j,1)
+}
+count[found]=-1
+label=label found
+if(i==1) { val=maxcount+1 } else { val=ordered[i-1] }
+if(countsingle+val>maxcount) {
+ subset[label]=countsingle
+ print "Adding label: ", label, "matching files:" countsingle
+ groups++
+ countsingle=0
+ label=""
+}
+ }
+ print "patterns:"
+ asort(subset,ordered)
+ for(i=groups;i>=1;i--) {
+for(j in subset){
+ if(subset[j]==ordered[i]) found=j
+}
+subset[found]=-1
+if (length(found)==1) {
+ printf("%s%s* \\\n",prefix,found)
+} else {
+ sortandcompress()
+ printf("%s\\[%s\\]* \\\n",prefix,found)
+}
+ }
+}
+function sortandcompress(i,n,tmp,bestj)
+{
+ n=length(found)
+ for(i=1; i<=n; i++) tmp[i]=substr(found,i,1)
+ asort(tmp)
+ for(i=1;i<=n;i++){
+ipos=index(achars,tmp[i])
+for(j=i;j<=n;j++){
+ jpos=index(achars,tmp[j])
+ if (jpos-ipos==j-i && substr(ranget,ipos,1)==substr(ranget,jpos,1)) bestj=j
+}
+if (bestj-i>3) {
+ tmp[i+1]="-"
+ for(j=i+2;j
RE: [PATCH] PR fortran/62245 fix INT docs.
> OK. Committed revision 214958.
RE: [PATCH] PR fortran/62245 fix INT docs.
> >The last sentence above is not needed.
So, revised patch without the last sentence.
2014-09-05 Joost VandeVondele
PR fortran/62245
* intrinsic.texi (INT): clarify result.
Index: fortran/intrinsic.texi
===
--- fortran/intrinsic.texi (revision 214949)
+++ fortran/intrinsic.texi (working copy)
@@ -7370,9 +7370,10 @@ the following rules:
@item (A)
If @var{A} is of type @code{INTEGER}, @code{INT(A) = A}
@item (B)
-If @var{A} is of type @code{REAL} and @math{|A| < 1}, @code{INT(A)} equals
@code{0}.
-If @math{|A| \geq 1}, then @code{INT(A)} equals the largest integer that does
not exceed
-the range of @var{A} and whose sign is the same as the sign of @var{A}.
+If @var{A} is of type @code{REAL} and @math{|A| < 1}, @code{INT(A)}
+equals @code{0}. If @math{|A| \geq 1}, then @code{INT(A)} is the integer
+whose magnitude is the largest integer that does not exceed the magnitude
+of @var{A} and whose sign is the same as the sign of @var{A}.
@item (C)
If @var{A} is of type @code{COMPLEX}, rule B is applied to the real part of
@var{A}.
@end table
RE: [PATCH] PR fortran/62245 fix INT docs.
>> but helpful for gfortran users ? >The problem is that a similar sentence would then need to be >added to other intrinsic subroutines/functions where an actual >argument might produce a result that is out-of-range of the >representable entity. So, I could remove the sentence if there is consensus. >> maybe you could prepare a patch to add something like that to the >> section on intrinsics? > >Sure, I'll come up with a statement to be added to Sec. 8.1 of >the gfortran manual. actually, it is more complicated, since gfortran does support things such as NaN, and Inf, depending on the hardware and the compilation options.
RE: [PATCH] PR fortran/62245 fix INT docs.
>> +of @var{A} and whose sign is the same as the sign of @var{A}. The result
>> +is undefined if it can not be represented as an @code{INTEGER} of the
>> +given @code{KIND}.
>
>The last sentence above is not needed.
but helpful for gfortran users ?
> There is a general
> prohibition in 13.7.1 against calling INT(A) with |A| > HUGE(1_4).
maybe you could prepare a patch to add something like that to the section on
intrinsics ?
RE: [PATCH] PR fortran/62245 fix INT docs.
> One small ask: these lines are too long (already before your patch);
> can you please reformat those lines your patch touches to <80 columns?
sure:
Index: intrinsic.texi
===
--- intrinsic.texi (revision 214408)
+++ intrinsic.texi (working copy)
@@ -7370,9 +7370,12 @@ the following rules:
@item (A)
If @var{A} is of type @code{INTEGER}, @code{INT(A) = A}
@item (B)
-If @var{A} is of type @code{REAL} and @math{|A| < 1}, @code{INT(A)} equals
@code{0}.
-If @math{|A| \geq 1}, then @code{INT(A)} equals the largest integer that does
not exceed
-the range of @var{A} and whose sign is the same as the sign of @var{A}.
+If @var{A} is of type @code{REAL} and @math{|A| < 1}, @code{INT(A)}
+equals @code{0}. If @math{|A| \geq 1}, then @code{INT(A)} is the integer
+whose magnitude is the largest integer that does not exceed the magnitude
+of @var{A} and whose sign is the same as the sign of @var{A}. The result
+is undefined if it can not be represented as an @code{INTEGER} of the
+given @code{KIND}.
@item (C)
If @var{A} is of type @code{COMPLEX}, rule B is applied to the real part of
@var{A}.
@end table
[PATCH] PR fortran/62245 fix INT docs.
A doc change to refine wording for result value of int, avoiding the word range
and using magnitude as does the standard. Mentions undefined behavior.
2014-08-24 Joost VandeVondele
PR fortran/62245
* intrinsic.texi (INT): clarify result and undefined behavior.
Index: intrinsic.texi
===
--- intrinsic.texi (revision 214408)
+++ intrinsic.texi (working copy)
@@ -7371,8 +7371,10 @@ the following rules:
If @var{A} is of type @code{INTEGER}, @code{INT(A) = A}
@item (B)
If @var{A} is of type @code{REAL} and @math{|A| < 1}, @code{INT(A)} equals
@code{0}.
-If @math{|A| \geq 1}, then @code{INT(A)} equals the largest integer that does
not exceed
-the range of @var{A} and whose sign is the same as the sign of @var{A}.
+If @math{|A| \geq 1}, then @code{INT(A)} is the integer whose magnitude is the
largest
+integer that does not exceed the magnitude of @var{A} and whose sign is the
same as
+the sign of @var{A}. The result is undefined if it can not be represented as an
+@code{INTEGER} of the given @code{KIND}.
@item (C)
If @var{A} is of type @code{COMPLEX}, rule B is applied to the real part of
@var{A}.
@end table
RE: [PATCH, Fortran] PR61234: -Wuse-no-only
> OK with the documentation change and with the re-named option. Please > also update the name in the code. changes made and committed as r214311
RE: [PATCH, Fortan] fix initialization of flag_errno_math and flag_associative_math
Committed revision 214211.
[COMMITTED] Add myself to MAINTAINERS (Write After Approval)
Committed revision 214159. 2014-08-19 Joost VandeVondele * MAINTAINERS (Write After Approval): Add myself. Index: MAINTAINERS === --- MAINTAINERS (revision 214158) +++ MAINTAINERS (working copy) @@ -561,6 +561,7 @@ Markus Trippelsdorf markus@trippelsdo David Ung dav...@mips.com Neil Vachharajani nvach...@gmail.com Kris Van Hees kris.van.h...@oracle.com +Joost VandeVondele joost.vandevond...@mat.ethz.ch Ilya Verbiniver...@gmail.com Kugan Vivekanandarajah kug...@linaro.org Tom de Vries t...@codesourcery.com
RE: [PATCH, Fortran] PR61234: -Wuse-no-only
>> So the negative version is -Wno-use-no-only? That sounds weird. > What about -Wuse-without-only? Would be fine with me. Approved with this change ?
RE: [PATCH, Fortan] fix initialization of flag_errno_math and flag_associative_math
Thanks, can somebody with svn write access commit ?
RE: [PATCH, Fortran] PR61234: -Wuse-no-only
ping ? https://gcc.gnu.org/ml/fortran/2014-06/msg00114.html
RE: [PATCH, Fortan] fix initialization of flag_errno_math and flag_associative_math
ping ? https://gcc.gnu.org/ml/fortran/2014-05/msg00162.html
RE: [PATCH, Fortran] PR61234: -Wuse-no-only
>> This explicitly tests that no bogus error message is issued
>> for a use statement that has an only qualifier ?
>
>I don't see the need for '! { dg-bogus "has no ONLY qualifier" }'.
>AFAICT there is no warning emitted for this line (unless you add -Wall)
>and if some day it happens that an error/warning is issued, the test will fail.
>
>Otherwise the new patch is OK for me.
if so, could the patch be applied by somebody with svn write permission (with
or without the dg-bogus), fsf assignment is now OK.
Joost
RE: [PATCH, Fortan] fix initialization of flag_errno_math and flag_associative_math
Dominque, after Janne's OK, and FX judgement, I wonder if you have reached a conclusion. If so, the fsf assignment is now complete, and the patch could be applied. Joost > Ok for trunk. Thanks! Please don't rush! The behavior of -fno-signed-zeros -fno-trapping-math implying associative math has been changed (as in reverted) between r165758 (implied associative math) and r165930 (lost associative math). AFAICT it could be due to 165823. Investigating! I am also lookinf for the introduction of *frontend_set*. TIA Dominique
RE: [PATCH, Fortran] PR61234: -Wuse-no-only
Attached the reworked patch. The only change is that the warning is now not
part of -Wall, given the consensus on the list.
The patch has been bootstrapped and regtested on x86_64-unknown-linux-gnu. If
OK, please apply to trunk.
gcc/fortran/ChangeLog:
2014-06-04 Joost VandeVondele
PR fortran/61234
* lang.opt (Wuse-no-only): New flag.
* gfortran.h (gfc_option_t): Add it.
* invoke.texi: Document it.
* module.c (gfc_use_module): Warn if needed.
* options.c (gfc_init_options,gfc_handle_option): Init accordingly.
gcc/testsuite/ChangeLog:
2014-06-04 Joost VandeVondele
* gfortran.dg/use_no_only_1.f90: New test.
Index: gcc/fortran/options.c
===
--- gcc/fortran/options.c (revision 211094)
+++ gcc/fortran/options.c (working copy)
@@ -105,6 +105,7 @@ gfc_init_options (unsigned int decoded_o
gfc_option.warn_tabs = 1;
gfc_option.warn_underflow = 1;
gfc_option.warn_intrinsic_shadow = 0;
+ gfc_option.warn_use_no_only = 0;
gfc_option.warn_intrinsics_std = 0;
gfc_option.warn_align_commons = 1;
gfc_option.warn_real_q_constant = 0;
@@ -730,6 +731,10 @@ gfc_handle_option (size_t scode, const c
gfc_option.warn_intrinsic_shadow = value;
break;
+case OPT_Wuse_no_only:
+ gfc_option.warn_use_no_only = value;
+ break;
+
case OPT_Walign_commons:
gfc_option.warn_align_commons = value;
break;
Index: gcc/fortran/gfortran.h
===
--- gcc/fortran/gfortran.h (revision 211022)
+++ gcc/fortran/gfortran.h (working copy)
@@ -2321,6 +2321,7 @@ typedef struct
int warn_tabs;
int warn_underflow;
int warn_intrinsic_shadow;
+ int warn_use_no_only;
int warn_intrinsics_std;
int warn_character_truncation;
int warn_array_temp;
Index: gcc/fortran/lang.opt
===
--- gcc/fortran/lang.opt (revision 211022)
+++ gcc/fortran/lang.opt (working copy)
@@ -257,6 +257,10 @@ Wintrinsics-std
Fortran Warning
Warn on intrinsics not part of the selected standard
+Wuse-no-only
+Fortran Warning
+Warn about USE statements that have no only qualifier
+
Wopenmp-simd
Fortran
; Documented in C
Index: gcc/fortran/invoke.texi
===
--- gcc/fortran/invoke.texi (revision 211022)
+++ gcc/fortran/invoke.texi (working copy)
@@ -142,7 +142,7 @@ and warnings}.
@gccoptlist{-Waliasing -Wall -Wampersand -Warray-bounds
-Wc-binding-type -Wcharacter-truncation @gol
-Wconversion -Wfunction-elimination -Wimplicit-interface @gol
--Wimplicit-procedure -Wintrinsic-shadow -Wintrinsics-std @gol
+-Wimplicit-procedure -Wintrinsic-shadow -Wuse-no-only -Wintrinsics-std @gol
-Wline-truncation -Wno-align-commons -Wno-tabs -Wreal-q-constant @gol
-Wsurprising -Wunderflow -Wunused-parameter -Wrealloc-lhs -Wrealloc-lhs-all @gol
-Wtarget-lifetime -fmax-errors=@var{n} -fsyntax-only -pedantic -pedantic-errors
@@ -896,6 +896,13 @@ intrinsic; in this case, an explicit int
@code{INTRINSIC} declaration might be needed to get calls later resolved to
the desired intrinsic/procedure. This option is implied by @option{-Wall}.
+@item -Wuse-no-only
+@opindex @code{Wuse-no-only}
+@cindex warnings, use statements
+@cindex intrinsic
+Warn if a use statement has no only qualifier and thus implicitly imports
+all public entities of the used module.
+
@item -Wunused-dummy-argument
@opindex @code{Wunused-dummy-argument}
@cindex warnings, unused dummy argument
Index: gcc/fortran/module.c
===
--- gcc/fortran/module.c (revision 211022)
+++ gcc/fortran/module.c (working copy)
@@ -6398,6 +6398,9 @@ gfc_use_module (gfc_use_list *module)
gfc_rename_list = module->rename;
only_flag = module->only_flag;
+ if (!only_flag && gfc_option.warn_use_no_only)
+gfc_warning_now ("USE statement at %C has no ONLY qualifier");
+
filename = XALLOCAVEC (char, strlen (module_name) + strlen (MODULE_EXTENSION)
+ 1);
strcpy (filename, module_name);
Index: gcc/testsuite/gfortran.dg/use_no_only_1.f90
===
--- gcc/testsuite/gfortran.dg/use_no_only_1.f90 (revision 0)
+++ gcc/testsuite/gfortran.dg/use_no_only_1.f90 (revision 0)
@@ -0,0 +1,44 @@
+! PR fortran/61234 Warn for use-stmt without explicit only-list.
+! { dg-do compile }
+! { dg-options "-Wuse-no-only" }
+MODULE foo
+ INTEGER :: bar
+END MODULE
+
+MODULE testmod
+ USE foo ! { dg-warning "has no ONLY qualifier" }
+ IMPLICIT NONE
+CONTAINS
+ SUBROUTINE S1
+ USE foo ! { dg-warning "has no ONLY qualifier" }
+ END SUBROUTINE S1
+ SUBROUTINE S2
+ USE foo, ONLY: bar ! { dg-bogus "has no ONLY qualifier" }
+ END SUBROUTINE
+ SUBROUTINE S3
+ USE ISO_C_BINDING ! { dg-warning "has no ONLY qualifier" }
+ END SUBROUTINE
RE: [PATCH, Fortan] fix initialization of flag_errno_math and flag_associative_math
I have now verified that both new testcases indeed pass with gcc version 4.6.0 20100520 (experimental) [trunk revision 159620] (GCC) that is the revision where Tobias enabled associative math by default. So that this patch fixes a regression.
RE: [PATCH, Fortan] fix initialization of flag_errno_math and flag_associative_math
> Why do you want -fno-math-errno to be the default for gfortran? because it was with rth's commit? This makes sense also because errno is a variable that is defined for C, and not accessible from Fortran. Why would you want -fmath-errno to be the default ? > if (flag_associative_math == -1) >flag_associative_math = (!flag_trapping_math && !flag_signed_zeros); > Why this is not working is beyond my understanding, but I am not sure that > your fix is the right one. while the details of the option handling are rather opaque to me to, at the point of reaching this statement, flag_associative_math currently equals 0 or 1 even if the user didn't specify the flag. I believe as set by common_handle_option. With my patch, the value of -1 is seen when the user did not specify the flag explicitly. Note that the go frontend uses a similar approach for errno. Joost
RE: [PATCH, Fortran] PR61234: -Wuse-no-only
> What is the rationale of
>
> + SUBROUTINE S2
> + USE foo, ONLY: bar ! { dg-bogus "has no ONLY qualifier" }
> + END SUBROUTINE
This explicitly tests that no bogus error message is issued for a use statement
that has an only qualifier ?
[PATCH, Fortan] fix initialization of flag_errno_math and flag_associative_math
Hi,
the Fortran FE sets flag_errno_math and flag_associative_math
https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=94447
https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=159620
Seemingly it (now?) needs to communicate that it is doing so, since otherwise
this is overwritten later on and ignored.
The attached patch does so and comes with two testcases to verify the expected
effect.
The patch has been bootstrapped and regtested on x86_64-unknown-linux-gnu. If
OK, please apply to trunk.
Thanks in advance,
Joostgcc/fortran/ChangeLog:
2014-05-30 Joost VandeVondele
* options.c (gfc_init_options_struct): assert that the frontend sets
flag_errno_math and flag_associative_math.
gcc/testsuite/ChangeLog:
2014-05-30 Joost VandeVondele
* gfortran.dg/errnocheck_1.f90: New test.
* gfortran.dg/associative_1.f90: New test.
Index: gcc/fortran/options.c
===
--- gcc/fortran/options.c (revision 211022)
+++ gcc/fortran/options.c (working copy)
@@ -66,7 +66,9 @@ void
gfc_init_options_struct (struct gcc_options *opts)
{
opts->x_flag_errno_math = 0;
+ opts->frontend_set_flag_errno_math = true;
opts->x_flag_associative_math = -1;
+ opts->frontend_set_flag_associative_math = true;
}
/* Get ready for options handling. Keep in sync with
Index: gcc/testsuite/gfortran.dg/errnocheck_1.f90
===
--- gcc/testsuite/gfortran.dg/errnocheck_1.f90 (revision 0)
+++ gcc/testsuite/gfortran.dg/errnocheck_1.f90 (revision 0)
@@ -0,0 +1,8 @@
+! { dg-do compile { target x86_64-*-* } }
+! Fortran should default to -fno-math-errno
+! and thus no call to sqrt in asm
+subroutine mysqrt(a)
+ real(KIND=KIND(0.0D0)) :: a
+ a=sqrt(a)
+end subroutine
+! { dg-final { scan-assembler-times "call" 0 } }
Index: gcc/testsuite/gfortran.dg/associative_1.f90
===
--- gcc/testsuite/gfortran.dg/associative_1.f90 (revision 0)
+++ gcc/testsuite/gfortran.dg/associative_1.f90 (revision 0)
@@ -0,0 +1,10 @@
+! { dg-do compile }
+! { dg-options "-O1 -fno-signed-zeros -fno-trapping-math -fdump-tree-optimized" }
+! Fortran defaults to associative by default,
+! with -fno-signed-zeros -fno-trapping-math this should optimize away all additions
+SUBROUTINE S1(a)
+ REAL :: a
+ a=1+a-1
+END SUBROUTINE S1
+! { dg-final { scan-tree-dump-times " \\\+ " 0 "optimized" } }
+! { dg-final { cleanup-tree-dump "optimized" } }
RE: [PATCH, Fortran] PR61234: -Wuse-no-only
> I think it is really weird if a coding style warning is included in -Wall. I have no strong opinion on this, I followed the -Wintrinsic-shadow example, and I'm happy to change things. Note however, that even the Fortran standard recommends the ONLY option for example for intrinsic modules, to guarantee that a program is portable to other processors and future versions of the standard, i.e. to avoid potential name conflicts (see Note 15.1 in F2003 standard).
[PATCH, Fortran] PR61234: -Wuse-no-only
Hi,
the attached patch fixes PR61234 by introducing a warning for modules used
without an only qualifier, triggered by a flag '-Wuse-no-only'.
It is a two line addition to module.c, plus mechanical changes to initialize
the flag and keep the tests up-to-date. I currently have no copyright
assignment filed, so I hope it can pass as 'trivial' or it needs to wait till I
get the paperwork sorted.
The patch has been bootstrapped and regtested on x86_64-unknown-linux-gnu. If
OK, please apply to trunk.
gcc/fortran/ChangeLog:
2014-05-29 Joost VandeVondele
PR fortran/61234
* lang.opt (Wuse-no-only): New flag.
* gfortran.h (gfc_option_t): Add it.
* invoke.texi: Document it.
* module.c (gfc_use_module): Warn if needed.
* options.c (gfc_init_options,set_Wall,gfc_handle_option): Init
accordingly.
gcc/testsuite/ChangeLog:
2014-05-29 Joost VandeVondele
* gfortran.dg/c_by_val_5.f90: Add expected -Wall warning.
* gfortran.dg/transfer_check_4.f90: idem.
* gfortran.dg/iso_c_binding_compiler_3.f90: idem.
* gfortran.dg/pr52370.f90: idem.
* gfortran.dg/use_no_only_1.f90: New test.
Thanks in advance,
Joostgcc/fortran/ChangeLog:
2014-05-29 Joost VandeVondele
PR fortran/61234
* lang.opt (Wuse-no-only): New flag.
* gfortran.h (gfc_option_t): Add it.
* invoke.texi: Document it.
* module.c (gfc_use_module): Warn if needed.
* options.c (gfc_init_options,set_Wall,gfc_handle_option): Init accordingly.
gcc/testsuite/ChangeLog:
2014-05-29 Joost VandeVondele
* gfortran.dg/c_by_val_5.f90: Add expected -Wall warning.
* gfortran.dg/transfer_check_4.f90: idem.
* gfortran.dg/iso_c_binding_compiler_3.f90: idem.
* gfortran.dg/pr52370.f90: idem.
* gfortran.dg/use_no_only_1.f90: New test.
Index: gcc/testsuite/gfortran.dg/c_by_val_5.f90
===
--- gcc/testsuite/gfortran.dg/c_by_val_5.f90 (revision 211022)
+++ gcc/testsuite/gfortran.dg/c_by_val_5.f90 (working copy)
@@ -59,7 +59,7 @@ END module x
!end subroutine test
program main
- use x
+ use x ! { dg-warning "has no ONLY qualifier" }
implicit none
! external test
call Grid2BMP(10)
Index: gcc/testsuite/gfortran.dg/transfer_check_4.f90
===
--- gcc/testsuite/gfortran.dg/transfer_check_4.f90 (revision 211022)
+++ gcc/testsuite/gfortran.dg/transfer_check_4.f90 (working copy)
@@ -6,7 +6,7 @@
subroutine transfers (test)
- use, intrinsic :: iso_fortran_env
+ use, intrinsic :: iso_fortran_env ! { dg-warning "has no ONLY qualifier" }
integer, intent(in) :: test
Index: gcc/testsuite/gfortran.dg/iso_c_binding_compiler_3.f90
===
--- gcc/testsuite/gfortran.dg/iso_c_binding_compiler_3.f90 (revision 211022)
+++ gcc/testsuite/gfortran.dg/iso_c_binding_compiler_3.f90 (working copy)
@@ -7,8 +7,8 @@
! "Type specified for intrinsic function" for this file
!
-use iso_c_binding
-use iso_Fortran_env
+use iso_c_binding ! { dg-warning "has no ONLY qualifier" }
+use iso_Fortran_env ! { dg-warning "has no ONLY qualifier" }
implicit none
intrinsic sin
real :: x = 3.4
@@ -17,9 +17,9 @@ end
module test_mod
-use iso_fortran_env
+use iso_fortran_env ! { dg-warning "has no ONLY qualifier" }
end module test_mod
subroutine test
-use test_mod
+use test_mod ! { dg-warning "has no ONLY qualifier" }
end subroutine test
Index: gcc/testsuite/gfortran.dg/use_no_only_1.f90
===
--- gcc/testsuite/gfortran.dg/use_no_only_1.f90 (revision 0)
+++ gcc/testsuite/gfortran.dg/use_no_only_1.f90 (revision 0)
@@ -0,0 +1,22 @@
+! PR fortran/61234 Warn for use-stmt without explicit only-list.
+! { dg-do compile }
+! { dg-options "-Wuse-no-only" }
+MODULE foo
+ INTEGER :: bar
+END MODULE
+
+MODULE testmod
+ USE foo ! { dg-warning "has no ONLY qualifier" }
+ IMPLICIT NONE
+CONTAINS
+ SUBROUTINE S1
+ USE foo ! { dg-warning "has no ONLY qualifier" }
+ END SUBROUTINE S1
+ SUBROUTINE S2
+ USE foo, ONLY: bar ! { dg-bogus "has no ONLY qualifier" }
+ END SUBROUTINE
+ SUBROUTINE S3
+ USE ISO_C_BINDING ! { dg-warning "has no ONLY qualifier" }
+ END SUBROUTINE S3
+END MODULE
+! { dg-final { cleanup-modules "foo testmod" } }
Index: gcc/testsuite/gfortran.dg/pr52370.f90
===
--- gcc/testsuite/gfortran.dg/pr52370.f90 (revision 211022)
+++ gcc/testsuite/gfortran.dg/pr52370.f90 (working copy)
@@ -15,7 +15,7 @@ contains
end module pr52370
program prg52370
- use pr52370
+ use pr52370 ! { dg-warning "has no ONLY qualifier" }
real :: a
call foo(a)
end program prg52370
Index: gcc/fortran/invoke.texi
===
--- gcc/fortran/invoke.texi (revision 211022)
RE: Fix for PR55561 race condition in libgomp
The updated changelog entry is below, but somebody with write access should do the commit, please. 2013-01-31 Dmitry Vyukov Joost VandeVondele PR libgomp/55561 * config/linux/wait.h (do_spin): Use atomic load for addr. * config/linux/ptrlock.c (gomp_ptrlock_get_slow): Use atomic for intptr and ptrlock. * config/linux/ptrlock.h (gomp_ptrlock_get): Use atomic load for ptrlock.
Fix for PR55561 race condition in libgomp
The attached patch fixes a race condition in libgomp. Based on the suggestions
by Dmitry, and verified that it fixes the corresponding sanitizer warnings.
2012-12-30 Dmitry Vyukov
PR libgomp/55561
* config/linux/wait.h (do_spin): Use atomic load for addr.
* config/linux/ptrlock.c (gomp_ptrlock_get_slow): Use atomic
for intptr and ptrlock.
* onfig/linux/ptrlock.h (gomp_ptrlock_get): Use atomic load
for ptrlock.
Index: libgomp/config/linux/wait.h
===
--- libgomp/config/linux/wait.h (revision 194730)
+++ libgomp/config/linux/wait.h (working copy)
@@ -51,7 +51,7 @@ static inline int do_spin (int *addr, in
if (__builtin_expect (gomp_managed_threads > gomp_available_cpus, 0))
count = gomp_throttled_spin_count_var;
for (i = 0; i < count; i++)
-if (__builtin_expect (*addr != val, 0))
+if (__builtin_expect (__atomic_load_n (addr, MEMMODEL_RELAXED) != val, 0))
return 0;
else
cpu_relax ();
Index: libgomp/config/linux/ptrlock.c
===
--- libgomp/config/linux/ptrlock.c (revision 194730)
+++ libgomp/config/linux/ptrlock.c (working copy)
@@ -50,9 +50,9 @@ gomp_ptrlock_get_slow (gomp_ptrlock_t *p
#endif
do
do_wait (intptr, 2);
- while (*intptr == 2);
+ while (__atomic_load_n (intptr, MEMMODEL_RELAXED) == 2);
__asm volatile ("" : : : "memory");
- return *ptrlock;
+ return (void *) __atomic_load_n (ptrlock, MEMMODEL_ACQUIRE);
}
void
Index: libgomp/config/linux/ptrlock.h
===
--- libgomp/config/linux/ptrlock.h (revision 194730)
+++ libgomp/config/linux/ptrlock.h (working copy)
@@ -48,8 +48,9 @@ static inline void *gomp_ptrlock_get (go
{
uintptr_t oldval;
- if ((uintptr_t) *ptrlock > 2)
-return *ptrlock;
+ uintptr_t v = (uintptr_t) __atomic_load_n (ptrlock, MEMMODEL_ACQUIRE);
+ if (v > 2)
+return (void *) v;
oldval = 0;
if (__atomic_compare_exchange_n (ptrlock, &oldval, 1, false,
